mm/vmscan: fix swappiness=max and clean up per-node proactive reclaim

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.

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

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

index 859655fcf60d..254827544461 100644
@@ -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);
 
--

2.34.1

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

Fixes 68a1436bde00 ("mm: add swappiness=max arg to memory.reclaim for only anon reclaim")

Signed-off-by Ridong Chen <[email protected]>

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

Fixes 68a1436bde00 ("mm: add swappiness=max arg to memory.reclaim for only anon reclaim")

Signed-off-by Ridong Chen <[email protected]>

---

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.

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

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?

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

Quoted 3 lines

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?

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

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 16400

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

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

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

index 35c3bb15ae96..2c689682b952 100644
@@ -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,

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

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

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

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

index 35c3bb15ae96..2c689682b952 100644
@@ -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 +0 -0

index 35c3bb15ae96..4fd38e3f1b05 100644

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

Quoted 3 lines

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?

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 0

Ridong, please correct me if I am wrong.

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

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

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

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

index 35c3bb15ae96..2c689682b952 100644
@@ -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 +0 -0

index 35c3bb15ae96..4fd38e3f1b05 100644
@@ -2503,6 +2503,10 @@ static void get_scan_count(struct lruvec *lruvec, struct scan_control *sc,
 

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_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?

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

index 35c3bb15ae96..4fd38e3f1b05 100644
@@ -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_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?

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.

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.

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_ONLY already implies sc->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.