domain_adjust_tot_pages() is used to update the domain's total pages
after allocating and freeing memory.

Simplify the design for updating it for single and even more so
for multi-node claims regarding the case where we could have
allocated more memory than we had claims left.

Replace it with min() to avoid reducing the outstadings claims
by more than we had left to claim:

When domain memory is freed, we skip changing the claim. Thus, this
only handles reducing the claims after allocating. So, min() is fine.

Signed-off-by: Bernhard Kaindl <bernhard.kaindl@cloud.com>
Cc: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
---
 xen/common/page_alloc.c | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index ec9dec365e..e1ac22b9ed 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -510,8 +510,14 @@ static unsigned long avail_heap_pages(
     return free_pages;
 }
 
+/*
+ * Update the total number of pages and outstanding claims of a domain.
+ * - When pages were freed, we do not increase outstanding claims.
+ */
 unsigned long domain_adjust_tot_pages(struct domain *d, long pages)
 {
+    unsigned long adjustment;
+
     ASSERT(rspin_is_locked(&d->page_alloc_lock));
     d->tot_pages += pages;
 
@@ -519,23 +525,22 @@ unsigned long domain_adjust_tot_pages(struct domain *d, long pages)
      * can test d->outstanding_pages race-free because it can only change
      * if d->page_alloc_lock and heap_lock are both held, see also
      * domain_set_outstanding_pages below
+     *
+     * If the domain has no outstanding claims (or we freed pages instead),
+     * we don't update outstanding claims and skip the claims adjustment.
      */
     if ( !d->outstanding_pages || pages <= 0 )
         goto out;
 
     spin_lock(&heap_lock);
     BUG_ON(outstanding_claims < d->outstanding_pages);
-    if ( d->outstanding_pages < pages )
-    {
-        /* `pages` exceeds the domain's outstanding count. Zero it out. */
-        outstanding_claims -= d->outstanding_pages;
-        d->outstanding_pages = 0;
-    }
-    else
-    {
-        outstanding_claims -= pages;
-        d->outstanding_pages -= pages;
-    }
+    /*
+     * Reduce claims by outstanding claims or pages (whichever is smaller):
+     * If allocated > outstanding, reduce the claims only by outstanding pages.
+     */
+    adjustment = min(d->outstanding_pages, (unsigned int)pages);
+    d->outstanding_pages -= adjustment;
+    outstanding_claims -= adjustment;
     spin_unlock(&heap_lock);
 
 out:
-- 
2.43.0

On 16.08.2025 13:19, Bernhard Kaindl wrote:
> --- a/xen/common/page_alloc.c
> +++ b/xen/common/page_alloc.c
> @@ -510,8 +510,14 @@ static unsigned long avail_heap_pages(
>      return free_pages;
>  }
>  
> +/*
> + * Update the total number of pages and outstanding claims of a domain.
> + * - When pages were freed, we do not increase outstanding claims.
> + */

If already you add such a comment, please have it be complete: There's
also an update to the global "outstanding_claims" in here.

>  unsigned long domain_adjust_tot_pages(struct domain *d, long pages)
>  {
> +    unsigned long adjustment;
> +
>      ASSERT(rspin_is_locked(&d->page_alloc_lock));
>      d->tot_pages += pages;
>  
> @@ -519,23 +525,22 @@ unsigned long domain_adjust_tot_pages(struct domain *d, long pages)
>       * can test d->outstanding_pages race-free because it can only change
>       * if d->page_alloc_lock and heap_lock are both held, see also
>       * domain_set_outstanding_pages below
> +     *
> +     * If the domain has no outstanding claims (or we freed pages instead),
> +     * we don't update outstanding claims and skip the claims adjustment.
>       */
>      if ( !d->outstanding_pages || pages <= 0 )
>          goto out;
>  
>      spin_lock(&heap_lock);
>      BUG_ON(outstanding_claims < d->outstanding_pages);
> -    if ( d->outstanding_pages < pages )
> -    {
> -        /* `pages` exceeds the domain's outstanding count. Zero it out. */
> -        outstanding_claims -= d->outstanding_pages;
> -        d->outstanding_pages = 0;
> -    }
> -    else
> -    {
> -        outstanding_claims -= pages;
> -        d->outstanding_pages -= pages;
> -    }
> +    /*
> +     * Reduce claims by outstanding claims or pages (whichever is smaller):
> +     * If allocated > outstanding, reduce the claims only by outstanding pages.
> +     */
> +    adjustment = min(d->outstanding_pages, (unsigned int)pages);

This would be all fine if there wasn't the cast. It's only a latent problem,
yes, but I think we still would better avoid introducing such. Imo this wants
to be

    adjustment = min_t(unsigned long, d->outstanding_pages, pages);

or the equivalent

    adjustment = min(d->outstanding_pages + 0UL, pages + 0UL);

(personally I'd prefer the latter despite its odd look, for not involving
any casts).

Jan