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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 26.02.2026 15:29, Bernhard Kaindl wrote:
> --- 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

Generally we prefer to avoid "inline" in .c files. This is better left to the
compiler. Furthermore while we have a few examples of this kind of line split,
it's clearly not the preferred form. You'll find ample well-formed static
functions in this one source file alone.

> +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));

Why is this not the first thing in the function?

> @@ -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.
> -         *
> -         * Note that by adjusting the claimed amount here it's possible for
> -         * pages to fail to be assigned to the claiming domain while already
> -         * having been subtracted from d->outstanding_pages.  Such claimed
> -         * amount is then lost, as the pages that fail to be assigned to the
> -         * domain are freed without replenishing the claim.  This is fine 
> given
> -         * claims are only to be used during physmap population as part of
> -         * domain build, and any failure in assign_pages() there will result 
> in
> -         * the domain being destroyed before creation is finished.  Losing 
> part
> -         * of the claim makes no difference.
> -         */

Much of this comment is lost. Parts have been moved, but I think another part
(in particular the first paragraph) wants to be retained here. Plus in general
when rearranging code it is best to take the original commentary as is (typo
or factual corrections of course included as necessary).

Jan