Re: [PATCH v4 02/10] xen/page_alloc: Optimize getting per-NUMA-node free page counts

[Jan Beulich <jbeulich@xxxxxxxx>]

On 26.02.2026 15:29, Bernhard Kaindl wrote:
> From: Alejandro Vallejo <alejandro.vallejo@xxxxxxxxx>
> 
> Add per-node free page counters (node_avail_pages[]), protected by
> heap_lock, updated in real-time in lockstep with total_avail_pages
> as pages are allocated and freed.
> 
> This replaces the avail_heap_pages() loop over all online nodes and
> zones in avail_node_heap_pages() with a direct O(1) array lookup,
> making it efficient to get the total free pages for a given NUMA node.
> 
> The per-node counts are currently provided using sysctl for NUMA
> placement decisions of domain builders and monitoring, and for
> debugging with the debug-key 'u' to print NUMA info to the printk buffer.
> 
> They will also be used for checking if a NUMA node may be able to
> satisfy a NUMA-node-specific allocation by comparing node availability
> against node-specific claims before looking for pages in the zones
> of the node.
> 
> Also change total_avail_pages and outstanding_claims to unsigned long:
> 
> Those never become negative (we protect that with ASSERT/BUG_ON already),
> and converting them to unsigned long makes that explicit, and also
> fixes signed/unsigned comparison warnings.

This wants to be a separate commit. It hasn't got anything to do in here.

> This only needs moving the ASSERT to before the subtraction.
> See the previous commit moving the BUG_ON for outstanding_claims.

Please can you avoid such statements? You won't know in which order the
patches are committed: Patch 01 may go in weeks or months before patch
02.

> --- a/xen/common/page_alloc.c
> +++ b/xen/common/page_alloc.c
> @@ -483,11 +483,32 @@ static heap_by_zone_and_order_t *_heap[MAX_NUMNODES];
>  
>  static unsigned long node_need_scrub[MAX_NUMNODES];
>  
> +/* avail[node][zone] is the number of free pages on that node and zone. */
>  static unsigned long *avail[MAX_NUMNODES];
> -static long total_avail_pages;
> +/* Global available pages, updated in real-time, protected by heap_lock */
> +static unsigned long total_avail_pages;
>  
> +/* The global heap lock, protecting access to the heap and related 
> structures */
>  static DEFINE_SPINLOCK(heap_lock);
> -static long outstanding_claims; /* total outstanding claims by all domains */
> +
> +/*
> + * Per-node count of available pages, protected by heap_lock, updated in
> + * lockstep with total_avail_pages as pages are allocated and freed.
> + *
> + * Each entry holds the sum of avail[node][zone] across all zones, used for
> + * efficiently checking node-local availability for allocation requests.
> + * Also provided via sysctl for NUMA placement decisions of domain builders
> + * and monitoring, and logged with debug-key 'u' for NUMA debugging.
> + *
> + * Maintaining this under heap_lock does not reduce scalability, as the
> + * allocator is already serialized on it. The accessor macro abstracts the
> + * storage to ease future changes (e.g. moving to per-node lock granularity).
> + */
> +#define node_avail_pages(node) (node_avail_pages[node])

This isn't really needed when ...

> +static unsigned long node_avail_pages[MAX_NUMNODES];

... it's a static array anyway. Plus you may want to talk to Andrew regarding
the use of such a macro as an lvalue.

> +/* total outstanding claims by all domains */
> +static unsigned long outstanding_claims;

As you touch it, comment style wants correcting.

Jan