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

[Jan Beulich <jbeulich@suse.com>]

On 02.05.2024 18:55, Carlo Nonato wrote:
> --- a/xen/common/page_alloc.c
> +++ b/xen/common/page_alloc.c
> @@ -159,6 +159,7 @@
>  #endif
>  
>  #define PGC_no_buddy_merge PGC_static
> +#define PGC_preserved (PGC_extra | PGC_static)

Seeing this again and its use ...

> @@ -1426,11 +1427,11 @@ static bool mark_page_free(struct page_info *pg, mfn_t mfn)
>      {
>      case PGC_state_inuse:
>          BUG_ON(pg->count_info & PGC_broken);
> -        pg->count_info = PGC_state_free;
> +        pg->count_info = PGC_state_free | (pg->count_info & PGC_preserved);
>          break;
>  
>      case PGC_state_offlining:
> -        pg->count_info = (pg->count_info & PGC_broken) |
> +        pg->count_info = (pg->count_info & (PGC_broken | PGC_preserved)) |
>                           PGC_state_offlined;
>          pg_offlined = true;
>          break;

... here: Shouldn't PGC_broken also be included in PGC_preserved?

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

For larger order this loop is non-trivial and may have a fair effect on
caches. Looking at the code just outside of upper patch context, is this
loop needed at all when MEMF_no_refcount is clear in memflags?

Jan