With a single global count for the claims it is easy to substract
domain_tot_pages() from the claim so the number given in the hypercall
is the real reservation of the domain. This is the current behaviour.

However, a later patch introduces node-specific claims and those interact
very poorly with such a scheme. Since accounting domain_tot_pages() in
one case but not the other seems strictly worse than not accounting them
at all (which is at least consistent), this patch stops substracting
tot_pages from the claim and instead checks that claimed memory +
allocated memory don't exceed max_mem.

Arguably it's also clearer for the caller to align the amount of claimed
memory with that of the requested claim. xl/libxenguest code never updated
an existing claim: It stakes a claim, allocates all domain memory, cancels
a possible leftover claim, finishes building the domain and unpauses it.

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

diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index e1ac22b9ed..7e90b9cc1e 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -550,7 +550,7 @@ out:
 int domain_set_outstanding_pages(struct domain *d, unsigned long pages)
 {
     int ret = -ENOMEM;
-    unsigned long claim, avail_pages;
+    unsigned long avail_pages;
 
     /*
      * take the domain's page_alloc_lock, else all d->tot_page adjustments
@@ -576,28 +576,21 @@ int domain_set_outstanding_pages(struct domain *d, unsigned long pages)
         goto out;
     }
 
-    /* disallow a claim not exceeding domain_tot_pages() or above max_pages */
-    if ( (pages <= domain_tot_pages(d)) || (pages > d->max_pages) )
+    /* Don't claim past max_pages */
+    if ( (domain_tot_pages(d) + pages) > d->max_pages )
     {
         ret = -EINVAL;
         goto out;
     }
 
     /* how much memory is available? */
-    avail_pages = total_avail_pages;
+    avail_pages = total_avail_pages - outstanding_claims;
 
-    avail_pages -= outstanding_claims;
-
-    /*
-     * Note, if domain has already allocated memory before making a claim
-     * then the claim must take domain_tot_pages() into account
-     */
-    claim = pages - domain_tot_pages(d);
-    if ( claim > avail_pages )
+    if ( pages > avail_pages )
         goto out;
 
     /* yay, claim fits in available memory, stake the claim, success! */
-    d->outstanding_pages = claim;
+    d->outstanding_pages = pages;
     outstanding_claims += d->outstanding_pages;
     ret = 0;
 
-- 
2.43.0

On 16.08.2025 13:19, Bernhard Kaindl wrote:
> With a single global count for the claims it is easy to substract
> domain_tot_pages() from the claim so the number given in the hypercall
> is the real reservation of the domain. This is the current behaviour.
> 
> However, a later patch introduces node-specific claims and those interact
> very poorly with such a scheme. Since accounting domain_tot_pages() in
> one case but not the other seems strictly worse than not accounting them
> at all (which is at least consistent), this patch stops substracting
> tot_pages from the claim and instead checks that claimed memory +
> allocated memory don't exceed max_mem.
> 
> Arguably it's also clearer for the caller to align the amount of claimed
> memory with that of the requested claim. xl/libxenguest code never updated
> an existing claim: It stakes a claim, allocates all domain memory, cancels
> a possible leftover claim, finishes building the domain and unpauses it.
> 
> Signed-off-by: Bernhard Kaindl <bernhard.kaindl@cloud.com>
> Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>

Is this order (and the lack of From:) correct? A patch of the same title was
submitted by Alejandro at some point. Additionally the cover letter lists
this one patch as the sole Alejandro-only one. I'm also uncertain if you may
freely alter the original S-o-b, which was still having his @cloud.com email
address afaict.

> ---
>  xen/common/page_alloc.c | 19 ++++++-------------
>  1 file changed, 6 insertions(+), 13 deletions(-)

From eyeballing both patches nothing has changed. That would support the
tagging as Alejandro-only in the cover letter, but it also means review
comments weren't addressed. Such non-addressing would, however, require a
verbal reply to those review comments, which I can't find any record of.
Instead in a reply to Roger's comments Alejandro indicated that there
indeed was an oversight on his part. My separate comment wasn't replied to
at all.

Jan