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
