Re: [PATCH v3 0/4] mm/vmscan: fix swappiness=max and clean up per-node proactive reclaim

[Ridong Chen]

On 7/24/2026 7:12 PM, Barry Song wrote:
> On Fri, Jul 24, 2026 at 10:43 AM Ridong Chen <ridong.chen@xxxxxxxxx> wrote:
> > On 7/24/2026 8:18 AM, Andrew Morton wrote:
> > > On Thu, 23 Jul 2026 12:57:14 +0800 Ridong <ridong.chen@xxxxxxxxx> 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/20260723045718.2052070-1-ridong.chen@xxxxxxxxx
> >
> > 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) {

--
Best regards
Ridong