[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] xen/mm: Avoid assuming PG_state_inuse == 0 in assign_pages()

On 04/02/2020 13:40, Jan Beulich wrote:
On 04.02.2020 14:33, Julien Grall wrote:
At the moment, assign_pages() relies on PG_state_inuse to be 0. This
makes the code slightly more difficult to understand.

I can certainly see where you're coming from, but ...

--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -2286,10 +2286,11 @@ int assign_pages(
      for ( i = 0; i < (1 << order); i++ )
          ASSERT(page_get_owner(&pg[i]) == NULL);
-        ASSERT(!pg[i].count_info);
+        ASSERT(page_state_is(&pg[i], inuse));
+        ASSERT(!(pg[i].count_info & (~PGC_state)));

... I think this one is better in its original form. An option
might be to put a BUILD_BUG_ON() next to it.

I want to avoid a BUILD_BUG_ON() if possible. I just realized, I could simplify to "(pg[i].count_info != PGC_state_inuse)".

Would that be more suitable?

(In no case, imo,
there should be parentheses around a negation expression.)

While I understand this would be valid, I find it a bit easier to read.


Julien Grall

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.