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

[Julien Grall <julien@xxxxxxx>]

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.

Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel