Re: [PATCH v7 1/3] xen/mm: Introduce per-node free page counter

[Jan Beulich <jbeulich@xxxxxxxx>]

On 08.05.2026 22:27, Bernhard Kaindl wrote:
> From: Alejandro Vallejo <alejandro.vallejo@xxxxxxxxx>
> 
> Add node_avail_pages[], updated under heap_lock in sync with
> avail[node][zone] to cache the per-node sum of free pages.
> 
> Use it in avail_node_heap_pages() to avoid summing all zones on each
> call. Guard it with nodeid < MAX_NUMNODES and node_online(nodeid).
> 
> Signed-off-by: Alejandro Vallejo <alejandro.vallejo@xxxxxxxxx>
> Signed-off-by: Bernhard Kaindl <bernhard.kaindl@xxxxxxxxxx>
> ---
> This patch was originally sent by Alejandro Vallejo:
> https://lists.xenproject.org/archives/html/xen-devel/2025-03/msg01130.html
> 
> I use node_avail_pages[] in avail_node_heap_pages() as an optimisation.
> 
> Verification of the changes:
> 
> 1. node_avail_pages[node] is updated whenever avail[node][zone] changes,
>    so the two remain in sync.
> 
> 2. avail_node_heap_pages() previously summed all zones of a node and now
>    returns node_avail_pages[node], so the same free buddy pages are
>    counted.
> 
> 3. avail_node_heap_pages() returns 0 for offline nodes and for nodes
>    >= MAX_NUMNODES as before.
> 
> 4. avail_node_heap_pages(-1) returned the sum from all nodes, equal
>    to total_avail_pages, but this is not used by current callers.
>    avail_heap_pages(z, z, -1) is used by other callers for that instead.
>    To avoid dead code, a check for -1 to implement this is not added.
> 
> Update locations:
> 
> - free_heap_pages() increments node_avail_pages[node] alongside
>   avail[node][zone] when pages are freed, including during heap
>   initialisation.
> 
> - alloc_heap_pages() decrements node_avail_pages[node] alongside
>   avail[node][zone] when pages are allocated.
> 
> - reserve_offlined_page() decrements node_avail_pages[node] alongside
>   avail[node][zone] when pages are offlined.
> 
> Colored pages do not go through the buddy allocator.
> Since they do not update avail[node][zone], they are
> not reflected in node_avail_pages[node] either.
> 
> N.B. Current callers already iterate over online nodes only.
> 
> Changes since v6:
> - Preserved the 0 return for offline nodes and nodes >= MAX_NUMNODES.

Hard to identify what v6 was (and who, if anyone, asked for the change) when,
afaict, the patch subject changed.

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
with ...

> @@ -2831,7 +2837,9 @@ unsigned long avail_domheap_pages_region(
>  
>  unsigned long avail_node_heap_pages(unsigned int nodeid)
>  {
> -    return avail_heap_pages(MEMZONE_XEN, NR_ZONES -1, nodeid);
> +    if ( nodeid < MAX_NUMNODES && node_online(nodeid) )
> +        return node_avail_pages[nodeid];
> +    return 0;
>  }

... a blank line inserted ahead of the "main" (not really here, just by its
indentation) return.

One other remark: With the function called from just a sysctl and a
keyhandler, the direct array access is likely fine. Generally it would
want to be array_access_nospec() though, when accessible from guests.

Jan