Re: [PATCH v9 08/13] xen/page_alloc: introduce preserved page flags macro

[Jan Beulich <jbeulich@xxxxxxxx>]

On 25.10.2024 11:50, Carlo Nonato wrote:
> PGC_static and PGC_extra needs to be preserved when assigning a page.
> Define a new macro that groups those flags and use it instead of or'ing
> every time.
> 
> To make preserved flags even more meaningful, they are kept also when
> switching state in mark_page_free().
> Enforce the removal of PGC_extra before freeing new pages as this is
> considered an error and can cause ASSERT violations.

What does "new" here mean?

> Signed-off-by: Carlo Nonato <carlo.nonato@xxxxxxxxxxxxxxx>
> ---
> v9:
> - add PGC_broken to PGC_preserved

Which then also wants reflecting in the description.

> @@ -2485,6 +2485,14 @@ struct page_info *alloc_domheap_pages(
>          }
>          if ( assign_page(pg, order, d, memflags) )
>          {
> +            if ( memflags & MEMF_no_refcount )
> +            {
> +                unsigned long i;
> +
> +                for ( i = 0; i < (1UL << order); i++ )
> +                    pg[i].count_info &= ~PGC_extra;
> +            }

The description doesn't cover this, only ...

> @@ -2539,6 +2547,7 @@ void free_domheap_pages(struct page_info *pg, unsigned 
> int order)
>                  {
>                      ASSERT(d->extra_pages);
>                      d->extra_pages--;
> +                    pg[i].count_info &= ~PGC_extra;
>                  }
>              }

... this is mentioned there. And it's not clear to me why it would need doing
in both places.

Jan