mm/zswap: Fixes and improves the zswap global shrinker

From: Hao Jia <[email protected]>

Zswap writeback on hitting the pool limit is broken when memory cgroup is disabled, because mem_cgroup_iter() always returns NULL. Therefore, the global shrinker shrink_worker() always takes the !memcg branch. After MAX_RECLAIM_RETRIES empty walks, the worker simply gives up, so it fails to write back anything.

Therefore, when memory cgroup is disabled, fall through with the !memcg branch and shrink the root memcg directly.

With memcg disabled, shrink_memcg() only returns -ENOENT when the root LRU is empty, which means the total pages are already below thr. In the absence of heavy concurrent zswap stores, the loop then safely bails out via the zswap_total_pages() <= thr check; otherwise, it will resume shrinking the memcg after processing the reschedule check. For any other return value from shrink_memcg(), the loop is guaranteed to terminate, either after MAX_RECLAIM_RETRIES failures or once the threshold is met.

Fixes a65b0e7607cc ("zswap: make shrinking memcg-aware")

Cc [email protected]

Suggested-by Nhat Pham <[email protected]>

Acked-by Nhat Pham <[email protected]>

Acked-by Yosry Ahmed <[email protected]>

Reported-by Yosry Ahmed <[email protected]>

Closes https://lore.kernel.org/all/CAO9r8zPVzMKFbCixxD-qgtRrkFxWVrHiZZeLc=eyTPKPVQgX4g@mail.gmail.com open here

Signed-off-by Hao Jia <[email protected]>

From: Hao Jia <[email protected]>

Currently, shrink_memcg() writes back at most one entry per-node during its traversal. This makes shrink_worker() inefficient, as it must repeatedly re-enter shrink_memcg() to make any substantial progress. Under high memory pressure, this can cause the writeback speed to be too slow to keep up with refaults, leading to zswap store failures and forcing pages to skip zswap and go directly to disk, which results in an LRU inversion.

To address this, extend shrink_memcg() and rewrite its LRU iteration logic to support batch writeback. Introduce the nr_to_scan parameter to bound how many pages are scanned per call. This enables batch writeback in the shrink_worker() path, while maintaining a low scan budget in the zswap_store() path.

Test Setup:
- Total memory: 32 GB.
- zswap settings: max_pool_percent=1, accept_threshold_percent=50, shrinker_enabled=N.

Test Case 1:
Allocate 512MB of anonymous pages and fill them with random data (to avoid compression), then use cgroup memory.reclaim to force a large amount of anonymous pages into zswap. At an interval of 2ms, allocate a 4K anonymous page where the first 4 bytes are random numbers and the rest are zeros, and then trigger a reclamation of this 4K anonymous page through cgroup memory.reclaim. When the pool threshold is reached, shrink_memcg() will be triggered.
The test data after running for 120s is as follows:
Baseline Patched
shrink_worker wakeups 5,363 85
shrink_memcg calls 11,373,201 180,928
written_back pages 40,212 40,236

zswap_store calls                161,190       168,741
   store succeeded (ret=1)       102,743       127,644
   store rejected (ret=0)         58,447        41,097
   store reject rate                ~36%          ~24%
pool_limit_hit delta              55,826        14,062
pswpout                           98,659        81,333

pswpin 2 1

Test Case 2:
To consistently force zswap store failures and trigger shrink_worker(), the following stress-ng command was run for 120 seconds within a cgroup limited to a memory.max of 1G:

   bash -c 'echo $$ > /sys/fs/cgroup/zswaptest/cgroup.procs ; \
   exec stress-ng --vm 4 --vm-bytes 4G --vm-keep --vm-method rand-set -t \

120s -q'
The test data after running for 120s is as follows:
Baseline Patched
shrink_worker wakeups 5,640 987
shrink_memcg calls 8,481,500 2,504,818
written_back pages 260 768,576
zswap_store calls 2,742,756 2,301,414
store succeeded (ret=1) 934,640 1,308,686
store rejected (ret=0) 1,808,116 992,728
store reject rate ~66% ~43%

pool_limit_hit delta           1,181,310       101,593
pswpout                        1,808,376     1,761,304
pswpin                         4,288,497     3,902,658

Under identical workloads and runtimes, batching the zswap shrinker exhibits a significant reduction in both shrink_worker wakeups and shrink_memcg calls. Furthermore, the sharp drop in both pool_limit_hit and zswap_store rejections demonstrates that batching the zswap shrinker effectively mitigates zswap_store failures caused by hitting the pool limit. This significantly prevents pages from bypassing zswap and falling back directly to disk, thereby reducing LRU inversion.

Suggested-by Yosry Ahmed <[email protected]>

Signed-off-by Hao Jia <[email protected]>

From: Hao Jia <[email protected]>

Currently, shrink_memcg() writes back at most one entry per-node during its traversal. This makes shrink_worker() inefficient, as it must repeatedly re-enter shrink_memcg() to make any substantial progress. Under high memory pressure, this can cause the writeback speed to be too slow to keep up with refaults, leading to zswap store failures and forcing pages to skip zswap and go directly to disk, which results in an LRU inversion.

To address this, extend shrink_memcg() and rewrite its LRU iteration logic to support batch writeback. Introduce the nr_to_scan parameter to bound how many pages are scanned per call. This enables batch writeback in the shrink_worker() path, while maintaining a low scan budget in the zswap_store() path.

Test Setup:
- Total memory: 32 GB.
- zswap settings: max_pool_percent=1, accept_threshold_percent=50, shrinker_enabled=N.

Test Case 1:
Allocate 512MB of anonymous pages and fill them with random data (to avoid compression), then use cgroup memory.reclaim to force a large amount of anonymous pages into zswap. At an interval of 2ms, allocate a 4K anonymous page where the first 4 bytes are random numbers and the rest are zeros, and then trigger a reclamation of this 4K anonymous page through cgroup memory.reclaim. When the pool threshold is reached, shrink_memcg() will be triggered.
The test data after running for 120s is as follows:
Baseline Patched
shrink_worker wakeups 5,363 85
shrink_memcg calls 11,373,201 180,928
written_back pages 40,212 40,236

