Re: [RFC PATCH 3/4] mm/vmscan: drop the combined limit gate in __node_reclaim()

[Ridong Chen]

On 8/21/2026 8:49 PM, Johannes Weiner wrote:
> On Fri, Aug 21, 2026 at 04:17:40PM +0800, Ridong Chen wrote:
> > From: Ridong Chen <chenridong@xxxxxxxxxx>
> >
> > __node_reclaim() only ran shrink_node() when unmapped page cache was
> > over min_unmapped_pages OR reclaimable slab was over min_slab_pages.
> >
> > With slab and file reclaim now gated per type by sc->skip_slab_reclaim and
> > sc->skip_file_reclaim, this combined gate is either redundant or harmful:
> >
> >   - for the NUMA node reclaim caller it is always true, since
> >     node_reclaim() only calls in when at least one limit is exceeded;
> >
> >   - for the per-node proactive reclaim caller (which does not go through
> >     node_reclaim()'s checks) it wrongly suppressed all reclaim -- anon
> >     included -- whenever both page cache and slab happened to sit at or
> >     below their limits, even with plenty of reclaimable anon present.
> >
> > Drop the gate and let the per-type flags decide what to reclaim.  The
> > node reclaim path is unchanged; the proactive path can now reclaim anon
> > as requested.
> >
> > Assisted-by: Claude:claude-opus-4-8
> > Signed-off-by: Ridong Chen <chenridong@xxxxxxxxxx>
>
> Could we go with this patch and table the rest of the series?
>
> These minimums are specifically for zone_reclaim_mode. I don't know
> who is using that at this point, and it seems the behavior has been
> like that for a while with no practical complaints. So my take is,
> leave it until somebody has a real problem.
>
> Applying those limits to proactive reclaim, on the other hand, wasn't
> intentional, isn't documented, and can lead to unexpected behavior -
> considering we have non-zero default values on these knobs.
>
> Since the gate was already redundant for some reason, leaving it in
> node_reclaim() (zone_reclaim_mode) and killing it in __node_reclaim()
> (the path shared with proactive reclaim), like you did here, sounds
> like the best way forward for now to me.
>
> With an updated changelog to that end,
>
> Acked-by: Johannes Weiner <hannes@xxxxxxxxxxx>

Thanks. Will update.

> > ---
> >   mm/vmscan.c | 20 ++++++++++----------
> >   1 file changed, 10 insertions(+), 10 deletions(-)
> >
> > diff --git a/mm/vmscan.c b/mm/vmscan.c
> > index 1e56973ceb73..5a3f67b3ba32 100644
> > --- a/mm/vmscan.c
> > +++ b/mm/vmscan.c
> > @@ -7906,16 +7906,16 @@ static unsigned long __node_reclaim(struct pglist_data *pgdat,
> >   	noreclaim_flag = memalloc_noreclaim_save();
> >   	set_task_reclaim_state(p, &sc->reclaim_state);
> >   
> > -	if (node_pagecache_reclaimable(pgdat) > pgdat->min_unmapped_pages ||
> > -	    node_page_state_pages(pgdat, NR_SLAB_RECLAIMABLE_B) > pgdat->min_slab_pages) {
> > -		/*
> > -		 * Free memory by calling shrink node with increasing
> > -		 * priorities until we have enough memory freed.
> > -		 */
> > -		do {
> > -			shrink_node(pgdat, sc);
> > -		} while (sc->nr_reclaimed < nr_pages && --sc->priority >= 0);
> > -	}
> > +	/*
> > +	 * Free memory by calling shrink node with increasing
> > +	 * priorities until we have enough memory freed.
>
> Might as well drop this comment. It just says what the code does.
>
> > +	 *
> > +	 * What to reclaim is gated per type by sc->skip_slab_reclaim and
> > +	 * sc->skip_file_reclaim.
> > +	 */
> > +	do {
> > +		shrink_node(pgdat, sc);
> > +	} while (sc->nr_reclaimed < nr_pages && --sc->priority >= 0);
> >   
> >   	set_task_reclaim_state(p, NULL);
> >   	memalloc_noreclaim_restore(noreclaim_flag);

--
Best regards
Ridong