Re: [PATCH v6 1/7] xen/mm: Refactor claim deduction for later functional changes

[Jan Beulich <jbeulich@xxxxxxxx>]

On 14.04.2026 15:22, Bernhard Kaindl wrote:
> Refactor claim deduction to make the claims accounting model easier to
> follow ahead of later functional changes.
> 
> Three new callers will need to deduct claims, and two of them will also
> need the number of pages deducted to be returned.
> 
> No functional change.
> 
> Signed-off-by: Bernhard Kaindl <bernhard.kaindl@xxxxxxxxxx>
> ---
> History:
> 
> Functionally unchanged since v4, only minor cleanups have been applied and:
> - Updated the function name deduct_global_claims() to follow standard naming.
> - Moved the other function into another commit as it had no users here.

Is this true? In v4 ...

> --- a/xen/common/page_alloc.c
> +++ b/xen/common/page_alloc.c
> @@ -518,6 +518,19 @@ unsigned long domain_adjust_tot_pages(struct domain *d, 
> long pages)
>      return d->tot_pages;
>  }
>  
> +/* Deduct up to the given amount of pages from the global claims of a domain 
> */
> +static unsigned long deduct_global_claims(struct domain *d,
> +                                          unsigned long reduction)
> +{
> +    reduction = min(reduction, d->outstanding_pages + 0UL);
> +    ASSERT(reduction <= outstanding_claims);

... this was still BUG_ON(), matching ...

> @@ -1067,11 +1079,7 @@ static struct page_info *alloc_heap_pages(
>           * the domain being destroyed before creation is finished.  Losing 
> part
>           * of the claim makes no difference.
>           */
> -        unsigned long outstanding = min(d->outstanding_pages + 0UL, request);
> -
> -        BUG_ON(outstanding > outstanding_claims);

... the original code. Changing this may be okay (albeit I'm unconvinced),
but it would want justifying in the description then.

As to the rename to deduct_global_claims(): With that, wouldn't its 2nd
parameter then better also change to "deduction"? Furthermore, "global" in
the name is ambiguous: It may mean "not per-node", but it may also mean
"not per-domain". The v4 name didn't have such an issue.

One other, wider aspect (which I may or may not have mentioned before):
The mix of unsigned long vs unsigned int for page counts is concerning. It
is why the not overly nice "+ 0UL" is needed. I wonder if we wouldn't do
ourselves a favor if we first normalized all that. Considering that
systems (and hence guests) only ever get larger, it likely should all be
unsigned long.

Jan