Re: [PATCH v4 01/10] xen/page_alloc: Extract code for consuming claims into inline function

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Thu, Feb 26, 2026 at 02:29:15PM +0000, Bernhard Kaindl wrote:
> Refactor the claims consumption code in preparation for node-claims.
> Lays the groundwork for adding the consumption of NUMA claims to it.
> 
> Signed-off-by: Bernhard Kaindl <bernhard.kaindl@xxxxxxxxxx>
> ---
>  xen/common/page_alloc.c | 56 +++++++++++++++++++++++------------------
>  1 file changed, 31 insertions(+), 25 deletions(-)
> 
> diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
> index 588b5b99cbc7..6f7f30c64605 100644
> --- a/xen/common/page_alloc.c
> +++ b/xen/common/page_alloc.c
> @@ -518,6 +518,34 @@ unsigned long domain_adjust_tot_pages(struct domain *d, 
> long pages)
>      return d->tot_pages;
>  }
>  
> +/* Release outstanding claims on the domain, host and later also node */
> +static inline
> +void release_outstanding_claims(struct domain *d, unsigned long release)
> +{
> +    ASSERT(spin_is_locked(&heap_lock));
> +    BUG_ON(outstanding_claims < release);
> +    outstanding_claims -= release;
> +    d->outstanding_pages -= release;
> +}
> +
> +/*
> + * Consume outstanding claimed pages when allocating pages for a domain.
> + * NB. The alloc could (in principle) fail in assign_pages() afterwards. In 
> that
> + * case, the consumption is not reversed, but as claims are used only during
> + * domain build and d is destroyed if the build fails, this has no 
> significance.
> + */
> +static inline
> +void consume_outstanding_claims(struct domain *d, unsigned long allocation)
> +{
> +    if ( !d || !d->outstanding_pages )
> +        return;
> +    ASSERT(spin_is_locked(&heap_lock));
> +
> +    /* Of course, the domain can only release up its outstanding claims */
> +    allocation = min(allocation, d->outstanding_pages + 0UL);
> +    release_outstanding_claims(d, allocation);
> +}
> +
>  int domain_set_outstanding_pages(struct domain *d, unsigned long pages)
>  {
>      int ret = -ENOMEM;
> @@ -535,8 +563,7 @@ int domain_set_outstanding_pages(struct domain *d, 
> unsigned long pages)
>      /* pages==0 means "unset" the claim. */
>      if ( pages == 0 )
>      {
> -        outstanding_claims -= d->outstanding_pages;
> -        d->outstanding_pages = 0;
> +        release_outstanding_claims(d, d->outstanding_pages);
>          ret = 0;
>          goto out;
>      }
> @@ -1048,29 +1075,8 @@ static struct page_info *alloc_heap_pages(
>      total_avail_pages -= request;
>      ASSERT(total_avail_pages >= 0);
>  
> -    if ( d && d->outstanding_pages && !(memflags & MEMF_no_refcount) )
> -    {
> -        /*
> -         * Adjust claims in the same locked region where total_avail_pages is
> -         * adjusted, not doing so would lead to a window where the amount of
> -         * free memory (avail - claimed) would be incorrect.

As Jan mentioned, you really need to keep this part of the comment.
Claims had been broken since its introduction because the above was
not respected, and that resulted in the accounting for free pages
being transiently incorrect while an allocation was taking place.

Thanks, Roger.