zswap_store calls                161,190       168,741
   store succeeded (ret=1)       102,743       127,644
   store rejected (ret=0)         58,447        41,097
   store reject rate                ~36%          ~24%
pool_limit_hit delta              55,826        14,062
pswpout                           98,659        81,333

pswpin 2 1

Test Case 2:
To consistently force zswap store failures and trigger shrink_worker(), the following stress-ng command was run for 120 seconds within a cgroup limited to a memory.max of 1G:

   bash -c 'echo $$ > /sys/fs/cgroup/zswaptest/cgroup.procs ; \
   exec stress-ng --vm 4 --vm-bytes 4G --vm-keep --vm-method rand-set -t \

120s -q'
The test data after running for 120s is as follows:
Baseline Patched
shrink_worker wakeups 5,640 987
shrink_memcg calls 8,481,500 2,504,818
written_back pages 260 768,576
zswap_store calls 2,742,756 2,301,414
store succeeded (ret=1) 934,640 1,308,686
store rejected (ret=0) 1,808,116 992,728
store reject rate ~66% ~43%

pool_limit_hit delta           1,181,310       101,593
pswpout                        1,808,376     1,761,304
pswpin                         4,288,497     3,902,658

mm/zswap.c +1 -1

@@ -1369,7 +1402,7 @@ static void shrink_worker(struct work_struct *w)
 			goto resched;
 		}
 
-		ret = shrink_memcg(memcg);
+		ret = shrink_memcg(memcg, NR_ZSWAP_WB_BATCH);
 		/* drop the extra reference */
 		mem_cgroup_put(memcg);
 

Quoted 9 lines · Hao Jia

mm/zswap.c +1 -1

@@ -1369,7 +1402,7 @@ static void shrink_worker(struct work_struct *w)
                      goto resched;
              }
 
-             ret = shrink_memcg(memcg);
+             ret = shrink_memcg(memcg, NR_ZSWAP_WB_BATCH);
              /* drop the extra reference */
              mem_cgroup_put(memcg);
 

mm/zswap.c bool zswap_store(struct folio *folio) · line 1529

Quoted 6 lines

mm/zswap.c +1 -1

