mm/vmscan: fix swappiness=max and clean up per-node proactive reclaim
Subsystems (3)
cover mm/vmscan: fix swappiness=max and clean up per-node proactive reclaim
From: Ridong Chen <[email protected]>
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.
---
v3 -> v4:
- Patch 4: Update comments and commit message.
- Patch 4: Adopted Sashiko's suggestion to use WARN_ON_ONCE.
v2 -> v3:
- Update comments and commit message.
v1 -> v2:
- Collected review tags for patches 1-3
- Added patch 4 to fix the same issue in the MGLRU path.
Ridong Chen (4):
mm/vmscan: fix anon-only reclaim evicting file pages when
swappiness=max
mm: vmscan: propagate real error code from per-node proactive reclaim
mm: vmscan: drop unused gfp_mask parameter from __node_reclaim() mm/mglru: fix anon-only reclaim evicting file pages when
swappiness=max
mm/vmscan.c | 55 +++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 41 insertions(+), 14 deletions(-)
Signature · 1 line
2.34.1
PATCH 1/4 mm/vmscan: fix anon-only reclaim evicting file pages when swappiness=max
From: Ridong Chen <[email protected]>
As Qi mentioned [1], when swappiness=max (SWAPPINESS_ANON_ONLY) is set, the reclaim logic is expected to reclaim anonymous pages exclusively. However, due to the current ordering of checks in get_scan_count(), file pages may still be evicted if can_reclaim_anon_pages() returns false, which contradicts the semantics of SWAPPINESS_ANON_ONLY.
Reproducer in a cgroup holding 64M of file cache, with no swap configured:
Before (file cache is wrongly evicted):
# cat memory.stat
anon 196608
file 67178496
pgscan_proactive 0
# echo "64M swappiness=max" > memory.reclaim
# cat memory.stat
anon 208896
file 4096 <- page cache evicted
pgsteal_proactive 16400
pgscan_proactive 16400After (file cache is left intact):
# cat memory.stat
anon 200704
file 67178496
pgscan_proactive 0
# echo "64M swappiness=max" > memory.reclaim
-bash: echo: write error: Resource temporarily unavailable
# cat memory.stat
anon 208896
file 67178496 <- page cache untouched
pgsteal_proactive 0
pgscan_proactive 0Fix this by bailing out early when SWAPPINESS_ANON_ONLY is set and no anonymous pages are reclaimable, before falling back to file reclaim.
[1] https://lore.kernel.org/cgroups/[email protected]/
--- mm/vmscan.c | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-)
mm/vmscan.c +17 -7
@@ -2501,6 +2501,23 @@ static void get_scan_count(struct lruvec *lruvec, struct scan_control *sc,enum scan_balance scan_balance;enum lru_list lru; +/* +* Proactive reclaim initiated by userspace for anonymous memory only. +* SWAPPINESS_ANON_ONLY is set only on the proactive reclaim path, so +* warn if it shows up elsewhere. When anon cannot be reclaimed (e.g. +* no swap), bail out instead of falling back to evicting file pages, +* which would violate the anon-only semantics. +*/ +if (swappiness ==SWAPPINESS_ANON_ONLY ) { +WARN_ON_ONCE (!sc->proactive); +if (!can_reclaim_anon_pages (memcg, pgdat->node_id, sc)) { +memset (nr,0 ,sizeof (*nr) *NR_LRU_LISTS ); +return ; + } + scan_balance =SCAN_ANON ; + goto out; + } +/* If we have no swap space, do not bother scanning anon folios. */ if (!sc->may_swap || !can_reclaim_anon_pages (memcg, pgdat->node_id, sc)) { scan_balance =SCAN_FILE ;
@@ -2519,13 +2536,6 @@ static void get_scan_count(struct lruvec *lruvec, struct scan_control *sc, goto out; } -/* Proactive reclaim initiated by userspace for anonymous memory only */ -if (swappiness ==SWAPPINESS_ANON_ONLY ) { -WARN_ON_ONCE (!sc->proactive); - scan_balance =SCAN_ANON ; - goto out; - } -/* * Do not apply any pressure balancing cleverness when the * system is close to OOM, scan both anon and file equally
Signature · 1 line
2.34.1
Re: [PATCH -v4 1/4] mm/vmscan: fix anon-only reclaim evicting file pages when swappiness=max
Quoted 7 lines · Ridong
From: Ridong Chen <[email protected]>
As Qi mentioned [1], when swappiness=max (
SWAPPINESS_ANON_ONLY) is set, the reclaim logic is expected to reclaim anonymous pages exclusively. However, due to the current ordering of checks inget_scan_count(), file pages may still be evicted ifcan_reclaim_anon_pages()returns false, which contradicts the semantics ofSWAPPINESS_ANON_ONLY.
But if can_reclaim_anon_pages tells us that anonymous pages are not reclaimable then it makes no sense to try that, no? What do you expect to see? An OOM killer?
Signature · 2 lines
Michal Hocko SUSE Labs
Barry Song
reviewerRe: [PATCH -v4 1/4] mm/vmscan: fix anon-only reclaim evicting file pages when swappiness=max
Quoted 10 lines · Michal Hocko
Quoted 7 lines · Ridong
From: Ridong Chen <[email protected]>
As Qi mentioned [1], when swappiness=max (
SWAPPINESS_ANON_ONLY) is set, the reclaim logic is expected to reclaim anonymous pages exclusively. However, due to the current ordering of checks inget_scan_count(), file pages may still be evicted ifcan_reclaim_anon_pages()returns false, which contradicts the semantics ofSWAPPINESS_ANON_ONLY.
Quoted 3 lines
But if
can_reclaim_anon_pagestells us that anonymous pages are not reclaimable then it makes no sense to try that, no? What do you expect to see? An OOM killer?
This is proactive reclaim rather than direct reclaim, and SWAPPINESS_ANON_ONLY is only supported for proactive reclaim. It therefore cannot invoke the OOM killer.
If I understand correctly, this is exactly the issue this patch is trying to address. Instead of falling back to reclaiming file folios, it returns "bash: echo: write error: Resource temporarily unavailable".
After (file cache is left intact):
# cat memory.stat
anon 200704
file 67178496
pgscan_proactive 0
# echo "64M swappiness=max" > memory.reclaim
-bash: echo: write error: Resource temporarily unavailable
# cat memory.stat
anon 208896
file 67178496 <- page cache untouched
pgsteal_proactive 0
pgscan_proactive 0Ridong, please correct me if I am wrong.
Re: [PATCH -v4 1/4] mm/vmscan: fix anon-only reclaim evicting file pages when swappiness=max
Quoted 15 lines · Barry Song
Quoted 10 lines · Michal Hocko
Quoted 7 lines · Ridong
From: Ridong Chen <[email protected]>
As Qi mentioned [1], when swappiness=max (
SWAPPINESS_ANON_ONLY) is set, the reclaim logic is expected to reclaim anonymous pages exclusively. However, due to the current ordering of checks inget_scan_count(), file pages may still be evicted ifcan_reclaim_anon_pages()returns false, which contradicts the semantics ofSWAPPINESS_ANON_ONLY.Quoted 3 lines
But if
can_reclaim_anon_pagestells us that anonymous pages are not reclaimable then it makes no sense to try that, no? What do you expect to see? An OOM killer?
Quoted 7 lines
This is proactive reclaim rather than direct reclaim, and
SWAPPINESS_ANON_ONLYis only supported for proactive reclaim. It therefore cannot invoke the OOM killer.If I understand correctly, this is exactly the issue this patch is trying to address. Instead of falling back to reclaiming file folios, it returns "bash: echo: write error: Resource temporarily unavailable".
My bad. I have misread the changelog and even missed the WARN_ON for !pr-oactive reclaim path.
Failing pro-active makes sense.
Signature · 2 lines
Michal Hocko SUSE Labs
Re: [PATCH -v4 1/4] mm/vmscan: fix anon-only reclaim evicting file pages when swappiness=max
In reply to Barry SongQuoted 36 lines · Barry Song
Quoted 10 lines · Michal Hocko
Quoted 7 lines · Ridong
From: Ridong Chen <[email protected]>
As Qi mentioned [1], when swappiness=max (
SWAPPINESS_ANON_ONLY) is set, the reclaim logic is expected to reclaim anonymous pages exclusively. However, due to the current ordering of checks inget_scan_count(), file pages may still be evicted ifcan_reclaim_anon_pages()returns false, which contradicts the semantics ofSWAPPINESS_ANON_ONLY.Quoted 3 lines
But if
can_reclaim_anon_pagestells us that anonymous pages are not reclaimable then it makes no sense to try that, no? What do you expect to see? An OOM killer?This is proactive reclaim rather than direct reclaim, and
SWAPPINESS_ANON_ONLYis only supported for proactive reclaim. It therefore cannot invoke the OOM killer.If I understand correctly, this is exactly the issue this patch is trying to address. Instead of falling back to reclaiming file folios, it returns "bash: echo: write error: Resource temporarily unavailable".
After (file cache is left intact):
# cat memory.stat anon 200704 file 67178496 pgscan_proactive 0 # echo "64M swappiness=max" > memory.reclaim -bash: echo: write error: Resource temporarily unavailable # cat memory.stat anon 208896 file 67178496 <- page cache untouched pgsteal_proactive 0 pgscan_proactive 0
Quoted 1 line
Ridong, please correct me if I am wrong.
Thank you, Barry, it's correct.
Signature · 2 lines
Best regards Ridong
Re: [PATCH -v4 1/4] mm/vmscan: fix anon-only reclaim evicting file pages when swappiness=max
In reply to RidongQuoted 65 lines · Ridong
From: Ridong Chen <[email protected]>
As Qi mentioned [1], when swappiness=max (
SWAPPINESS_ANON_ONLY) is set, the reclaim logic is expected to reclaim anonymous pages exclusively. However, due to the current ordering of checks inget_scan_count(), file pages may still be evicted ifcan_reclaim_anon_pages()returns false, which contradicts the semantics ofSWAPPINESS_ANON_ONLY.Reproducer in a cgroup holding 64M of file cache, with no swap configured:
Before (file cache is wrongly evicted):
# cat memory.stat anon 196608 file 67178496 pgscan_proactive 0 # echo "64M swappiness=max" > memory.reclaim # cat memory.stat anon 208896 file 4096 <- page cache evicted pgsteal_proactive 16400 pgscan_proactive 16400After (file cache is left intact):
# cat memory.stat anon 200704 file 67178496 pgscan_proactive 0 # echo "64M swappiness=max" > memory.reclaim -bash: echo: write error: Resource temporarily unavailable # cat memory.stat anon 208896 file 67178496 <- page cache untouched pgsteal_proactive 0 pgscan_proactive 0Fix this by bailing out early when
SWAPPINESS_ANON_ONLYis set and no anonymous pages are reclaimable, before falling back to file reclaim.[1] https://lore.kernel.org/cgroups/[email protected]/
Fixes 68a1436bde00 ("mm: add swappiness=max arg to memory.reclaim for only anon reclaim")Suggested-by Qi Zheng <[email protected]>Acked-by Shakeel Butt <[email protected]>Acked-by Johannes Weiner <[email protected]>Reviewed-by Muchun Song <[email protected]>Reviewed-by Qi Zheng <[email protected]>Reviewed-by Barry Song <[email protected]>Signed-off-by Ridong Chen <[email protected]>--- mm/vmscan.c | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-)mm/vmscan.c +5
-0@@ -2501,6 +2501,23 @@ static void get_scan_count(struct lruvec *lruvec, struct scan_control *sc,enum scan_balance scan_balance;enum lru_list lru; +/* +* Proactive reclaim initiated by userspace for anonymous memory only. +* SWAPPINESS_ANON_ONLY is set only on the proactive reclaim path, so +* warn if it shows up elsewhere. When anon cannot be reclaimed (e.g. +* no swap), bail out instead of falling back to evicting file pages,
mm/vmscan.c static void get_scan_count(struct lruvec *lruvec, struct scan_control *sc, · line 2518
Quoted 10 lines
mm/vmscan.c +10
-0+* which would violate the anon-only semantics. +*/ +if (swappiness ==SWAPPINESS_ANON_ONLY ) { +WARN_ON_ONCE (!sc->proactive); +if (!can_reclaim_anon_pages (memcg, pgdat->node_id, sc)) { +memset (nr,0 ,sizeof (*nr) *NR_LRU_LISTS ); +return ; + } + scan_balance =SCAN_ANON ; + goto out;
Rather than warning the code would be easier to understand (SWAPPINESS_ANON_ONLY implying sc->proactive is very subtle assumption that might change in the future) I would go with and explicit check.
Looking at how the code is structured currently doesn't the following express the intention slightly better?
mm/vmscan.c +4 -1
@@ -2503,6 +2503,10 @@ static void get_scan_count(struct lruvec *lruvec, struct scan_control *sc,/* If we have no swap space, do not bother scanning anon folios. */ if (!sc->may_swap || !can_reclaim_anon_pages (memcg, pgdat->node_id, sc)) { +if (swappiness ==SWAPPINESS_ANON_ONLY && sc->proactive) { +memset (nr,0 ,sizeof (*nr) *NR_LRU_LISTS ); +return ; + } scan_balance =SCAN_FILE ; goto out; }
@@ -2521,7 +2525,6 @@ static void get_scan_count(struct lruvec *lruvec, struct scan_control *sc,/* Proactive reclaim initiated by userspace for anonymous memory only */ if (swappiness ==SWAPPINESS_ANON_ONLY ) { -WARN_ON_ONCE (!sc->proactive); scan_balance =SCAN_ANON ; goto out; }
Signature · 2 lines
Michal Hocko SUSE Labs
Barry Song
reviewerRe: [PATCH -v4 1/4] mm/vmscan: fix anon-only reclaim evicting file pages when swappiness=max
Quoted 90 lines · Michal Hocko
Quoted 65 lines · Ridong
From: Ridong Chen <[email protected]>
As Qi mentioned [1], when swappiness=max (
SWAPPINESS_ANON_ONLY) is set, the reclaim logic is expected to reclaim anonymous pages exclusively. However, due to the current ordering of checks inget_scan_count(), file pages may still be evicted ifcan_reclaim_anon_pages()returns false, which contradicts the semantics ofSWAPPINESS_ANON_ONLY.Reproducer in a cgroup holding 64M of file cache, with no swap configured:
Before (file cache is wrongly evicted):
# cat memory.stat anon 196608 file 67178496 pgscan_proactive 0 # echo "64M swappiness=max" > memory.reclaim # cat memory.stat anon 208896 file 4096 <- page cache evicted pgsteal_proactive 16400 pgscan_proactive 16400After (file cache is left intact):
# cat memory.stat anon 200704 file 67178496 pgscan_proactive 0 # echo "64M swappiness=max" > memory.reclaim -bash: echo: write error: Resource temporarily unavailable # cat memory.stat anon 208896 file 67178496 <- page cache untouched pgsteal_proactive 0 pgscan_proactive 0Fix this by bailing out early when
SWAPPINESS_ANON_ONLYis set and no anonymous pages are reclaimable, before falling back to file reclaim.[1] https://lore.kernel.org/cgroups/[email protected]/
Fixes 68a1436bde00 ("mm: add swappiness=max arg to memory.reclaim for only anon reclaim")Suggested-by Qi Zheng <[email protected]>Acked-by Shakeel Butt <[email protected]>Acked-by Johannes Weiner <[email protected]>Reviewed-by Muchun Song <[email protected]>Reviewed-by Qi Zheng <[email protected]>Reviewed-by Barry Song <[email protected]>Signed-off-by Ridong Chen <[email protected]>--- mm/vmscan.c | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-)mm/vmscan.c +5
-0@@ -2501,6 +2501,23 @@ static void get_scan_count(struct lruvec *lruvec, struct scan_control *sc,enum scan_balance scan_balance;enum lru_list lru; +/* +* Proactive reclaim initiated by userspace for anonymous memory only. +* SWAPPINESS_ANON_ONLY is set only on the proactive reclaim path, so +* warn if it shows up elsewhere. When anon cannot be reclaimed (e.g. +* no swap), bail out instead of falling back to evicting file pages, mm/vmscan.c static void get_scan_count(struct lruvec *lruvec, struct scan_control *sc, · line 2518
Quoted 10 lines
mm/vmscan.c +10
-0+* which would violate the anon-only semantics. +*/ +if (swappiness ==SWAPPINESS_ANON_ONLY ) { +WARN_ON_ONCE (!sc->proactive); +if (!can_reclaim_anon_pages (memcg, pgdat->node_id, sc)) { +memset (nr,0 ,sizeof (*nr) *NR_LRU_LISTS ); +return ; + } + scan_balance =SCAN_ANON ; + goto out;Rather than warning the code would be easier to understand (
SWAPPINESS_ANON_ONLYimplyingsc->proactiveis very subtle assumption that might change in the future) I would go with and explicit check.Looking at how the code is structured currently doesn't the following express the intention slightly better?
mm/vmscan.c +0
-0
mm/vmscan.c static void get_scan_count(struct lruvec *lruvec, struct scan_control *sc, · line 2510
Quoted 8 lines
mm/vmscan.c +4
-0@@ -2503,6 +2503,10 @@ static void get_scan_count(struct lruvec *lruvec, struct scan_control *sc,/* If we have no swap space, do not bother scanning anon folios. */ if (!sc->may_swap || !can_reclaim_anon_pages (memcg, pgdat->node_id, sc)) { +if (swappiness ==SWAPPINESS_ANON_ONLY && sc->proactive) { +memset (nr,0 ,sizeof (*nr) *NR_LRU_LISTS ); +return ; + } scan_balance =SCAN_FILE ;
The question is whether we should allow falling back to SCAN_FILE when swappiness == SWAPPINESS_ANON_ONLY and !sc->proactive, if 201 is ever allowed for non-proactive reclaim in the future. I guess not, since SWAPPINESS_ANON_ONLY literally means reclaiming anonymous memory only?
Re: [PATCH -v4 1/4] mm/vmscan: fix anon-only reclaim evicting file pages when swappiness=max
Quoted 92 lines · Barry Song
Quoted 91 lines · Michal Hocko
Quoted 65 lines · Ridong
From: Ridong Chen <[email protected]>
As Qi mentioned [1], when swappiness=max (
SWAPPINESS_ANON_ONLY) is set, the reclaim logic is expected to reclaim anonymous pages exclusively. However, due to the current ordering of checks inget_scan_count(), file pages may still be evicted ifcan_reclaim_anon_pages()returns false, which contradicts the semantics ofSWAPPINESS_ANON_ONLY.Reproducer in a cgroup holding 64M of file cache, with no swap configured:
Before (file cache is wrongly evicted):
# cat memory.stat anon 196608 file 67178496 pgscan_proactive 0 # echo "64M swappiness=max" > memory.reclaim # cat memory.stat anon 208896 file 4096 <- page cache evicted pgsteal_proactive 16400 pgscan_proactive 16400After (file cache is left intact):
# cat memory.stat anon 200704 file 67178496 pgscan_proactive 0 # echo "64M swappiness=max" > memory.reclaim -bash: echo: write error: Resource temporarily unavailable # cat memory.stat anon 208896 file 67178496 <- page cache untouched pgsteal_proactive 0 pgscan_proactive 0Fix this by bailing out early when
SWAPPINESS_ANON_ONLYis set and no anonymous pages are reclaimable, before falling back to file reclaim.[1] https://lore.kernel.org/cgroups/[email protected]/
Fixes 68a1436bde00 ("mm: add swappiness=max arg to memory.reclaim for only anon reclaim")Suggested-by Qi Zheng <[email protected]>Acked-by Shakeel Butt <[email protected]>Acked-by Johannes Weiner <[email protected]>Reviewed-by Muchun Song <[email protected]>Reviewed-by Qi Zheng <[email protected]>Reviewed-by Barry Song <[email protected]>Signed-off-by Ridong Chen <[email protected]>--- mm/vmscan.c | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-)mm/vmscan.c +5
-0@@ -2501,6 +2501,23 @@ static void get_scan_count(struct lruvec *lruvec, struct scan_control *sc,enum scan_balance scan_balance;enum lru_list lru; +/* +* Proactive reclaim initiated by userspace for anonymous memory only. +* SWAPPINESS_ANON_ONLY is set only on the proactive reclaim path, so +* warn if it shows up elsewhere. When anon cannot be reclaimed (e.g. +* no swap), bail out instead of falling back to evicting file pages, mm/vmscan.c static void get_scan_count(struct lruvec *lruvec, struct scan_control *sc, · line 2518
Quoted 10 lines
mm/vmscan.c +10
-0+* which would violate the anon-only semantics. +*/ +if (swappiness ==SWAPPINESS_ANON_ONLY ) { +WARN_ON_ONCE (!sc->proactive); +if (!can_reclaim_anon_pages (memcg, pgdat->node_id, sc)) { +memset (nr,0 ,sizeof (*nr) *NR_LRU_LISTS ); +return ; + } + scan_balance =SCAN_ANON ; + goto out;Rather than warning the code would be easier to understand (
SWAPPINESS_ANON_ONLYimplyingsc->proactiveis very subtle assumption that might change in the future) I would go with and explicit check.Looking at how the code is structured currently doesn't the following express the intention slightly better?
mm/vmscan.c +0
-0@@ -2503,6 +2503,10 @@ static void get_scan_count(struct lruvec *lruvec, struct scan_control *sc,
Quoted 13 lines
mm/vmscan.c static void get_scan_count(struct lruvec *lruvec, struct scan_control *sc, · line 2510
Quoted 7 lines
mm/vmscan.c +4
-0/* If we have no swap space, do not bother scanning anon folios. */ if (!sc->may_swap || !can_reclaim_anon_pages (memcg, pgdat->node_id, sc)) { +if (swappiness ==SWAPPINESS_ANON_ONLY && sc->proactive) { +memset (nr,0 ,sizeof (*nr) *NR_LRU_LISTS ); +return ; + } scan_balance =SCAN_FILE ;The question is whether we should allow falling back to
SCAN_FILEwhen swappiness ==SWAPPINESS_ANON_ONLYand !sc->proactive, if 201 is ever allowed for non-proactive reclaim in the future. I guess not, sinceSWAPPINESS_ANON_ONLYliterally means reclaiming anonymous memory only?
If we follow swappiness==0 then yes we should fallback if we are close to system OOM. Now arguably SWAPPINESS_ANON_ONLY is a wierd global policy and it is quite possible this will not be ever allowed. My argument was not to prepare the existing code for that. I just meant to structure the code in a way that it current assumptions are more explicit.
Signature · 2 lines
Michal Hocko SUSE Labs
Barry Song
reviewerRe: [PATCH -v4 1/4] mm/vmscan: fix anon-only reclaim evicting file pages when swappiness=max
Quoted 1 line · Michal Hocko
[...]
Quoted 24 lines
Quoted 10 lines
Quoted 10 lines
Looking at how the code is structured currently doesn't the following express the intention slightly better?
mm/vmscan.c +0
-0@@ -2503,6 +2503,10 @@ static void get_scan_count(struct lruvec *lruvec, struct scan_control *sc,Quoted 13 lines
mm/vmscan.c static void get_scan_count(struct lruvec *lruvec, struct scan_control *sc, · line 2510
Quoted 7 lines
mm/vmscan.c +4
-0/* If we have no swap space, do not bother scanning anon folios. */ if (!sc->may_swap || !can_reclaim_anon_pages (memcg, pgdat->node_id, sc)) { +if (swappiness ==SWAPPINESS_ANON_ONLY && sc->proactive) { +memset (nr,0 ,sizeof (*nr) *NR_LRU_LISTS ); +return ; + } scan_balance =SCAN_FILE ;The question is whether we should allow falling back to
SCAN_FILEwhen swappiness ==SWAPPINESS_ANON_ONLYand !sc->proactive, if 201 is ever allowed for non-proactive reclaim in the future. I guess not, sinceSWAPPINESS_ANON_ONLYliterally means reclaiming anonymous memory only?
Quoted 2 lines
If we follow swappiness==0 then yes we should fallback if we are close to system OOM. Now arguably
SWAPPINESS_ANON_ONLYis a wierd global
SWAPPINESS_ANON_ONLY with swappiness=201 is a rather odd interface. I suspect it was introduced as a workaround for MGLRU's swappiness behavior, where swappiness=200 can still result in significant file reclaim (not much different from swappiness=60 or 100), while the active/inactive LRU reclaims few or no file folios.
So 201 was introduced to prevent MGLRU from reclaiming file folios by bypassing the file reclaim path?
Qi, please correct me if I'm wrong. :-) If it serves a purpose beyond this, even for the active/inactive LRU, please let me know.
I'm working on addressing mglru swappiness issue:
https://lore.kernel.org/linux-mm/[email protected]/
With my patchset, swappiness=200 under MGLRU reclaims very few file folios, similar to the active/inactive LRU. If that works well, I suspect we may be able to drop the special 201 value in the future.
Quoted 4 lines
policy and it is quite possible this will not be ever allowed. My argument was not to prepare the existing code for that. I just meant to structure the code in a way that it current assumptions are more explicit.
I understand your point. I just find
if (swappiness == SWAPPINESS_ANON_ONLY && sc->proactive)
a bit odd, since swappiness == SWAPPINESS_ANON_ONLY already implies sc->proactive.
So maybe:
if (!sc->may_swap || !can_reclaim_anon_pages(memcg,
pgdat->node_id, sc)) {
+ if (swappiness == SWAPPINESS_ANON_ONLY) {
+ WARN_ON_ONCE(!sc->proactive);
+ memset(nr, 0, sizeof(*nr) * NR_LRU_LISTS);
+ return;+ }
?
Best Regards
Barry
Re: [PATCH -v4 1/4] mm/vmscan: fix anon-only reclaim evicting file pages when swappiness=max
On Tue 28-07-26 18:03:24, Barry Song wrote:
[...]
Quoted 6 lines
I understand your point. I just find
if (swappiness == SWAPPINESS_ANON_ONLY && sc->proactive)a bit odd, since
swappiness == SWAPPINESS_ANON_ONLYalready impliessc->proactive.
It does so very implicitely and you need to look around to learn about that. That is my biggest concen here. WARN on is just a very poor annotation to remind us about that assumption.
I would much rather have get_scan_count more self explanatory as it is quite heavy already.
Signature · 2 lines
Michal Hocko SUSE Labs
Barry Song
reviewerRe: [PATCH -v4 1/4] mm/vmscan: fix anon-only reclaim evicting file pages when swappiness=max
Quoted 10 lines · Michal Hocko
On Tue 28-07-26 18:03:24, Barry Song wrote:
[...]Quoted 6 lines
I understand your point. I just find
if (swappiness == SWAPPINESS_ANON_ONLY && sc->proactive)a bit odd, since
swappiness == SWAPPINESS_ANON_ONLYalready impliessc->proactive.
Quoted 6 lines
It does so very implicitely and you need to look around to learn about that. That is my biggest concen here. WARN on is just a very poor annotation to remind us about that assumption.
I would much rather have
get_scan_countmore self explanatory as it is quite heavy already.
OK. The only thing that surprises me is that this seems to leave open the possibility thatswappiness == SWAPPINESS_ANON_ONLY && !sc->proactive is legal. Otherwise, I am fine with it.
If we decide to make this change, it seems we need to do the same thing in patch 4 as well:
https://lore.kernel.org/linux-mm/[email protected]/
Best Regards
Barry
PATCH 2/4 mm: vmscan: propagate real error code from per-node proactive reclaim
In reply to RidongFrom: Ridong Chen <[email protected]>
It has been observed that per-node proactive reclaim always returns -EAGAIN when any error occurs. As discussed in the mailing list [1], the interface should distinguish between cases where no reclaimable memory is left and where another entity is concurrently using the same interface. Propagate the real error code, consistent with how memcg proactive reclaim handles errors.
--- mm/vmscan.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
mm/vmscan.c +1 -1
@@ -8023,7 +8023,7 @@ static ssize_t reclaim_store(struct device *dev,int ret, nid = dev->id; ret =user_proactive_reclaim ((char *)buf,NULL ,NODE_DATA (nid)); -return ret ? -EAGAIN : count; +return ret ? ret : count; }static DEVICE_ATTR_WO (reclaim);
Signature · 1 line
2.34.1
PATCH 3/4 mm: vmscan: drop unused gfp_mask parameter from __node_reclaim()
In reply to RidongFrom: Ridong Chen <[email protected]>
Commit 57972c78e678 ("mm/vmscan: make __node_reclaim() more generic") moved the scan_control construction out to the callers and passed the struct in by pointer. After that change every use of the gfp mask inside __node_reclaim() goes through sc->gfp_mask, leaving the gfp_mask parameter unused. Just remove the dead parameter and update the callers accordingly. No functional change.
--- mm/vmscan.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)
mm/vmscan.c +4 -5
@@ -7748,7 +7748,7 @@ static unsigned long node_pagecache_reclaimable(struct pglist_data *pgdat)/* * Try to free up some pages from this node through reclaim. */ -static unsigned long __node_reclaim (struct pglist_data *pgdat,gfp_t gfp_mask, +static unsigned long __node_reclaim (struct pglist_data *pgdat,unsigned long nr_pages,struct scan_control *sc) {
@@ -7840,7 +7840,7 @@ int node_reclaim(struct pglist_data *pgdat, gfp_t gfp_mask, unsigned int order)if (test_and_set_bit_lock (PGDAT_RECLAIM_LOCKED , &pgdat->flags))return NODE_RECLAIM_NOSCAN ; - ret =__node_reclaim (pgdat, gfp_mask, nr_pages, &sc) >= nr_pages; + ret =__node_reclaim (pgdat, nr_pages, &sc) >= nr_pages;clear_bit_unlock (PGDAT_RECLAIM_LOCKED , &pgdat->flags);if (ret)
@@ -7853,7 +7853,7 @@ int node_reclaim(struct pglist_data *pgdat, gfp_t gfp_mask, unsigned int order)#else -static unsigned long __node_reclaim (struct pglist_data *pgdat,gfp_t gfp_mask, +static unsigned long __node_reclaim (struct pglist_data *pgdat,unsigned long nr_pages,struct scan_control *sc) {
@@ -7955,8 +7955,7 @@ int user_proactive_reclaim(char *buf, &pgdat->flags))return -EBUSY ; - reclaimed =__node_reclaim (pgdat, gfp_mask, - batch_size, &sc); + reclaimed =__node_reclaim (pgdat, batch_size, &sc);clear_bit_unlock (PGDAT_RECLAIM_LOCKED , &pgdat->flags); }
Signature · 1 line
2.34.1
PATCH 4/4 mm/mglru: fix anon-only reclaim evicting file pages when swappiness=max
In reply to RidongFrom: Ridong Chen <[email protected]>
The previous patch fixed this issue for the traditional LRU. The same problem exists in MGLRU [1]: when swappiness=max (SWAPPINESS_ANON_ONLY) is set, reclaim is expected to evict anonymous pages exclusively, but file pages can still be reclaimed when anonymous pages cannot be reclaimed (e.g. no swap and no demotion target).
Fix it the same way as the traditional LRU: keep returning SWAPPINESS_ANON_ONLY in get_swappiness(), and return 0 from get_nr_to_scan() when SWAPPINESS_ANON_ONLY is set but anon pages cannot be reclaimed. Since get_nr_to_scan() decides how much MGLRU scans, returning 0 skips the scan entirely and avoids the useless scan work when there is nothing eligible to reclaim.
The test result:
Before fix:
# cat /sys/kernel/mm/lru_gen/enabled 0x0007 # cat memory.stat anon 204800 file 67108864 ... pgsteal_proactive 0 pgscan_proactive 0
# echo "64M swappiness=max" > memory.reclaim # cat memory.stat anon 208896 file 0 ... pgsteal_proactive 16384 pgscan_proactive 16384
After fix:
# cat memory.stat anon 188416 file 67215360 kernel 1970176 ... pgsteal_proactive 0 pgscan_proactive 0
# echo "64M swappiness=max" > memory.reclaim -bash: echo: write error: Resource temporarily unavailable # cat memory.stat anon 204800 file 67215360 ... pgsteal_proactive 0 pgscan_proactive 0
[1] https://sashiko.dev/#/patchset/[email protected]
--- mm/vmscan.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-)
mm/vmscan.c +19 -1
@@ -2708,6 +2708,10 @@ static int get_swappiness(struct lruvec *lruvec, struct scan_control *sc) {struct mem_cgroup *memcg =lruvec_memcg (lruvec);struct pglist_data *pgdat =lruvec_pgdat (lruvec); +int swappiness =sc_swappiness (sc, memcg); + +if (swappiness ==SWAPPINESS_ANON_ONLY ) +return swappiness;if (!sc->may_swap)return 0 ;
@@ -2716,7 +2720,7 @@ static int get_swappiness(struct lruvec *lruvec, struct scan_control *sc) mem_cgroup_get_nr_swap_pages(memcg) < MIN_LRU_BATCH)return 0 ; -return sc_swappiness (sc, memcg); +return swappiness; }static int get_nr_gens (struct lruvec *lruvec,int type)
@@ -4921,6 +4925,20 @@ static long get_nr_to_scan(struct lruvec *lruvec, struct scan_control *sc, struct mem_cgroup *memcg, int swappiness) {unsigned long nr_to_scan, evictable; +struct pglist_data *pgdat =lruvec_pgdat (lruvec); + +/* +* Proactive reclaim initiated by userspace for anonymous memory only. +* SWAPPINESS_ANON_ONLY is set only on the proactive reclaim path, so +* warn if it shows up elsewhere. When anon cannot be reclaimed (e.g. +* no swap), return 0 to skip the scan entirely, avoiding useless scan +* work when there is nothing eligible to reclaim. +*/ +if (swappiness ==SWAPPINESS_ANON_ONLY ) { +WARN_ON_ONCE (!sc->proactive); +if (!can_reclaim_anon_pages (memcg, pgdat->node_id, sc)) +return 0 ; + } evictable =lruvec_evictable_size (lruvec, swappiness);
Signature · 1 line
2.34.1
Re: [PATCH -v4 4/4] mm/mglru: fix anon-only reclaim evicting file pages when swappiness=max
Quoted 109 lines · Ridong
From: Ridong Chen <[email protected]>
The previous patch fixed this issue for the traditional LRU. The same problem exists in MGLRU [1]: when swappiness=max (
SWAPPINESS_ANON_ONLY) is set, reclaim is expected to evict anonymous pages exclusively, but file pages can still be reclaimed when anonymous pages cannot be reclaimed (e.g. no swap and no demotion target).Fix it the same way as the traditional LRU: keep returning
SWAPPINESS_ANON_ONLYinget_swappiness(), and return 0 fromget_nr_to_scan()whenSWAPPINESS_ANON_ONLYis set but anon pages cannot be reclaimed. Sinceget_nr_to_scan()decides how much MGLRU scans, returning 0 skips the scan entirely and avoids the useless scan work when there is nothing eligible to reclaim.The test result:
Before fix:# cat /sys/kernel/mm/lru_gen/enabled 0x0007 # cat memory.stat anon 204800 file 67108864 ... pgsteal_proactive 0 pgscan_proactive 0# echo "64M swappiness=max" > memory.reclaim # cat memory.stat anon 208896 file 0 ... pgsteal_proactive 16384 pgscan_proactive 16384After fix:
# cat memory.stat anon 188416 file 67215360 kernel 1970176 ... pgsteal_proactive 0 pgscan_proactive 0# echo "64M swappiness=max" > memory.reclaim -bash: echo: write error: Resource temporarily unavailable # cat memory.stat anon 204800 file 67215360 ... pgsteal_proactive 0 pgscan_proactive 0[1] https://sashiko.dev/#/patchset/[email protected]
Fixes 68a1436bde00 ("mm: add swappiness=max arg to memory.reclaim for only anon reclaim")Signed-off-by Ridong Chen <[email protected]>--- mm/vmscan.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-)mm/vmscan.c +19
-2@@ -2708,6 +2708,10 @@ static int get_swappiness(struct lruvec *lruvec, struct scan_control *sc) {struct mem_cgroup *memcg =lruvec_memcg (lruvec);struct pglist_data *pgdat =lruvec_pgdat (lruvec); +int swappiness =sc_swappiness (sc, memcg); + +if (swappiness ==SWAPPINESS_ANON_ONLY ) +return swappiness;if (!sc->may_swap)return 0 ;@@ -2716,7 +2720,7 @@ static int get_swappiness(struct lruvec *lruvec, struct scan_control *sc) mem_cgroup_get_nr_swap_pages(memcg) < MIN_LRU_BATCH)return 0 ; -return sc_swappiness (sc, memcg); +return swappiness; }static int get_nr_gens (struct lruvec *lruvec,int type)@@ -4921,6 +4925,20 @@ static long get_nr_to_scan(struct lruvec *lruvec, struct scan_control *sc, struct mem_cgroup *memcg, int swappiness) {unsigned long nr_to_scan, evictable; +struct pglist_data *pgdat =lruvec_pgdat (lruvec); + +/* +* Proactive reclaim initiated by userspace for anonymous memory only. +* SWAPPINESS_ANON_ONLY is set only on the proactive reclaim path, so +* warn if it shows up elsewhere. When anon cannot be reclaimed (e.g. +* no swap), return 0 to skip the scan entirely, avoiding useless scan +* work when there is nothing eligible to reclaim. +*/ +if (swappiness ==SWAPPINESS_ANON_ONLY ) { +WARN_ON_ONCE (!sc->proactive); +if (!can_reclaim_anon_pages (memcg, pgdat->node_id, sc)) +return 0 ; + } evictable =lruvec_evictable_size (lruvec, swappiness); --
Quoted 1 line
2.34.1
Looks good to me, thanks!
Re: [PATCH -v4 4/4] mm/mglru: fix anon-only reclaim evicting file pages when swappiness=max
In reply to RidongQuoted 55 lines · Ridong
From: Ridong Chen <[email protected]>
The previous patch fixed this issue for the traditional LRU. The same problem exists in MGLRU [1]: when swappiness=max (
SWAPPINESS_ANON_ONLY) is set, reclaim is expected to evict anonymous pages exclusively, but file pages can still be reclaimed when anonymous pages cannot be reclaimed (e.g. no swap and no demotion target).Fix it the same way as the traditional LRU: keep returning
SWAPPINESS_ANON_ONLYinget_swappiness(), and return 0 fromget_nr_to_scan()whenSWAPPINESS_ANON_ONLYis set but anon pages cannot be reclaimed. Sinceget_nr_to_scan()decides how much MGLRU scans, returning 0 skips the scan entirely and avoids the useless scan work when there is nothing eligible to reclaim.The test result:
Before fix:# cat /sys/kernel/mm/lru_gen/enabled 0x0007 # cat memory.stat anon 204800 file 67108864 ... pgsteal_proactive 0 pgscan_proactive 0# echo "64M swappiness=max" > memory.reclaim # cat memory.stat anon 208896 file 0 ... pgsteal_proactive 16384 pgscan_proactive 16384After fix:
# cat memory.stat anon 188416 file 67215360 kernel 1970176 ... pgsteal_proactive 0 pgscan_proactive 0# echo "64M swappiness=max" > memory.reclaim -bash: echo: write error: Resource temporarily unavailable # cat memory.stat anon 204800 file 67215360 ... pgsteal_proactive 0 pgscan_proactive 0
Quoted 4 lines
[1] https://sashiko.dev/#/patchset/[email protected]
Fixes 68a1436bde00 ("mm: add swappiness=max arg to memory.reclaim for only anon reclaim")Signed-off-by Ridong Chen <[email protected]>
Re: [PATCH -v4 4/4] mm/mglru: fix anon-only reclaim evicting file pages when swappiness=max
In reply to RidongQuoted 54 lines · Ridong
From: Ridong Chen <[email protected]>
The previous patch fixed this issue for the traditional LRU. The same problem exists in MGLRU [1]: when swappiness=max (
SWAPPINESS_ANON_ONLY) is set, reclaim is expected to evict anonymous pages exclusively, but file pages can still be reclaimed when anonymous pages cannot be reclaimed (e.g. no swap and no demotion target).Fix it the same way as the traditional LRU: keep returning
SWAPPINESS_ANON_ONLYinget_swappiness(), and return 0 fromget_nr_to_scan()whenSWAPPINESS_ANON_ONLYis set but anon pages cannot be reclaimed. Sinceget_nr_to_scan()decides how much MGLRU scans, returning 0 skips the scan entirely and avoids the useless scan work when there is nothing eligible to reclaim.The test result:
Before fix:# cat /sys/kernel/mm/lru_gen/enabled 0x0007 # cat memory.stat anon 204800 file 67108864 ... pgsteal_proactive 0 pgscan_proactive 0# echo "64M swappiness=max" > memory.reclaim # cat memory.stat anon 208896 file 0 ... pgsteal_proactive 16384 pgscan_proactive 16384After fix:
# cat memory.stat anon 188416 file 67215360 kernel 1970176 ... pgsteal_proactive 0 pgscan_proactive 0# echo "64M swappiness=max" > memory.reclaim -bash: echo: write error: Resource temporarily unavailable # cat memory.stat anon 204800 file 67215360 ... pgsteal_proactive 0 pgscan_proactive 0
Quoted 5 lines
[1] https://sashiko.dev/#/patchset/[email protected]
Fixes 68a1436bde00 ("mm: add swappiness=max arg to memory.reclaim for only anon reclaim")Signed-off-by Ridong Chen <[email protected]>---
LGTM.
Andrew Morton
maintainerQuoted 7 lines · On Fri, 24 Jul 2026 11:34:31 +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 toSCAN_FILEand evicted page cache instead.
Quoted 7 lines
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_maskparameter from__node_reclaim().Patch 4 fixes the same "swappiness=max" issue for MGLRU.
Thanks, I've updated mm.git's mm-unstable branch to this version.
Sashiko said one thing, but you've examined this before. Hopefully MGLRU maintainers will consider your suggestion in
https://lore.kernel.org/[email protected]
https://sashiko.dev/#/patchset/[email protected]
Quoted 3 lines
v3 -> v4:
- Patch 4: Update comments and commit message.
- Patch 4: Adopted Sashiko's suggestion to useWARN_ON_ONCE.
Here's how v4 altered mm.git:
mm/vmscan.c | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-)
mm/vmscan.c renamed from mm/vmscan.c~b +26 -2
@@ -2492,7 +2492,13 @@ static void get_scan_count(struct lruvecenum scan_balance scan_balance;enum lru_list lru; -/* Proactive reclaim initiated by userspace for anonymous memory only */ +/* +* Proactive reclaim initiated by userspace for anonymous memory only. +* SWAPPINESS_ANON_ONLY is set only on the proactive reclaim path, so +* warn if it shows up elsewhere. When anon cannot be reclaimed (e.g. +* no swap), bail out instead of falling back to evicting file pages, +* which would violate the anon-only semantics. +*/ if (swappiness ==SWAPPINESS_ANON_ONLY ) {WARN_ON_ONCE (!sc->proactive);if (!can_reclaim_anon_pages (memcg, pgdat->node_id, sc)) {
@@ -2693,6 +2699,10 @@ static int get_swappiness(struct lruvec {struct mem_cgroup *memcg =lruvec_memcg (lruvec);struct pglist_data *pgdat =lruvec_pgdat (lruvec); +int swappiness =sc_swappiness (sc, memcg); + +if (swappiness ==SWAPPINESS_ANON_ONLY ) +return swappiness;if (!sc->may_swap)return 0 ;
@@ -2701,7 +2711,7 @@ static int get_swappiness(struct lruvec mem_cgroup_get_nr_swap_pages(memcg) < MIN_LRU_BATCH)return 0 ; -return sc_swappiness (sc, memcg); +return swappiness; }static int get_nr_gens (struct lruvec *lruvec,int type)
@@ -4903,6 +4913,20 @@ static long get_nr_to_scan(struct lruvec struct mem_cgroup *memcg, int swappiness) {unsigned long nr_to_scan, evictable; +struct pglist_data *pgdat =lruvec_pgdat (lruvec); + +/* +* Proactive reclaim initiated by userspace for anonymous memory only. +* SWAPPINESS_ANON_ONLY is set only on the proactive reclaim path, so +* warn if it shows up elsewhere. When anon cannot be reclaimed (e.g. +* no swap), return 0 to skip the scan entirely, avoiding useless scan +* work when there is nothing eligible to reclaim. +*/ +if (swappiness ==SWAPPINESS_ANON_ONLY ) { +WARN_ON_ONCE (!sc->proactive); +if (!can_reclaim_anon_pages (memcg, pgdat->node_id, sc)) +return 0 ; + } evictable =lruvec_evictable_size (lruvec, swappiness);
_