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

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 <[email protected]>
>>
>> __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 <[email protected]>
> 
> 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 <[email protected]>
> 

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