@@ -1493,7 +1526,7 @@ bool zswap_store(struct folio *folio)
      objcg = get_obj_cgroup_from_folio(folio);
      if (objcg && !obj_cgroup_may_zswap(objcg)) {
              memcg = get_mem_cgroup_from_objcg(objcg);
-             if (shrink_memcg(memcg)) {
+             if (shrink_memcg(memcg, 1)) {

Why 64 for the global limit but only 1 for the cgroup limit? That seems arbitrary in multiple ways.

Direct reclaim, kswapd, proactive reclaim, cgroup limit reclaim use SWAP_CLUSTER_MAX for the batch size near-universally. It's magic too to be sure, but at least you wouldn't have to make up new magic?

Otherwise, the patch looks good to me.

Quoted 11 lines · Johannes Weiner

Quoted 9 lines · Hao Jia

mm/zswap.c +1 -1

@@ -1369,7 +1402,7 @@ static void shrink_worker(struct work_struct *w)
                      goto resched;
              }
 
-             ret = shrink_memcg(memcg);
+             ret = shrink_memcg(memcg, NR_ZSWAP_WB_BATCH);
              /* drop the extra reference */
              mem_cgroup_put(memcg);
 

Quoted 9 lines

mm/zswap.c bool zswap_store(struct folio *folio) · line 1529

Quoted 6 lines

mm/zswap.c +1 -1

@@ -1493,7 +1526,7 @@ bool zswap_store(struct folio *folio)
      objcg = get_obj_cgroup_from_folio(folio);
      if (objcg && !obj_cgroup_may_zswap(objcg)) {
              memcg = get_mem_cgroup_from_objcg(objcg);
-             if (shrink_memcg(memcg)) {
+             if (shrink_memcg(memcg, 1)) {

Why 64 for the global limit but only 1 for the cgroup limit? That seems arbitrary in multiple ways.

I suggested that we keep the writeback here without batching and do that change separately, mainly out of abundance of caution as writeback is done synchronously here so the extra latency could be problematic. I think we probably want to measure the performance impact of that separately.

That being said, this path is potentially too expensive anyway due to the flush, but I would rather we do some basic measurements before batching here.

What do you think?

Quoted 28 lines · Yosry Ahmed

Quoted 11 lines · Johannes Weiner

Quoted 9 lines · Hao Jia

mm/zswap.c +1 -1

@@ -1369,7 +1402,7 @@ static void shrink_worker(struct work_struct *w)
                      goto resched;
              }
 
-             ret = shrink_memcg(memcg);
+             ret = shrink_memcg(memcg, NR_ZSWAP_WB_BATCH);
              /* drop the extra reference */
              mem_cgroup_put(memcg);
 

Quoted 9 lines

mm/zswap.c bool zswap_store(struct folio *folio) · line 1529

Quoted 6 lines

mm/zswap.c +1 -1

@@ -1493,7 +1526,7 @@ bool zswap_store(struct folio *folio)
      objcg = get_obj_cgroup_from_folio(folio);
      if (objcg && !obj_cgroup_may_zswap(objcg)) {
              memcg = get_mem_cgroup_from_objcg(objcg);
-             if (shrink_memcg(memcg)) {
+             if (shrink_memcg(memcg, 1)) {

Why 64 for the global limit but only 1 for the cgroup limit? That seems arbitrary in multiple ways.

I suggested that we keep the writeback here without batching and do that change separately, mainly out of abundance of caution as writeback is done synchronously here so the extra latency could be problematic. I think we probably want to measure the performance impact of that separately.

Quoted 5 lines

That being said, this path is potentially too expensive anyway due to the flush, but I would rather we do some basic measurements before batching here.

What do you think?

It's not an unknown, right? We know this works for direct reclaimers, cgroup limit reclaim e.g., and what the latency implications are.

Because of how reclaim works, we also know it'll call zswap_store() in batches of SWAP_CLUSTER_MAX. If we don't batch here, they're likely to each call shrink_memcg() once we're at the limit - while still risking rejections due to compressibility differences.

My worry is that if we start with an inconsistency, we'll be stuck with it for a long time.

I'd rather start with the clean, consistent version. Dial it back only if we have data to justfiy the complication that we can put into a comment and the changelog that outlines why exactly it's different.

Quoted 44 lines · Johannes Weiner

Quoted 28 lines · Yosry Ahmed

Quoted 11 lines · Johannes Weiner

Quoted 9 lines · Hao Jia

mm/zswap.c +1 -1

@@ -1369,7 +1402,7 @@ static void shrink_worker(struct work_struct *w)
                      goto resched;
              }
 
-             ret = shrink_memcg(memcg);
+             ret = shrink_memcg(memcg, NR_ZSWAP_WB_BATCH);
              /* drop the extra reference */
              mem_cgroup_put(memcg);
 

Quoted 9 lines

mm/zswap.c bool zswap_store(struct folio *folio) · line 1529

Quoted 6 lines

mm/zswap.c +1 -1

@@ -1493,7 +1526,7 @@ bool zswap_store(struct folio *folio)
      objcg = get_obj_cgroup_from_folio(folio);
      if (objcg && !obj_cgroup_may_zswap(objcg)) {
              memcg = get_mem_cgroup_from_objcg(objcg);
-             if (shrink_memcg(memcg)) {
+             if (shrink_memcg(memcg, 1)) {

Why 64 for the global limit but only 1 for the cgroup limit? That seems arbitrary in multiple ways.

I suggested that we keep the writeback here without batching and do that change separately, mainly out of abundance of caution as writeback is done synchronously here so the extra latency could be problematic. I think we probably want to measure the performance impact of that separately.

Quoted 5 lines

That being said, this path is potentially too expensive anyway due to the flush, but I would rather we do some basic measurements before batching here.

What do you think?

It's not an unknown, right? We know this works for direct reclaimers, cgroup limit reclaim e.g., and what the latency implications are.

Because of how reclaim works, we also know it'll call zswap_store() in batches of SWAP_CLUSTER_MAX. If we don't batch here, they're likely to each call shrink_memcg() once we're at the limit - while still risking rejections due to compressibility differences.

Quoted 6 lines

My worry is that if we start with an inconsistency, we'll be stuck with it for a long time.

I'd rather start with the clean, consistent version. Dial it back only if we have data to justfiy the complication that we can put into a comment and the changelog that outlines why exactly it's different.

I am fine with doing that and basically always using NR_ZSWAP_WB_BATCH as the batch size in shrink_memcg(), but I would be more comfortable if we did some sanity testing.

Hao, would you be able to do some smoke testing with NR_ZSWAP_WB_BATCH used for all paths, and memory.zswap.max set in a way that induces writeback? You can probably set memory.zswap.max to 1% of total memory instead of the global pool limit and rerun the same test.

Quoted 44 lines · Johannes Weiner

Quoted 28 lines · Yosry Ahmed

Quoted 11 lines · Johannes Weiner

Quoted 9 lines · Hao Jia

mm/zswap.c +1 -1

@@ -1369,7 +1402,7 @@ static void shrink_worker(struct work_struct *w)
                       goto resched;
               }
 
-             ret = shrink_memcg(memcg);
+             ret = shrink_memcg(memcg, NR_ZSWAP_WB_BATCH);
               /* drop the extra reference */
               mem_cgroup_put(memcg);
 

Quoted 9 lines

mm/zswap.c bool zswap_store(struct folio *folio) · line 1529

Quoted 6 lines

mm/zswap.c +1 -1

@@ -1493,7 +1526,7 @@ bool zswap_store(struct folio *folio)
       objcg = get_obj_cgroup_from_folio(folio);
       if (objcg && !obj_cgroup_may_zswap(objcg)) {
               memcg = get_mem_cgroup_from_objcg(objcg);
-             if (shrink_memcg(memcg)) {
+             if (shrink_memcg(memcg, 1)) {

Why 64 for the global limit but only 1 for the cgroup limit? That seems arbitrary in multiple ways.

I suggested that we keep the writeback here without batching and do that change separately, mainly out of abundance of caution as writeback is done synchronously here so the extra latency could be problematic. I think we probably want to measure the performance impact of that separately.

Quoted 5 lines

That being said, this path is potentially too expensive anyway due to the flush, but I would rather we do some basic measurements before batching here.

What do you think?

It's not an unknown, right? We know this works for direct reclaimers, cgroup limit reclaim e.g., and what the latency implications are.

Because of how reclaim works, we also know it'll call zswap_store() in batches of SWAP_CLUSTER_MAX. If we don't batch here, they're likely to each call shrink_memcg() once we're at the limit - while still risking rejections due to compressibility differences.

Quoted 6 lines

My worry is that if we start with an inconsistency, we'll be stuck with it for a long time.

I'd rather start with the clean, consistent version. Dial it back only if we have data to justfiy the complication that we can put into a comment and the changelog that outlines why exactly it's different.

I am fine with doing that and basically always using NR_ZSWAP_WB_BATCH as the batch size in shrink_memcg(), but I would be more comfortable if we did some sanity testing.

Hao, would you be able to do some smoke testing with NR_ZSWAP_WB_BATCH used for all paths, and memory.zswap.max set in a way that induces writeback? You can probably set memory.zswap.max to 1% of total memory instead of the global pool limit and rerun the same test.

Quoted 52 lines · Yosry Ahmed

Quoted 44 lines · Johannes Weiner

Quoted 28 lines · Yosry Ahmed

Quoted 11 lines · Johannes Weiner

Quoted 9 lines · Hao Jia

mm/zswap.c +1 -1

@@ -1369,7 +1402,7 @@ static void shrink_worker(struct work_struct *w)
                       goto resched;
               }
 
-             ret = shrink_memcg(memcg);
+             ret = shrink_memcg(memcg, NR_ZSWAP_WB_BATCH);
               /* drop the extra reference */
               mem_cgroup_put(memcg);
 

Quoted 9 lines

mm/zswap.c bool zswap_store(struct folio *folio) · line 1529

Quoted 6 lines

mm/zswap.c +1 -1

@@ -1493,7 +1526,7 @@ bool zswap_store(struct folio *folio)
       objcg = get_obj_cgroup_from_folio(folio);
       if (objcg && !obj_cgroup_may_zswap(objcg)) {
               memcg = get_mem_cgroup_from_objcg(objcg);
-             if (shrink_memcg(memcg)) {
+             if (shrink_memcg(memcg, 1)) {

Why 64 for the global limit but only 1 for the cgroup limit? That seems arbitrary in multiple ways.

I suggested that we keep the writeback here without batching and do that change separately, mainly out of abundance of caution as writeback is done synchronously here so the extra latency could be problematic. I think we probably want to measure the performance impact of that separately.

Quoted 5 lines

That being said, this path is potentially too expensive anyway due to the flush, but I would rather we do some basic measurements before batching here.

What do you think?

It's not an unknown, right? We know this works for direct reclaimers, cgroup limit reclaim e.g., and what the latency implications are.

Because of how reclaim works, we also know it'll call zswap_store() in batches of SWAP_CLUSTER_MAX. If we don't batch here, they're likely to each call shrink_memcg() once we're at the limit - while still risking rejections due to compressibility differences.

Quoted 6 lines

My worry is that if we start with an inconsistency, we'll be stuck with it for a long time.

I'd rather start with the clean, consistent version. Dial it back only if we have data to justfiy the complication that we can put into a comment and the changelog that outlines why exactly it's different.

Quoted 8 lines

I am fine with doing that and basically always using NR_ZSWAP_WB_BATCH as the batch size in shrink_memcg(), but I would be more comfortable if we did some sanity testing.

Hao, would you be able to do some smoke testing with NR_ZSWAP_WB_BATCH used for all paths, and memory.zswap.max set in a way that induces writeback? You can probably set memory.zswap.max to 1% of total memory instead of the global pool limit and rerun the same test.

Building on Test Case 2, I set zswap.max=320M (~1% of total system memory) and updated both invocation paths of shrink_memcg() to process batches of 32 or 64. The resulting benchmark data is shown below. (Note: Test Case 2 also sets max_pool_percent=1.)

baseline-cgroup batch-all-32-cgroup
batch-all-64-cgroup
shrink_worker wakeups 7,238 766 367
shrink_memcg calls 12,059,142 1,961,194 983,878
written_back 28,277 301,157 327,997
zswap_store calls 1,349,572 1,168,190 1,114,549

   store succeeded            492,861         521,315           459,246
   store rejected             856,712         646,875           655,303

store reject rate ~63% ~55% ~58%
pool_limit_hit 510,130 50,096 57,715
pswpout 884,989 948,032 983,300
pswpin 1,251,268 1,638,668 1,878,453

The numbers generally look good, the store rejection rate is decreasing and we are naturally doing more writeback which makes sense. The store rejection rate actually increases when the batch size increases to 64, maybe aggressive writeback is more likely to fail and cause a store rejection, so that also makes sense.

The only part I can't immediately reason about is pswpin. How are we reading from physical swap more than we ever wrote to it?

Quoted 9 lines · Yosry Ahmed

The numbers generally look good, the store rejection rate is decreasing and we are naturally doing more writeback which makes sense. The store rejection rate actually increases when the batch size increases to 64, maybe aggressive writeback is more likely to fail and cause a store rejection, so that also makes sense.

The only part I can't immediately reason about is pswpin. How are we reading from physical swap more than we ever wrote to it?

The same page can be swapped out once, and swap in multiple times :)

Unlike zswap, physical swap in is not necessarily exclusive. You load the page in memory, but if the swapfile is not full, etc. etc. you don't invalidate the copy of the data on the swapfile. At reclaim time, we notice the page is not dirty, so we just skip (z)swapout.

Quoted 12 lines · Nhat Pham

Quoted 9 lines · Yosry Ahmed

The numbers generally look good, the store rejection rate is decreasing and we are naturally doing more writeback which makes sense. The store rejection rate actually increases when the batch size increases to 64, maybe aggressive writeback is more likely to fail and cause a store rejection, so that also makes sense.

The only part I can't immediately reason about is pswpin. How are we reading from physical swap more than we ever wrote to it?

Quoted 6 lines

The same page can be swapped out once, and swap in multiple times :)

Unlike zswap, physical swap in is not necessarily exclusive. You load the page in memory, but if the swapfile is not full, etc. etc. you don't invalidate the copy of the data on the swapfile. At reclaim time, we notice the page is not dirty, so we just skip (z)swapout.

Oh right, I forgot about that, thanks.

I still don't fully understand why pswpin is significantly increased with batching here. I would hope that with less LRU inversion we end up with less disk swapin. Probably the test access patterns are just too random compared to real workloads?

Quoted 20 lines · Yosry Ahmed

Quoted 12 lines · Nhat Pham

Quoted 9 lines · Yosry Ahmed

The numbers generally look good, the store rejection rate is decreasing and we are naturally doing more writeback which makes sense. The store rejection rate actually increases when the batch size increases to 64, maybe aggressive writeback is more likely to fail and cause a store rejection, so that also makes sense.

The only part I can't immediately reason about is pswpin. How are we reading from physical swap more than we ever wrote to it?

Quoted 6 lines

The same page can be swapped out once, and swap in multiple times :)

Unlike zswap, physical swap in is not necessarily exclusive. You load the page in memory, but if the swapfile is not full, etc. etc. you don't invalidate the copy of the data on the swapfile. At reclaim time, we notice the page is not dirty, so we just skip (z)swapout.

Quoted 6 lines

Oh right, I forgot about that, thanks.

I still don't fully understand why pswpin is significantly increased with batching here. I would hope that with less LRU inversion we end up with less disk swapin. Probably the test access patterns are just too random compared to real workloads?

Maybe more time in zswap for those tail entries left behind with the smaller batch?

That's a 32 entry window that could zswpin with the smaller batch but pswpin with the larger one.

Quoted 32 lines · Johannes Weiner

Quoted 20 lines · Yosry Ahmed

Quoted 12 lines · Nhat Pham

Quoted 9 lines · Yosry Ahmed

The numbers generally look good, the store rejection rate is decreasing and we are naturally doing more writeback which makes sense. The store rejection rate actually increases when the batch size increases to 64, maybe aggressive writeback is more likely to fail and cause a store rejection, so that also makes sense.

The only part I can't immediately reason about is pswpin. How are we reading from physical swap more than we ever wrote to it?

Quoted 6 lines

The same page can be swapped out once, and swap in multiple times :)

Unlike zswap, physical swap in is not necessarily exclusive. You load the page in memory, but if the swapfile is not full, etc. etc. you don't invalidate the copy of the data on the swapfile. At reclaim time, we notice the page is not dirty, so we just skip (z)swapout.

Quoted 6 lines

Oh right, I forgot about that, thanks.

I still don't fully understand why pswpin is significantly increased with batching here. I would hope that with less LRU inversion we end up with less disk swapin. Probably the test access patterns are just too random compared to real workloads?

Maybe more time in zswap for those tail entries left behind with the smaller batch?

Quoted 2 lines

That's a 32 entry window that could zswpin with the smaller batch but pswpin with the larger one.

Oh I was talking about going from no batching to batch=32. pspwin increased by 387,400 (~30%). We are writing back 272,880 more pages but we also have 209,837 less store rejections. So overall ~ 63,043 more pages should end up on disk, which doesn't explain the increase in pspwin.

The only explanation I can think of is that the test is just randomly accessing memory, so it ends up accessing colder memory that we moved to disk more than hotter memory that we kept in zswap. IOW, the access patterns do not conform to a "normal" workload that benefits from the LRU.

Quoted 33 lines · Yosry Ahmed

Quoted 32 lines · Johannes Weiner

Quoted 20 lines · Yosry Ahmed

Quoted 12 lines · Nhat Pham

Quoted 9 lines · Yosry Ahmed

The numbers generally look good, the store rejection rate is decreasing and we are naturally doing more writeback which makes sense. The store rejection rate actually increases when the batch size increases to 64, maybe aggressive writeback is more likely to fail and cause a store rejection, so that also makes sense.

The only part I can't immediately reason about is pswpin. How are we reading from physical swap more than we ever wrote to it?

Quoted 6 lines

The same page can be swapped out once, and swap in multiple times :)

Unlike zswap, physical swap in is not necessarily exclusive. You load the page in memory, but if the swapfile is not full, etc. etc. you don't invalidate the copy of the data on the swapfile. At reclaim time, we notice the page is not dirty, so we just skip (z)swapout.

Quoted 6 lines

Oh right, I forgot about that, thanks.

I still don't fully understand why pswpin is significantly increased with batching here. I would hope that with less LRU inversion we end up with less disk swapin. Probably the test access patterns are just too random compared to real workloads?

Maybe more time in zswap for those tail entries left behind with the smaller batch?

Quoted 9 lines

Quoted 2 lines

That's a 32 entry window that could zswpin with the smaller batch but pswpin with the larger one.

Oh I was talking about going from no batching to batch=32. pspwin increased by 387,400 (~30%). We are writing back 272,880 more pages but we also have 209,837 less store rejections. So overall ~ 63,043 more pages should end up on disk, which doesn't explain the increase in pspwin.

In my previous runs, I also collected zswpin. could the following explanation help account for what we are seeing?

Assuming stress-ng maintains a roughly similar memory access rate, batching flushes significantly more pages back to disk (meaning pages in the zswap pool are evicted faster, keeping zswap pool residency lower). As a result, when stress-ng accesses swapped memory, it is more likely to fault pages in from disk (pswpin) rather than hit zswap (zswpin).

In fact, zswpin in batch-32 dropped by 143,425 compared to baseline—which aligns with batching having a lower zswpin and a higher pswpin.

To be frank, under the Test Case 2 workload, we cannot strictly guarantee that the total volume of page reads and writes remains consistent across all three runs within the same time window.

baseline-cgroup batch-all-32-cgroup
batch-all-64-cgroup
shrink_worker wakeups 7,238 766 367
shrink_memcg calls 12,059,142 1,961,194 983,878
written_back 28,277 301,157 327,997
zswap_store calls 1,349,572 1,168,190 1,114,549

    store succeeded           492,861       521,315           459,246
    store rejected            856,712       646,875           655,303

store reject rate ~63% ~55% ~58%
pool_limit_hit 510,130 50,096 57,715
pswpout 884,989 948,032 983,300
pswpin 1,251,268 1,638,668 1,878,453

zswpout                    492,861         521,314            459,245
zswpin                     309,631         166,206            84,977 <-

Quoted 47 lines · Hao Jia

Quoted 33 lines · Yosry Ahmed

Quoted 32 lines · Johannes Weiner

Quoted 20 lines · Yosry Ahmed

Quoted 12 lines · Nhat Pham

Quoted 9 lines · Yosry Ahmed

The numbers generally look good, the store rejection rate is decreasing and we are naturally doing more writeback which makes sense. The store rejection rate actually increases when the batch size increases to 64, maybe aggressive writeback is more likely to fail and cause a store rejection, so that also makes sense.

The only part I can't immediately reason about is pswpin. How are we reading from physical swap more than we ever wrote to it?

Quoted 6 lines

The same page can be swapped out once, and swap in multiple times :)

Unlike zswap, physical swap in is not necessarily exclusive. You load the page in memory, but if the swapfile is not full, etc. etc. you don't invalidate the copy of the data on the swapfile. At reclaim time, we notice the page is not dirty, so we just skip (z)swapout.

Quoted 6 lines

Oh right, I forgot about that, thanks.

I still don't fully understand why pswpin is significantly increased with batching here. I would hope that with less LRU inversion we end up with less disk swapin. Probably the test access patterns are just too random compared to real workloads?

Maybe more time in zswap for those tail entries left behind with the smaller batch?

Quoted 9 lines

Quoted 2 lines

That's a 32 entry window that could zswpin with the smaller batch but pswpin with the larger one.

Oh I was talking about going from no batching to batch=32. pspwin increased by 387,400 (~30%). We are writing back 272,880 more pages but we also have 209,837 less store rejections. So overall ~ 63,043 more pages should end up on disk, which doesn't explain the increase in pspwin.

Quoted 8 lines

In my previous runs, I also collected zswpin. could the following explanation help account for what we are seeing?

Assuming stress-ng maintains a roughly similar memory access rate, batching flushes significantly more pages back to disk (meaning pages in the zswap pool are evicted faster, keeping zswap pool residency lower). As a result, when stress-ng accesses swapped memory, it is more likely to fault pages in from disk (pswpin) rather than hit zswap (zswpin).

We are not just writing back more pages to disk, we are also accepting more pages into zswap. The whole point of writeback here is to evict colder pages to disk and accept (relatively) hotter pages into zswap, as they are more likely to be swapped in again.

Quoted 15 lines

In fact, zswpin in batch-32 dropped by 143,425 compared to baseline—which aligns with batching having a lower zswpin and a higher pswpin.

To be frank, under the Test Case 2 workload, we cannot strictly guarantee that the total volume of page reads and writes remains consistent across all three runs within the same time window.

baseline-cgroup batch-all-32-cgroup
batch-all-64-cgroup
shrink_worker wakeups 7,238 766 367
shrink_memcg calls 12,059,142 1,961,194 983,878
written_back 28,277 301,157 327,997
zswap_store calls 1,349,572 1,168,190 1,114,549

Quoted 8 lines

     store succeeded           492,861       521,315           459,246
     store rejected            856,712       646,875           655,303

store reject rate ~63% ~55% ~58%
pool_limit_hit 510,130 50,096 57,715
pswpout 884,989 948,032 983,300
pswpin 1,251,268 1,638,668 1,878,453

zswpout                    492,861         521,314            459,245
zswpin                     309,631         166,206            84,977 <-

One possible explanation is that we are shrinking too aggressively due to concurrent shrinkers. That's one difference between hitting the global limit and the memcg limit. There's a single global shrinker that all store failures will just wake up, while all concurrent stores hitting the memcg limit will start shrinking.

The other explanation is that, as I mentioned earlier, the access pattern is just too random and we are consistently refaulting cold memory. I am honestly a bit concerned about these results.

Hao, would you be able to try re-running the test with a single stressor? It would help us figure out if concurrent shrinking is a problem. It would also be useful to increase the global limit, to isolate writeback from hitting the memcg limit. I don't think both limits will often be used in conjunction, at least not in a way where both limits are hit simultaneously.

It may also be useful to run an actual workload if you are able to, like a kernel build or something, to see if the access patterns change how much we end up swapping in from zswap vs. disk.

On 2026/7/28 00:18, Yosry Ahmed wrote:
Apologies—there was a flaw in my previous test setup. I had configured zswap.max=320M alongside max_pool_percent=1. Since the global zswap pool capacity was roughly 314M (< 320M), the global pool limit was actually being hit before the cgroup zswap.max limit. Both limits were likely interleaving throughout the run, making the results difficult to analyze.

Re-running with **zswap.max=280M** yields results that align with expectations:

Test Case: zmax280 stress-4 pool1%
baseline batch-all32 batch-all64
shrink_worker wakeups 0 0 0
shrink_memcg calls 698,990 57,187 29,649
written_back 655,570 865,131 883,508
store succeeded 1,054,928 1,124,990 1,190,608
store rejected 229,722 22,642 11,659
store reject rate ~17% ~1% ~0%
reject_reclaim_fail 110 15,454 12,985
pswpout 885,437 888,027 895,478

   pswpin                   2,035,415        1,305,341        1,384,751
   zswpout                  1,054,928        1,124,990        1,190,608

zswpin 280,675 162,684 158,556

To isolate the cgroup zswap limit, I also tested with max_pool_percent=50 (which should behave similarly to the zswap.max=280M case above, bounded strictly by cgroup zswap.max):

Test Case: zmax320 stress-4 pool50%
baseline batch-all32 batch-all64
shrink_worker wakeups 0 0 0

   shrink_memcg calls         687,608           54,002           28,116
   written_back               639,176          846,663          862,347
   store succeeded            992,816        1,208,123        1,213,286
   store rejected             231,431           20,425           10,915
   store reject rate             ~18%              ~1%              ~0%
   pswpout                    870,745          867,360          873,553
   pswpin                   1,707,823        1,216,814        1,378,381
   zswpout                    992,816        1,208,123        1,213,286

zswpin 250,220 210,791 185,716

Finally, here is the dataset for zswap.max=320M and max_pool_percent=1 with a single stress-ng worker (stress-1), which also looks consistent with your expectations:

Test Case: zmax320 stress-1 pool1%
baseline batch-all32 batch-all64
shrink_worker wakeups 9,279 980 557
shrink_memcg calls 14,247,647 2,638,751 1,465,915

   written_back                 7,702          439,809          482,809
   store succeeded            329,603          531,224          566,150
   store rejected             883,945          517,578          482,644
   store reject rate             ~73%             ~49%             ~46%
   pswpout                    891,647          957,388          965,453
   pswpin                     692,255          628,246          582,781
   zswpout                    329,603          531,224          566,150
   zswpin                     248,871                3            7,872

Quoted 8 lines · Hao Jia

On 2026/7/28 00:18, Yosry Ahmed wrote:
Apologies—there was a flaw in my previous test setup. I had configured zswap.max=320M alongside max_pool_percent=1. Since the global zswap pool capacity was roughly 314M (< 320M), the global pool limit was actually being hit before the cgroup zswap.max limit. Both limits were likely interleaving throughout the run, making the results difficult to analyze.

Quoted 44 lines

Re-running with **zswap.max=280M** yields results that align with expectations:

Test Case: zmax280 stress-4 pool1%
baseline batch-all32 batch-all64
shrink_worker wakeups 0 0 0
shrink_memcg calls 698,990 57,187 29,649
written_back 655,570 865,131 883,508
store succeeded 1,054,928 1,124,990 1,190,608
store rejected 229,722 22,642 11,659
store reject rate ~17% ~1% ~0%
reject_reclaim_fail 110 15,454 12,985
pswpout 885,437 888,027 895,478

   pswpin                   2,035,415        1,305,341        1,384,751
   zswpout                  1,054,928        1,124,990        1,190,608

zswpin 280,675 162,684 158,556

To isolate the cgroup zswap limit, I also tested with max_pool_percent=50 (which should behave similarly to the zswap.max=280M case above, bounded strictly by cgroup zswap.max):

Test Case: zmax320 stress-4 pool50%
baseline batch-all32 batch-all64
shrink_worker wakeups 0 0 0

   shrink_memcg calls         687,608           54,002           28,116
   written_back               639,176          846,663          862,347
   store succeeded            992,816        1,208,123        1,213,286
   store rejected             231,431           20,425           10,915
   store reject rate             ~18%              ~1%              ~0%
   pswpout                    870,745          867,360          873,553
   pswpin                   1,707,823        1,216,814        1,378,381
   zswpout                    992,816        1,208,123        1,213,286

zswpin 250,220 210,791 185,716

Finally, here is the dataset for zswap.max=320M and max_pool_percent=1 with a single stress-ng worker (stress-1), which also looks consistent with your expectations:

Test Case: zmax320 stress-1 pool1%
baseline batch-all32 batch-all64
shrink_worker wakeups 9,279 980 557
shrink_memcg calls 14,247,647 2,638,751 1,465,915

Quoted 8 lines

   written_back                 7,702          439,809          482,809
   store succeeded            329,603          531,224          566,150
   store rejected             883,945          517,578          482,644
   store reject rate             ~73%             ~49%             ~46%
   pswpout                    891,647          957,388          965,453
   pswpin                     692,255          628,246          582,781
   zswpout                    329,603          531,224          566,150
   zswpin                     248,871                3            7,872

Shall we just go with SWAP_CLUSTER_MAX per Johannes' suggestion then? 32 works the best here FWIW, but seems like it's all relatively good (as long as it's not 1).

(I wonder if this works well with your proactive reclaimer too but we shall see).

Quoted 52 lines · Yosry Ahmed

Quoted 44 lines · Johannes Weiner

Quoted 28 lines · Yosry Ahmed

Quoted 11 lines · Johannes Weiner

Quoted 9 lines · Hao Jia

mm/zswap.c +1 -1

@@ -1369,7 +1402,7 @@ static void shrink_worker(struct work_struct *w)
                       goto resched;
               }
 
-             ret = shrink_memcg(memcg);
+             ret = shrink_memcg(memcg, NR_ZSWAP_WB_BATCH);
               /* drop the extra reference */
               mem_cgroup_put(memcg);
 

Quoted 9 lines

mm/zswap.c bool zswap_store(struct folio *folio) · line 1529

Quoted 6 lines

mm/zswap.c +1 -1

@@ -1493,7 +1526,7 @@ bool zswap_store(struct folio *folio)
       objcg = get_obj_cgroup_from_folio(folio);
       if (objcg && !obj_cgroup_may_zswap(objcg)) {
               memcg = get_mem_cgroup_from_objcg(objcg);
-             if (shrink_memcg(memcg)) {
+             if (shrink_memcg(memcg, 1)) {

Why 64 for the global limit but only 1 for the cgroup limit? That seems arbitrary in multiple ways.

I suggested that we keep the writeback here without batching and do that change separately, mainly out of abundance of caution as writeback is done synchronously here so the extra latency could be problematic. I think we probably want to measure the performance impact of that separately.

Quoted 5 lines

That being said, this path is potentially too expensive anyway due to the flush, but I would rather we do some basic measurements before batching here.

What do you think?

It's not an unknown, right? We know this works for direct reclaimers, cgroup limit reclaim e.g., and what the latency implications are.

Because of how reclaim works, we also know it'll call zswap_store() in batches of SWAP_CLUSTER_MAX. If we don't batch here, they're likely to each call shrink_memcg() once we're at the limit - while still risking rejections due to compressibility differences.

Quoted 6 lines

My worry is that if we start with an inconsistency, we'll be stuck with it for a long time.

I'd rather start with the clean, consistent version. Dial it back only if we have data to justfiy the complication that we can put into a comment and the changelog that outlines why exactly it's different.

Quoted 8 lines

I am fine with doing that and basically always using NR_ZSWAP_WB_BATCH as the batch size in shrink_memcg(), but I would be more comfortable if we did some sanity testing.

Hao, would you be able to do some smoke testing with NR_ZSWAP_WB_BATCH used for all paths, and memory.zswap.max set in a way that induces writeback? You can probably set memory.zswap.max to 1% of total memory instead of the global pool limit and rerun the same test.

Building on Test Case 2, I set zswap.max=320M (~1% of total system memory) and updated both invocation paths of shrink_memcg() to process batches of 32 or 64. The resulting benchmark data is shown below. (Note: Test Case 2 also sets max_pool_percent=1.)

baseline-cgroup batch-all-32-cgroup
batch-all-64-cgroup
shrink_worker wakeups 7,238 766 367
shrink_memcg calls 12,059,142 1,961,194 983,878
written_back 28,277 301,157 327,997
zswap_store calls 1,349,572 1,168,190 1,114,549

   store succeeded            492,861         521,315           459,246
   store rejected             856,712         646,875           655,303

store reject rate ~63% ~55% ~58%
pool_limit_hit 510,130 50,096 57,715
pswpout 884,989 948,032 983,300
pswpin 1,251,268 1,638,668 1,878,453

This series fixes and improves the zswap global shrinker (shrink_worker()):
Patch 1: Fix missing global shrinker when memory cgroup is disabled. Patch 2: Extend shrink_memcg() to support batch writeback and update its
return value semantics, thereby improving the writeback efficiency in the shrink_worker() path.

Quoted 5 lines · On Fri, 17 Jul 2026 16:51:49 +0800 Hao Jia <[email protected]> wrote:

This series fixes and improves the zswap global shrinker (shrink_worker()):
Patch 1: Fix missing global shrinker when memory cgroup is disabled. Patch 2: Extend shrink_memcg() to support batch writeback and update its
return value semantics, thereby improving the writeback efficiency in the shrink_worker() path.

Thanks.

[1/2] is a cc:stable fix so it isn't really appropriate to combine this with [2/2] which doesn't fix any bugs. Because the two patches may take different paths into mainline, with different timings. But that's OK, I can deal with the splitup if needed.

The [1/2] changelog lacks a description of how the flaw impacts users. Please describe this fully and maintain that info within the changelogging. This info helps -stable maintainers and others understand why we're proposing a backport and helps myself and others with timing decisions.

Finally, AI review might have found an issue:

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

Quoted 5 lines · On Fri, 17 Jul 2026 16:51:49 +0800 Hao Jia <[email protected]> wrote:

This series fixes and improves the zswap global shrinker (shrink_worker()):
Patch 1: Fix missing global shrinker when memory cgroup is disabled. Patch 2: Extend shrink_memcg() to support batch writeback and update its
return value semantics, thereby improving the writeback efficiency in the shrink_worker() path.

Thanks.

[1/2] is a cc:stable fix so it isn't really appropriate to combine this with [2/2] which doesn't fix any bugs. Because the two patches may take different paths into mainline, with different timings. But that's OK, I can deal with the splitup if needed.

The [1/2] changelog lacks a description of how the flaw impacts users. Please describe this fully and maintain that info within the changelogging. This info helps -stable maintainers and others understand why we're proposing a backport and helps myself and others with timing decisions.

Quoted 6 lines

The [1/2] changelog lacks a description of how the flaw impacts users. Please describe this fully and maintain that info within the changelogging. This info helps -stable maintainers and others understand why we're proposing a backport and helps myself and others with timing decisions.

The first line in the changelog should be sufficient imo: "Zswap writeback on hitting the pool limit is broken when memory cgroup is disabled"

Quoted 7 lines · On Fri, 17 Jul 2026 18:28:04 -0700 Yosry Ahmed <[email protected]> wrote:

Quoted 6 lines

The [1/2] changelog lacks a description of how the flaw impacts users. Please describe this fully and maintain that info within the changelogging. This info helps -stable maintainers and others understand why we're proposing a backport and helps myself and others with timing decisions.

Quoted 3 lines

The first line in the changelog should be sufficient imo: "Zswap writeback on hitting the pool limit is broken when memory cgroup is disabled"

"broken"? Perhaps this means "fails to occur".

But what is the userspace-visible impact? IOW, why are we proposing a backport?

Quoted 13 lines · Andrew Morton

Quoted 10 lines · On Fri, 17 Jul 2026 18:28:04 -0700 Yosry Ahmed <[email protected]> wrote:

Quoted 6 lines

The [1/2] changelog lacks a description of how the flaw impacts users. Please describe this fully and maintain that info within the changelogging.  This info helps -stable maintainers and others understand why we're proposing a backport and helps myself and others with timing decisions.

The first line in the changelog should be sufficient imo: "Zswap writeback on hitting the pool limit is broken when memory cgroup is disabled"

Quoted 5 lines

"broken"?  Perhaps this means "fails to occur".

But what is the userspace-visible impact?  IOW, why are we proposing a backport?

Perhaps the first paragraph of the commit1 message could be modified as follows? I have added a description of the issues that occur without this patch.

Zswap writeback on hitting the pool limit fails to occur when memory cgroup is disabled, because mem_cgroup_iter() always returns NULL. Therefore, the global shrinker shrink_worker() always takes the !memcg branch. After MAX_RECLAIM_RETRIES empty walks, the worker simply gives up, so it fails to write back anything. As a result, once the pool reaches the zswap limit, every subsequent zswap shrink work run is a no-op. This leads to zswap store failures, forcing pages to bypass zswap and be written directly to the backing swap device, which can trigger issues such as LRU inversion.

Thanks,
Hao

Quoted 22 lines · Hao Jia

Quoted 18 lines · Andrew Morton

Quoted 10 lines · On Fri, 17 Jul 2026 18:28:04 -0700 Yosry Ahmed <[email protected]> wrote:

Quoted 6 lines

The [1/2] changelog lacks a description of how the flaw impacts users. Please describe this fully and maintain that info within the changelogging. This info helps -stable maintainers and others understand why we're proposing a backport and helps myself and others with timing decisions.

The first line in the changelog should be sufficient imo: "Zswap writeback on hitting the pool limit is broken when memory cgroup is disabled"

"broken"? Perhaps this means "fails to occur".

But what is the userspace-visible impact? IOW, why are we proposing a backport?

Quoted 4 lines

Perhaps the first paragraph of the commit1 message could be modified as follows? I have added a description of the issues that occur without this patch.

Hi Andrew, Yosry, and Nhat,
Any thoughts on this change?

Quoted 26 lines · Hao Jia

Quoted 22 lines · Hao Jia

Quoted 18 lines · Andrew Morton

Quoted 10 lines · On Fri, 17 Jul 2026 18:28:04 -0700 Yosry Ahmed <[email protected]> wrote:

Quoted 6 lines

The [1/2] changelog lacks a description of how the flaw impacts users. Please describe this fully and maintain that info within the changelogging. This info helps -stable maintainers and others understand why we're proposing a backport and helps myself and others with timing decisions.

The first line in the changelog should be sufficient imo: "Zswap writeback on hitting the pool limit is broken when memory cgroup is disabled"

"broken"? Perhaps this means "fails to occur".

But what is the userspace-visible impact? IOW, why are we proposing a backport?

Quoted 7 lines

Quoted 4 lines

Perhaps the first paragraph of the commit1 message could be modified as follows? I have added a description of the issues that occur without this patch.

Hi Andrew, Yosry, and Nhat,
Any thoughts on this change?

I would front load the user impact in the first paragraph:

Zswap writeback when the global pool limit is hit fails when memory cgroups are disabled. The pool remains full until it is organically drained by swapins or memory freeing, leading to zswap store failures and pages bypassing getting written directly to the backing swap device, causing LRU inversion (hotter pages with higher fault latency).

...