Re: [PATCH v5 26/36] mm: add NODE_PRIVATE_CAP_RECLAIM for opted-in private node reclaim

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] calculate_totalreserve_pages() unconditionally skips private nodes, failing to account for their watermarks in totalreserve_pages.
- [Medium] Incorrect zone watermark calculation for private nodes allowing reclaim.
--

--- Patch [26]: [PATCH v5 26/36] mm: add NODE_PRIVATE_CAP_RECLAIM for opted-in private node reclaim ---
commit 29ab758bf14800171bcfa7488632ce55ed680b45
Author: Gregory Price <[email protected]>
Subject: mm: add NODE_PRIVATE_CAP_RECLAIM for opted-in private node reclaim

This commit introduces NODE_PRIVATE_CAP_RECLAIM to allow private memory
nodes to opt into reclaim processes. It replaces various node_is_private()
checks with node_allows_reclaim() across the memory management subsystem.
Private nodes that opt in will now have normal watermarks calculated for
them.

> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 2b08bea2379a9..2667a4564b7ac 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -6667,7 +6667,7 @@ static void __setup_per_zone_wmarks(void)
>  		u64 tmp;
>  
>  		spin_lock_irqsave(&zone->lock, flags);
> -		if (node_is_private(zone_to_nid(zone))) {
> +		if (!node_allows_reclaim(zone_to_nid(zone))) {
>  			zone->_watermark[WMARK_MIN] = 0;
>  			zone->_watermark[WMARK_LOW] = 0;
>  			zone->_watermark[WMARK_HIGH] = 0;

[Severity: Medium]
Will this lead to an incorrect watermark calculation for private nodes that
allow reclaim?

Earlier in __setup_per_zone_wmarks(), lowmem_pages is calculated by
explicitly excluding private memory:

	for_each_zone(zone) {
		if (!is_highmem(zone) && zone_idx(zone) != ZONE_MOVABLE &&
		    !node_is_private(zone_to_nid(zone)))
			lowmem_pages += zone_managed_pages(zone);
	}

Because lowmem_pages excludes private memory, when we calculate WMARK_MIN
for a private node later in this function using:

		tmp = (u64)pages_min * zone_managed_pages(zone);
		tmp = div64_ul(tmp, lowmem_pages);

does this mean the private node will reserve a disproportionately high
percentage of its memory if the system has a small amount of normal memory
and a large private node?


[Severity: Medium]
Also, does this miss updating the totalreserve_pages counter?

At the end of __setup_per_zone_wmarks(), we call
calculate_totalreserve_pages(). However, that function unconditionally
skips all private nodes:

	for_each_online_pgdat(pgdat) {
		/* private nodes have zero watermarks */
		if (node_is_private(pgdat->node_id))
			continue;

If a private node has CAP_RECLAIM and non-zero watermarks, does skipping it
here cause totalreserve_pages to remain artificially low?

This might cause node_dirtyable_memory() and global_dirtyable_memory() to
overestimate available memory, which could inflate dirty limits and
writeback thresholds.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=26