Re: [PATCH v3 0/4] mm/vmscan: fix swappiness=max and clean up per-node proactive reclaim
Ridong Chen <[email protected]>
| Newsgroups | gmane.linux.kernel,gmane.linux.kernel.mm |
|---|---|
| Message-ID | <[email protected]> |
On 8/14/2026 6:37 AM, Barry Song wrote: > On Tue, Jul 28, 2026 at 4:34 PM Ridong Chen <[email protected]> wrote: >> >> >> >> On 7/24/2026 7:12 PM, Barry Song wrote: >>> On Fri, Jul 24, 2026 at 10:43 AM Ridong Chen <[email protected]> wrote: >>>> >>>> >>>> >>>> On 7/24/2026 8:18 AM, Andrew Morton wrote: >>>>> On Thu, 23 Jul 2026 12:57:14 +0800 Ridong <[email protected]> wrote: >>>>> >>>>>> Fixes and one cleanup. >>>>>> >>>>>> Patch 1 fixes "swappiness=max": the anon-only test in get_scan_count() >>>>>> sat after the "cannot reclaim anon" check, so when no anon was >>>>>> reclaimable the request fell back to SCAN_FILE and evicted page cache >>>>>> instead. >>>>>> >>>>>> Patch 2 fixes reclaim_store() collapsing every error into -EAGAIN, so >>>>>> callers can no longer tell an invalid argument from a busy interface; >>>>>> propagate the real error code, matching the memcg path. >>>>>> >>>>>> Patch 3 drops the now-unused gfp_mask parameter from __node_reclaim(). >>>>>> >>>>>> Patch 4 fixes the same "swappiness=max" issue for MGLRU. >>>>> >>>>> Thanks. AI review might have found several pre-existing issues which >>>>> might interest the maintainers. Also a suggestion against your [4/4]: >>>>> >>>>> https://sashiko.dev/#/patchset/[email protected] >>>> >>>> Hi all, sashiko review: >>>> >>>> This is a pre-existing issue, but I noticed that per-node proactive reclaim >>>> fails to reclaim memory if the node's unmapped file or slab pages are below >>>> minimum thresholds, completely ignoring anonymous memory. >>>> In mm/vmscan.c:__node_reclaim(): >>>> if (node_pagecache_reclaimable(pgdat) > pgdat->min_unmapped_pages || >>>> node_page_state_pages(pgdat, NR_SLAB_RECLAIMABLE_B) > >>>> pgdat->min_slab_pages) { >>>> Does this condition incorrectly block proactive reclaim of anonymous memory >>>> by applying legacy zone-reclaim thresholds? >>>> It seems to bypass shrink_node() entirely if file/slab pages are low, even >>>> when sc->proactive is set, which could falsely return -EAGAIN to userspace >>>> for anon-heavy workloads. >>>> >>>> Should we add: >>>> >>>> if (sc->proactive || >>>> node_pagecache_reclaimable(pgdat) > pgdat->min_unmapped_pages || >>>> node_page_state_pages(pgdat, NR_SLAB_RECLAIMABLE_B) > pgdat->min_slab_pages) { >>> >> >> Hi Barry, sorry for the late reply. >> >>> Nop. >>> I assume reclaiming file cache and slab becomes problematic when their >>> amounts are already very limited, so we should still honor these two >>> checks. >>> >>> Maybe we could relax them only when swappiness == 201 >>> (SWAPPINESS_ANON_ONLY)? >>> >>> BTW, for global proactive reclaim, when setting swappiness to 201, does >>> it prevent slab shrinking? If not, it seems problematic when >>> node_page_state_pages(pgdat, NR_SLAB_RECLAIMABLE_B) < >>> pgdat->min_slab_pages. >>> >> >> in __node_reclaim, we will shrink the node when >> node_pagecache_reclaimable(pgdat) > pgdat->min_unmapped_pages, even if >> node_page_state_pages(pgdat, NR_SLAB_RECLAIMABLE_B) <= pgdat->min_slab_pages. >> This means slab shrinking can still occur even when below the limit (since >> shrink_slab is called unconditionally after shrink_lruvec). This is not an issue >> only for global proactive reclaim. >> >> >>> Your recent patchset prevents all file reclamation when swappiness is >>> set to 201, so we only need to check whether there could be a slab issue >>> before allowing shrink_node() to continue in this case. >>> >> >> So can we add just like? >> >> if ((sc->proactive && node_page_state_pages(pgdat, NR_SLAB_RECLAIMABLE_B) > >> pgdat->min_slab_pages) || >> node_pagecache_reclaimable(pgdat) > pgdat->min_unmapped_pages || >> node_page_state_pages(pgdat, NR_SLAB_RECLAIMABLE_B) > pgdat->min_slab_pages) { > > I feel both pgdat->min_unmapped_pages and > pgdat->min_slab_pages are quite broken in mainline. > > For example, even when the page cache is below > min_unmapped_pages, it may still be reclaimed. Similarly, slab may > still be reclaimed even when it is below min_slab_pages. > > Also, when both the page cache and slab are below their respective > thresholds, node_reclaim() may reclaim nothing even if we have > plenty of anon folios available. > > if (node_pagecache_reclaimable(pgdat) <= pgdat->min_unmapped_pages && > node_page_state_pages(pgdat, NR_SLAB_RECLAIMABLE_B) <= > pgdat->min_slab_pages) > return 0; > > For example, if slab > min_slab_pages but the page cache is below > min_unmapped_pages, we still reclaim file pages, even though the > comment says we should not. > > So we are not going to introduce another broken mechanism. > Maybe we should start by fixing the existing broken protection > against reclaiming slab and page cache? > For example, Maybe we can skip shrink_slab when node_page_state_pages(pgdat, NR_SLAB_RECLAIMABLE_B) <= pgdat->min_slab_pages? And similarly, in get_scan_count, we could avoid reclaiming file page cache if node_pagecache_reclaimable(pgdat) <= pgdat->min_unmapped_pages. -- Best regards Ridong