mm/vmscan: honour node reclaim limits per type

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]>

---
 mm/vmscan.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

mm/vmscan.c +0 -3

index 1e56973ceb73..5a3f67b3ba32 100644
@@ -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) {
-		/*

mm/vmscan.c +7 -0

+	 *
+	 * 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);

Quoted 16 lines · Ridong Chen

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.

Quoted 6 lines

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]>

Quoted 15 lines

---
  mm/vmscan.c | 20 ++++++++++----------
  1 file changed, 10 insertions(+), 10 deletions(-)

mm/vmscan.c +0 -3

index 1e56973ceb73..5a3f67b3ba32 100644
@@ -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) {
-		/*

mm/vmscan.c static unsigned long __node_reclaim(struct pglist_data *pgdat, · line 7911

Quoted 10 lines

mm/vmscan.c +3 -7

-		 * 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.

Quoted 10 lines

mm/vmscan.c +7 -0

+	 *
+	 * 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);

From: Ridong Chen <[email protected]>

min_unmapped_pages and min_slab_pages are documented as per-type limits, but node reclaim treats them as one combined gate: once either is exceeded, shrink_node() reclaims slab, file and anon together and pushes the other type below its limit. Per-node proactive reclaim reuses the same gate and fares worse -- with page cache and slab both under their limits it reclaims nothing and returns -EAGAIN on an anon-heavy node [1].

This series gates each type separately via two scan_control flags (skip_slab_reclaim, skip_file_reclaim) set only on the node reclaim path, drops the combined gate, and extends node_reclaim()'s early bail to check anon. The flags default to zero, so kswapd, direct, memcg and drop_caches are unaffected.

Tested on QEMU (x86_64, 2 NUMA nodes), A/B kernels differing only in this series.

Proactive reclaim (echo to node/reclaim) on an anon-heavy node, file and slab under their limits:

  metric              before   after
  -----------------   ------   -----------
  pages reclaimed     0 MiB    254 MiB anon
  return value        -EAGAIN  0

Node reclaim (zone_reclaim_mode) with one type under its limit -- the type under its limit must be left alone:

  type under limit    before          after
  -----------------   -------------   --------
  slab                645 scans       0 scans
  page cache          83 MiB scanned  0 MiB

[1] https://sashiko.dev/#/patchset/[email protected]

Ridong Chen (4):
mm/vmscan: only reclaim slab in node reclaim when over min_slab_pages mm/vmscan: only reclaim file pages in node reclaim when over
min_unmapped_pages
mm/vmscan: drop the combined limit gate in __node_reclaim() mm/vmscan: do not skip node reclaim when only anon is reclaimable

 mm/vmscan.c | 89 ++++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 67 insertions(+), 22 deletions(-)

Signature · 1 line

2.34.1

You are explaining what but missing the most important part _Why_ do we need to have this addressed? Is this just addressing Sashiko review refernced below? Is there any real usecase where the current behavior matters?

+cc Roman for suggestion.

Quoted 4 lines · Michal Hocko

You are explaining what but missing the most important part _Why_ do we need to have this addressed? Is this just addressing Sashiko review refernced below? Is there any real usecase where the current behavior matters?

This is exactly the issue with these 'unrelated to your patch but' suggestions from sashiko.

You end up in loops:

AI generated patch ---------------> AI generated review
^ |
| |
| v
AI generated 'unrelated to your patch but'

And _at every stage_ reviewers have to do _additional work_ (with ~50% signal/noise).

This isn't sustainable.

We already had _too much work_ prior to the slopgeddon. Now we have a multiple of that.

Roman - I really think we a way of switching off the 'unrelated to your patch but' stuff per-subsystem would be useful.

Maybe we could figure out a way of funnelling this stuff somewhere separately longer term.

(I have I think 2 slopped fixes to rewrite after the previous what like 7 or 8 this cycle? So forgive the grumpiness :)

Quoted 29 lines · Lorenzo Stoakes (ARM)

+cc Roman for suggestion.

Quoted 4 lines · Michal Hocko

You are explaining what but missing the most important part _Why_ do we need to have this addressed? Is this just addressing Sashiko review refernced below? Is there any real usecase where the current behavior matters?

This is exactly the issue with these 'unrelated to your patch but' suggestions from sashiko.

You end up in loops:

AI generated patch ---------------> AI generated review
^ |
| |
| v
AI generated 'unrelated to your patch but'

And _at every stage_ reviewers have to do _additional work_ (with ~50% signal/noise).

This isn't sustainable.

We already had _too much work_ prior to the slopgeddon. Now we have a multiple of that.

Roman - I really think we a way of switching off the 'unrelated to your patch but' stuff per-subsystem would be useful.

Quoted 5 lines

Maybe we could figure out a way of funnelling this stuff somewhere separately longer term.

(I have I think 2 slopped fixes to rewrite after the previous what like 7 or 8 this cycle? So forgive the grumpiness :)

I wouldn't blame Sashiko on this really. Yes it points to a theoretical problem. That is fine. But we should encourage people to not blindly

follow that lead and immediately jump at fixing something that is not a real problem. Quite honestly I even haven't looked into patches until it is clear that the usecase is sound. We should enforce this more and leave patches lingering if they are not sufficiently justified.

--
Michal Hocko
SUSE Labs

Quoted 33 lines · Michal Hocko

Quoted 32 lines · Lorenzo Stoakes (ARM)

+cc Roman for suggestion.

Quoted 4 lines · Michal Hocko

You are explaining what but missing the most important part _Why_ do we need to have this addressed? Is this just addressing Sashiko review refernced below? Is there any real usecase where the current behavior matters?

This is exactly the issue with these 'unrelated to your patch but' suggestions from sashiko.

You end up in loops:

AI generated patch ---------------> AI generated review
^ |
| |
| v
AI generated 'unrelated to your patch but'

And _at every stage_ reviewers have to do _additional work_ (with ~50% signal/noise).

This isn't sustainable.

We already had _too much work_ prior to the slopgeddon. Now we have a multiple of that.

Roman - I really think we a way of switching off the 'unrelated to your patch but' stuff per-subsystem would be useful.

Maybe we could figure out a way of funnelling this stuff somewhere separately longer term.

Quoted 5 lines

Quoted 2 lines

(I have I think 2 slopped fixes to rewrite after the previous what like 7 or 8 this cycle? So forgive the grumpiness :)

I wouldn't blame Sashiko on this really. Yes it points to a theoretical problem. That is fine. But we should encourage people to not blindly

I mean it's not only this case, it's a pattern I've been observing for a while.

Quoted 4 lines

follow that lead and immediately jump at fixing something that is not a real problem. Quite honestly I even haven't looked into patches until it is clear that the usecase is sound. We should enforce this more and leave patches lingering if they are not sufficiently justified.

Yes this is a needed change in mm, but until we fully transition workflow any patch might still land.

And I still find those kinds of suggestions deeply problematic for reviewer workload.

If you had a person repeatedly say 'hey unrelated to this series but...' you'd very quickly ask them to stop and if they persisted, >/dev/null them.

I get that passive passes are too expensive and it's not that much more work to have sashiko point this stuff out, but it's not that much more work for _it_, it's substantially increasing workload for reviewers.

As I said on a recent call - it's fine as long as you don't care about reviewer/maintainer burnout.

But I do so :)

Hey, do not get me wrong. I very much care about reviewers as well. I merely wanted to say that we've had peaks of tool driven patches no matter what. Things have eventually normalized but people need to be educated about expectations of maintainers. Sashiko suggesting "but this might need attention as well..." certainly contribute to more noise but keep in mind that AI driven people are going to find a tool of their choice to arm them with ideas to implement and post so focusing on Sashiko is not going to help all that much. My main message is to establish and enforce a notion that patches are going to be ignored if they are not justified properly (even if they might seem technically correct or fixing a theoretical problem).

--
Michal Hocko
SUSE Labs

+cc Roman for suggestion.

Quoted 4 lines · Michal Hocko

You are explaining what but missing the most important part _Why_ do we need to have this addressed? Is this just addressing Sashiko review refernced below? Is there any real usecase where the current behavior matters?

This is exactly the issue with these 'unrelated to your patch but' suggestions from sashiko.

You end up in loops:

AI generated patch ---------------> AI generated review
^ |
| |
| v
AI generated 'unrelated to your patch but'

And _at every stage_ reviewers have to do _additional work_ (with ~50% signal/noise).

This isn't sustainable.

We already had _too much work_ prior to the slopgeddon. Now we have a multiple of that.

Roman - I really think we a way of switching off the 'unrelated to your patch but' stuff per-subsystem would be useful.

Maybe we could figure out a way of funnelling this stuff somewhere separately longer term.

(I have I think 2 slopped fixes to rewrite after the previous what like 7 or 8 this cycle? So forgive the grumpiness :)

Quoted 1 line · Lorenzo Stoakes (ARM)

On Fri, Aug 21, 2026 at 10:31:06AM +0200, Michal Hocko wrote:

Quoted 4 lines

of that.

Roman - I really think we a way of switching off the 'unrelated to your patch but' stuff per-subsystem would be useful.

I thought about it, but switching it off completely looks dubious to me. So maybe moderating it a bit can help? Like not reporting completely unrelated issues or at least do not report them more than once?

Quoted 2 lines · Roman Gushchin

Quoted 1 line · Lorenzo Stoakes (ARM)

On Fri, Aug 21, 2026 at 10:31:06AM +0200, Michal Hocko wrote:

...>> We already had _too much work_ prior to the slopgeddon. Now we have a multiple

Quoted 8 lines

Quoted 4 lines

of that.

Roman - I really think we a way of switching off the 'unrelated to your patch but' stuff per-subsystem would be useful.

I thought about it, but switching it off completely looks dubious to me. So maybe moderating it a bit can help? Like not reporting completely unrelated issues or at least do not report them more than once?

Or, reporting them *once*, and not as a reply to the patch!

After all, it's "unrelated to your patch". So then, don't pollute that patch's review thread, right?

That would alleviate the most infuriating part of this cycle, imho.

"Lorenzo Stoakes (ARM)" <[email protected]> writes:

Quoted 2 lines

Maybe we could figure out a way of funnelling this stuff somewhere separately longer term.

Yes, there will be a database available to maintainers soon.

+cc Roman for suggestion.

Quoted 4 lines · Michal Hocko

You are explaining what but missing the most important part _Why_ do we need to have this addressed? Is this just addressing Sashiko review refernced below? Is there any real usecase where the current behavior matters?

This is exactly the issue with these 'unrelated to your patch but' suggestions from sashiko.

You end up in loops:

AI generated patch ---------------> AI generated review
^ |
| |
| v
AI generated 'unrelated to your patch but'

And _at every stage_ reviewers have to do _additional work_ (with ~50% signal/noise).

Quoted 9 lines · Ridong Chen

From: Ridong Chen <[email protected]>

min_unmapped_pages and min_slab_pages are documented as per-type limits, but node reclaim treats them as one combined gate: once either is exceeded, shrink_node() reclaims slab, file and anon together and pushes the other type below its limit. Per-node proactive reclaim reuses the same gate and fares worse -- with page cache and slab both under their limits it reclaims nothing and returns -EAGAIN on an anon-heavy node [1].

Quoted 5 lines

This series gates each type separately via two scan_control flags (skip_slab_reclaim, skip_file_reclaim) set only on the node reclaim path, drops the combined gate, and extends node_reclaim()'s early bail to check anon. The flags default to zero, so kswapd, direct, memcg and drop_caches are unaffected.

You are explaining what but missing the most important part _Why_ do we need to have this addressed? Is this just addressing Sashiko review refernced below? Is there any real usecase where the current behavior matters?

Quoted 33 lines

Tested on QEMU (x86_64, 2 NUMA nodes), A/B kernels differing only in this series.

Proactive reclaim (echo to node/reclaim) on an anon-heavy node, file and slab under their limits:

   metric              before   after
   -----------------   ------   -----------
   pages reclaimed     0 MiB    254 MiB anon
   return value        -EAGAIN  0

Node reclaim (zone_reclaim_mode) with one type under its limit -- the type under its limit must be left alone:

   type under limit    before          after
   -----------------   -------------   --------
   slab                645 scans       0 scans
   page cache          83 MiB scanned  0 MiB

[1] https://sashiko.dev/#/patchset/[email protected]

Ridong Chen (4):
mm/vmscan: only reclaim slab in node reclaim when over min_slab_pages mm/vmscan: only reclaim file pages in node reclaim when over
min_unmapped_pages
mm/vmscan: drop the combined limit gate in __node_reclaim() mm/vmscan: do not skip node reclaim when only anon is reclaimable

  mm/vmscan.c | 89 ++++++++++++++++++++++++++++++++++++++++-------------
  1 file changed, 67 insertions(+), 22 deletions(-)

Signature · 1 line

2.34.1

Quoted 16 lines · Michal Hocko

Quoted 9 lines · Ridong Chen

From: Ridong Chen <[email protected]>

min_unmapped_pages and min_slab_pages are documented as per-type limits, but node reclaim treats them as one combined gate: once either is exceeded, shrink_node() reclaims slab, file and anon together and pushes the other type below its limit. Per-node proactive reclaim reuses the same gate and fares worse -- with page cache and slab both under their limits it reclaims nothing and returns -EAGAIN on an anon-heavy node [1].

Quoted 5 lines

This series gates each type separately via two scan_control flags (skip_slab_reclaim, skip_file_reclaim) set only on the node reclaim path, drops the combined gate, and extends node_reclaim()'s early bail to check anon. The flags default to zero, so kswapd, direct, memcg and drop_caches are unaffected.

Quoted 5 lines

You are explaining what but missing the most important part _Why_ do we need to have this addressed? Is this just addressing Sashiko review refernced below? Is there any real usecase where the current behavior matters?

Hi Michal,

Thank you for your reply. I should have made the background much clearer.

Yes, the original issue comes from Sashiko's review. Sashiko found that proactive reclaim fails to reclaim memory when the node's unmapped file or slab pages are below the minimum thresholds, even though there is plenty of anonymous memory available.

After further discussion, we realized that min_unmapped_pages and min_slab_pages may not be used correctly. Apart from the issue above, there are other problems as mentioned by Barry in [2]:

Even when page cache is below min_unmapped_pages, it may still be reclaimed as long as slab is sufficient. Similarly, slab may still be reclaimed even when it is below min_slab_pages.

node_reclaim() cannot reclaim anonymous pages if both page cache and slab are below their respective thresholds, even when there is plenty of anonymous memory available.

To address these issues, I am sending this series to facilitate discussion. Your feedback would be greatly appreciated.

Quoted 46 lines · Ridong Chen

Quoted 16 lines · Michal Hocko

Quoted 9 lines · Ridong Chen

From: Ridong Chen <[email protected]>

min_unmapped_pages and min_slab_pages are documented as per-type limits, but node reclaim treats them as one combined gate: once either is exceeded, shrink_node() reclaims slab, file and anon together and pushes the other type below its limit. Per-node proactive reclaim reuses the same gate and fares worse -- with page cache and slab both under their limits it reclaims nothing and returns -EAGAIN on an anon-heavy node [1].

Quoted 5 lines

This series gates each type separately via two scan_control flags (skip_slab_reclaim, skip_file_reclaim) set only on the node reclaim path, drops the combined gate, and extends node_reclaim()'s early bail to check anon. The flags default to zero, so kswapd, direct, memcg and drop_caches are unaffected.

Quoted 5 lines

You are explaining what but missing the most important part _Why_ do we need to have this addressed? Is this just addressing Sashiko review refernced below? Is there any real usecase where the current behavior matters?

Hi Michal,

Thank you for your reply. I should have made the background much clearer.

Yes, the original issue comes from Sashiko's review. Sashiko found that proactive reclaim fails to reclaim memory when the node's unmapped file or slab pages are below the minimum thresholds, even though there is plenty of anonymous memory available.

After further discussion, we realized that min_unmapped_pages and min_slab_pages may not be used correctly. Apart from the issue above, there are other problems as mentioned by Barry in [2]:

Even when page cache is below min_unmapped_pages, it may still be reclaimed as long as slab is sufficient. Similarly, slab may still be reclaimed even when it is below min_slab_pages.

node_reclaim() cannot reclaim anonymous pages if both page cache and slab are below their respective thresholds, even when there is plenty of anonymous memory available.

Quoted 2 lines

To address these issues, I am sending this series to facilitate discussion. Your feedback would be greatly appreciated.

Those interfaces are relicts from the distant past same as the node reclaim. I wouldn't bother fixing those unless there is a real usecase. Pro-active per node reclaim is a different thing and we should probably divorce it from those min_$foo counters altogether (if they are not yet).

Same as checkpatch.pl, shashiko is giving you hints and you shouldn't simply follow them without a deeper considerations. Consider that there is review capacity required for any patch posted. We do not want to waste that scarce resource.

Quoted 47 lines · Michal Hocko

Quoted 46 lines · Ridong Chen

Quoted 16 lines · Michal Hocko

Quoted 9 lines · Ridong Chen

From: Ridong Chen <[email protected]>

min_unmapped_pages and min_slab_pages are documented as per-type limits, but node reclaim treats them as one combined gate: once either is exceeded, shrink_node() reclaims slab, file and anon together and pushes the other type below its limit. Per-node proactive reclaim reuses the same gate and fares worse -- with page cache and slab both under their limits it reclaims nothing and returns -EAGAIN on an anon-heavy node [1].

Quoted 5 lines

This series gates each type separately via two scan_control flags (skip_slab_reclaim, skip_file_reclaim) set only on the node reclaim path, drops the combined gate, and extends node_reclaim()'s early bail to check anon. The flags default to zero, so kswapd, direct, memcg and drop_caches are unaffected.

Quoted 5 lines

You are explaining what but missing the most important part _Why_ do we need to have this addressed? Is this just addressing Sashiko review refernced below? Is there any real usecase where the current behavior matters?

Hi Michal,

Thank you for your reply. I should have made the background much clearer.

Yes, the original issue comes from Sashiko's review. Sashiko found that proactive reclaim fails to reclaim memory when the node's unmapped file or slab pages are below the minimum thresholds, even though there is plenty of anonymous memory available.

After further discussion, we realized that min_unmapped_pages and min_slab_pages may not be used correctly. Apart from the issue above, there are other problems as mentioned by Barry in [2]:

Even when page cache is below min_unmapped_pages, it may still be reclaimed as long as slab is sufficient. Similarly, slab may still be reclaimed even when it is below min_slab_pages.

node_reclaim() cannot reclaim anonymous pages if both page cache and slab are below their respective thresholds, even when there is plenty of anonymous memory available.

Quoted 9 lines

Quoted 2 lines

To address these issues, I am sending this series to facilitate discussion. Your feedback would be greatly appreciated.

Those interfaces are relicts from the distant past same as the node reclaim. I wouldn't bother fixing those unless there is a real usecase. Pro-active per node reclaim is a different thing and we should probably divorce it from those min_$foo counters altogether (if they are not yet).

Yeah, proactive per-node reclaim currently does not divorce from those min_$foo counters.

Did you mean that min_slab_pages and min_unmapped_pages should influence proactive per-node reclaim?

Quoted 60 lines · Ridong Chen

Quoted 56 lines · Michal Hocko

Quoted 42 lines · Ridong Chen

Quoted 16 lines · Michal Hocko

Quoted 9 lines · Ridong Chen

From: Ridong Chen <[email protected]>

min_unmapped_pages and min_slab_pages are documented as per-type limits, but node reclaim treats them as one combined gate: once either is exceeded, shrink_node() reclaims slab, file and anon together and pushes the other type below its limit. Per-node proactive reclaim reuses the same gate and fares worse -- with page cache and slab both under their limits it reclaims nothing and returns -EAGAIN on an anon-heavy node [1].

Quoted 5 lines

This series gates each type separately via two scan_control flags (skip_slab_reclaim, skip_file_reclaim) set only on the node reclaim path, drops the combined gate, and extends node_reclaim()'s early bail to check anon. The flags default to zero, so kswapd, direct, memcg and drop_caches are unaffected.

Quoted 5 lines

You are explaining what but missing the most important part _Why_ do we need to have this addressed? Is this just addressing Sashiko review refernced below? Is there any real usecase where the current behavior matters?

Hi Michal,

Thank you for your reply. I should have made the background much clearer.

Yes, the original issue comes from Sashiko's review. Sashiko found that proactive reclaim fails to reclaim memory when the node's unmapped file or slab pages are below the minimum thresholds, even though there is plenty of anonymous memory available.

After further discussion, we realized that min_unmapped_pages and min_slab_pages may not be used correctly. Apart from the issue above, there are other problems as mentioned by Barry in [2]:

Even when page cache is below min_unmapped_pages, it may still be reclaimed as long as slab is sufficient. Similarly, slab may still be reclaimed even when it is below min_slab_pages.

Quoted 6 lines

node_reclaim() cannot reclaim anonymous pages if both page cache and slab are below their respective thresholds, even when there is plenty of anonymous memory available.

To address these issues, I am sending this series to facilitate discussion. Your feedback would be greatly appreciated.

Those interfaces are relicts from the distant past same as the node reclaim. I wouldn't bother fixing those unless there is a real usecase. Pro-active per node reclaim is a different thing and we should probably divorce it from those min_$foo counters altogether (if they are not yet).

Quoted 5 lines

Yeah, proactive per-node reclaim currently does not divorce from those min_$foo counters.

Did you mean that min_slab_pages and min_unmapped_pages should influence proactive per-node reclaim?

Nope, exactly opposite

Quoted 5 lines

Quoted 2 lines

Did you mean that min_slab_pages and min_unmapped_pages should influence proactive per-node reclaim?

Nope, exactly opposite

I would really appreciate it if you could clarify this further. Sorry, I'm not sure I fully understand what you meant.

Quoted 3 lines · Ridong Chen

Quoted 3 lines

Quoted 2 lines

Did you mean that min_slab_pages and min_unmapped_pages should influence proactive per-node reclaim?

Quoted 5 lines

Quoted 2 lines

Nope, exactly opposite

I would really appreciate it if you could clarify this further. Sorry, I'm not sure I fully understand what you meant.

pro-active (userspace triggered) node reclaim should completely ignore all those historical node_reclaim tunables.

Quoted 4 lines · Michal Hocko

Quoted 3 lines · Ridong Chen

Quoted 3 lines

Quoted 2 lines

Did you mean that min_slab_pages and min_unmapped_pages should influence proactive per-node reclaim?

I apologize for being incoherent in my last email, it was after a full day of work. What I wanted to say is that min_slab_pages and min_unmapped_pages should *not* influence proactive per-node reclaim.

Quoted 8 lines

Quoted 5 lines

Quoted 2 lines

Nope, exactly opposite

I would really appreciate it if you could clarify this further. Sorry, I'm not sure I fully understand what you meant.

pro-active (userspace triggered) node reclaim should completely ignore all those historical node_reclaim tunables.

Yeah, this matches my understanding. Apologies again for the confusing reply.

Since Johannes has acked patch 3, perhaps we can simply remove the conditional check in __node_reclaim and keep the shrink loop as follows:

```

(quoted diff) +10 -10

@@ -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.
+	 *
+	 * 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);

Quoted 53 lines · Ridong Chen

Quoted 4 lines · Michal Hocko

Quoted 3 lines · Ridong Chen

Quoted 3 lines

Quoted 2 lines

Did you mean that min_slab_pages and min_unmapped_pages should influence proactive per-node reclaim?

I apologize for being incoherent in my last email, it was after a full day of work. What I wanted to say is that min_slab_pages and min_unmapped_pages should *not* influence proactive per-node reclaim.

Quoted 8 lines

Quoted 5 lines

Quoted 2 lines

Nope, exactly opposite

I would really appreciate it if you could clarify this further. Sorry, I'm not sure I fully understand what you meant.

pro-active (userspace triggered) node reclaim should completely ignore all those historical node_reclaim tunables.

Yeah, this matches my understanding. Apologies again for the confusing reply.

Since Johannes has acked patch 3, perhaps we can simply remove the conditional check in __node_reclaim and keep the shrink loop as follows:

```

(quoted diff) +10 -10

@@ -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.
+	 *
+	 * 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);
 

Quoted 2 lines

(quoted diff) +0 -0

  	set_task_reclaim_state(p, NULL);

  	memalloc_noreclaim_restore(noreclaim_flag);

Could you post a stand alone patch with the full description please?