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

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?

Best Regards
Barry