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

[Jan Beulich <jbeulich@xxxxxxxx>]

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. (In no case, imo,
there should be parentheses around a negation expression.)

>          page_set_owner(&pg[i], d);
>          smp_wmb(); /* Domain pointer must be visible before updating refcnt. 
> */
> -        pg[i].count_info = PGC_allocated | 1;
> +        pg[i].count_info = PGC_state_inuse | PGC_allocated | 1;

As opposed to the above, I certainly appreciate the change here.

Jan

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