All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/2] mm/zswap: Fixes and improves the zswap global shrinker
@ 2026-07-17  8:51 Hao Jia
  2026-07-17  8:51 ` [PATCH v2 1/2] mm/zswap: Fix global shrinker when memory cgroup is disabled Hao Jia
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Hao Jia @ 2026-07-17  8:51 UTC (permalink / raw)
  To: akpm, tj, hannes, shakeel.butt, mhocko, yosry, mkoutny, nphamcs,
	chengming.zhou, muchun.song, roman.gushchin
  Cc: linux-mm, linux-kernel, linux-doc, Hao Jia

From: Hao Jia <jiahao1@lixiang.com>

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.

v1->v2:
    - Add a reschedule check to the -ENOENT return path in shrink_memcg() to
      handle the theoretical issue of prolonged heavy concurrent zswap stores.
    - Remove the shrink_memcg() return value changes part, and include a more
       detailed test report in the commit message.


[v1] https://lore.kernel.org/all/20260714081510.16895-1-jiahao.kernel@gmail.com

Hao Jia (2):
  mm/zswap: Fix global shrinker when memory cgroup is disabled
  mm/zswap: Support batch writeback in shrink_memcg()

 mm/zswap.c | 60 ++++++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 47 insertions(+), 13 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 1/2] mm/zswap: Fix global shrinker when memory cgroup is disabled
  2026-07-17  8:51 [PATCH v2 0/2] mm/zswap: Fixes and improves the zswap global shrinker Hao Jia
@ 2026-07-17  8:51 ` Hao Jia
  2026-07-23  2:13   ` Johannes Weiner
  2026-07-17  8:51 ` [PATCH v2 2/2] mm/zswap: Support batch writeback in shrink_memcg() Hao Jia
  2026-07-18  1:18 ` [PATCH v2 0/2] mm/zswap: Fixes and improves the zswap global shrinker Andrew Morton
  2 siblings, 1 reply; 15+ messages in thread
From: Hao Jia @ 2026-07-17  8:51 UTC (permalink / raw)
  To: akpm, tj, hannes, shakeel.butt, mhocko, yosry, mkoutny, nphamcs,
	chengming.zhou, muchun.song, roman.gushchin
  Cc: linux-mm, linux-kernel, linux-doc, Hao Jia, stable

From: Hao Jia <jiahao1@lixiang.com>

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: stable@vger.kernel.org
Suggested-by: Nhat Pham <nphamcs@gmail.com>
Acked-by: Nhat Pham <nphamcs@gmail.com>
Acked-by: Yosry Ahmed <yosry@kernel.org>
Reported-by: Yosry Ahmed <yosry@kernel.org>
Closes: https://lore.kernel.org/all/CAO9r8zPVzMKFbCixxD-qgtRrkFxWVrHiZZeLc=eyTPKPVQgX4g@mail.gmail.com
Signed-off-by: Hao Jia <jiahao1@lixiang.com>
---
 mm/zswap.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/mm/zswap.c b/mm/zswap.c
index b5a17ea20237..48fc7b575e24 100644
--- a/mm/zswap.c
+++ b/mm/zswap.c
@@ -1356,11 +1356,12 @@ static void shrink_worker(struct work_struct *w)
 		} while (memcg && !mem_cgroup_tryget_online(memcg));
 		spin_unlock(&zswap_shrink_lock);
 
-		if (!memcg) {
-			/*
-			 * Continue shrinking without incrementing failures if
-			 * we found candidate memcgs in the last tree walk.
-			 */
+		/*
+		 * A NULL memcg ends a full hierarchy pass (except when memcg is
+		 * disabled, where it is always NULL: fall through to the root LRU).
+		 * Count a failure only if the last pass found no candidates.
+		 */
+		if (!memcg && !mem_cgroup_disabled()) {
 			if (!attempts && ++failures == MAX_RECLAIM_RETRIES)
 				break;
 
@@ -1379,7 +1380,7 @@ static void shrink_worker(struct work_struct *w)
 		 * and failures.
 		 */
 		if (ret == -ENOENT)
-			continue;
+			goto resched;
 		++attempts;
 
 		if (ret && ++failures == MAX_RECLAIM_RETRIES)
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 2/2] mm/zswap: Support batch writeback in shrink_memcg()
  2026-07-17  8:51 [PATCH v2 0/2] mm/zswap: Fixes and improves the zswap global shrinker Hao Jia
  2026-07-17  8:51 ` [PATCH v2 1/2] mm/zswap: Fix global shrinker when memory cgroup is disabled Hao Jia
@ 2026-07-17  8:51 ` Hao Jia
  2026-07-17 16:45   ` Yosry Ahmed
                     ` (2 more replies)
  2026-07-18  1:18 ` [PATCH v2 0/2] mm/zswap: Fixes and improves the zswap global shrinker Andrew Morton
  2 siblings, 3 replies; 15+ messages in thread
From: Hao Jia @ 2026-07-17  8:51 UTC (permalink / raw)
  To: akpm, tj, hannes, shakeel.butt, mhocko, yosry, mkoutny, nphamcs,
	chengming.zhou, muchun.song, roman.gushchin
  Cc: linux-mm, linux-kernel, linux-doc, Hao Jia

From: Hao Jia <jiahao1@lixiang.com>

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 <yosry@kernel.org>
Signed-off-by: Hao Jia <jiahao1@lixiang.com>
---
 mm/zswap.c | 47 ++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 40 insertions(+), 7 deletions(-)

diff --git a/mm/zswap.c b/mm/zswap.c
index 48fc7b575e24..6a09b9ebfd25 100644
--- a/mm/zswap.c
+++ b/mm/zswap.c
@@ -1275,9 +1275,27 @@ static struct shrinker *zswap_alloc_shrinker(void)
 	return shrinker;
 }
 
-static int shrink_memcg(struct mem_cgroup *memcg)
+#define NR_ZSWAP_WB_BATCH	64UL
+
+/*
+ * Scan up to @nr_to_scan pages across the per-node zswap LRUs of @memcg
+ * and write back the reclaimable ones.
+ *
+ * Since the second-chance algorithm rotates referenced entries to the
+ * LRU tail, the per-node scan is capped at the current LRU length so
+ * each entry is scanned at most once per call. It is up to the caller
+ * to handle retries, deciding whether to scan another memcg to complete
+ * the full iteration, or to rescan the current memcg to drain its zswap
+ * entries.
+ *
+ * Return: 0 if at least one entry was written back, -EAGAIN if entries
+ * were scanned but none could be written back, or -ENOENT if @memcg has
+ * writeback disabled, is a zombie cgroup, or has empty zswap LRUs.
+ */
+static int shrink_memcg(struct mem_cgroup *memcg, unsigned long nr_to_scan)
 {
-	int nid, shrunk = 0, scanned = 0;
+	unsigned long nr_remaining = nr_to_scan;
+	int nid, shrunk = 0;
 
 	if (!mem_cgroup_zswap_writeback_enabled(memcg))
 		return -ENOENT;
@@ -1290,14 +1308,29 @@ static int shrink_memcg(struct mem_cgroup *memcg)
 		return -ENOENT;
 
 	for_each_node_state(nid, N_NORMAL_MEMORY) {
-		unsigned long nr_to_walk = 1;
+		unsigned long nr_to_walk;
 
+		/*
+		 * Cap the scan at per-node LRU length so each entry is scanned
+		 * at most once per call.
+		 */
+		nr_to_walk = min(nr_remaining,
+				 list_lru_count_one(&zswap_list_lru, nid, memcg));
+		if (!nr_to_walk)
+			continue;
+
+		nr_remaining -= nr_to_walk;
 		shrunk += list_lru_walk_one(&zswap_list_lru, nid, memcg,
 					    &shrink_memcg_cb, NULL, &nr_to_walk);
-		scanned += 1 - nr_to_walk;
+		/* Return the unused share of the budget to the pool. */
+		nr_remaining += nr_to_walk;
+
+		if (!nr_remaining)
+			break;
 	}
 
-	if (!scanned)
+	/* Nothing was scanned: every LRU under @memcg was empty. */
+	if (nr_remaining == nr_to_scan)
 		return -ENOENT;
 
 	return shrunk ? 0 : -EAGAIN;
@@ -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);
 
@@ -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)) {
 			mem_cgroup_put(memcg);
 			goto put_objcg;
 		}
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 2/2] mm/zswap: Support batch writeback in shrink_memcg()
  2026-07-17  8:51 ` [PATCH v2 2/2] mm/zswap: Support batch writeback in shrink_memcg() Hao Jia
@ 2026-07-17 16:45   ` Yosry Ahmed
  2026-07-17 16:46   ` Nhat Pham
  2026-07-23  2:27   ` Johannes Weiner
  2 siblings, 0 replies; 15+ messages in thread
From: Yosry Ahmed @ 2026-07-17 16:45 UTC (permalink / raw)
  To: Hao Jia
  Cc: akpm, tj, hannes, shakeel.butt, mhocko, mkoutny, nphamcs,
	chengming.zhou, muchun.song, roman.gushchin, linux-mm,
	linux-kernel, linux-doc, Hao Jia

On Fri, Jul 17, 2026 at 04:51:51PM +0800, Hao Jia wrote:
> From: Hao Jia <jiahao1@lixiang.com>
> 
> 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 <yosry@kernel.org>
> Signed-off-by: Hao Jia <jiahao1@lixiang.com>

Acked-by: Yosry Ahmed <yosry@kernel.org>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 2/2] mm/zswap: Support batch writeback in shrink_memcg()
  2026-07-17  8:51 ` [PATCH v2 2/2] mm/zswap: Support batch writeback in shrink_memcg() Hao Jia
  2026-07-17 16:45   ` Yosry Ahmed
@ 2026-07-17 16:46   ` Nhat Pham
  2026-07-23  2:27   ` Johannes Weiner
  2 siblings, 0 replies; 15+ messages in thread
From: Nhat Pham @ 2026-07-17 16:46 UTC (permalink / raw)
  To: Hao Jia
  Cc: akpm, tj, hannes, shakeel.butt, mhocko, yosry, mkoutny,
	chengming.zhou, muchun.song, roman.gushchin, linux-mm,
	linux-kernel, linux-doc, Hao Jia

On Fri, Jul 17, 2026 at 1:52 AM Hao Jia <jiahao.kernel@gmail.com> wrote:
>
> From: Hao Jia <jiahao1@lixiang.com>
>
> 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
>

Acked-by: Nhat Pham <nphamcs@gmail.com>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 0/2] mm/zswap: Fixes and improves the zswap global shrinker
  2026-07-17  8:51 [PATCH v2 0/2] mm/zswap: Fixes and improves the zswap global shrinker Hao Jia
  2026-07-17  8:51 ` [PATCH v2 1/2] mm/zswap: Fix global shrinker when memory cgroup is disabled Hao Jia
  2026-07-17  8:51 ` [PATCH v2 2/2] mm/zswap: Support batch writeback in shrink_memcg() Hao Jia
@ 2026-07-18  1:18 ` Andrew Morton
  2026-07-18  1:22   ` Yosry Ahmed
  2026-07-18  1:28   ` Yosry Ahmed
  2 siblings, 2 replies; 15+ messages in thread
From: Andrew Morton @ 2026-07-18  1:18 UTC (permalink / raw)
  To: Hao Jia
  Cc: tj, hannes, shakeel.butt, mhocko, yosry, mkoutny, nphamcs,
	chengming.zhou, muchun.song, roman.gushchin, linux-mm,
	linux-kernel, linux-doc, Hao Jia

On Fri, 17 Jul 2026 16:51:49 +0800 Hao Jia <jiahao.kernel@gmail.com> 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/20260717085151.22822-1-jiahao.kernel@gmail.com



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 0/2] mm/zswap: Fixes and improves the zswap global shrinker
  2026-07-18  1:18 ` [PATCH v2 0/2] mm/zswap: Fixes and improves the zswap global shrinker Andrew Morton
@ 2026-07-18  1:22   ` Yosry Ahmed
  2026-07-18  1:28   ` Yosry Ahmed
  1 sibling, 0 replies; 15+ messages in thread
From: Yosry Ahmed @ 2026-07-18  1:22 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Hao Jia, tj, hannes, shakeel.butt, mhocko, mkoutny, nphamcs,
	chengming.zhou, muchun.song, roman.gushchin, linux-mm,
	linux-kernel, linux-doc, Hao Jia

On Fri, Jul 17, 2026 at 6:18 PM Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Fri, 17 Jul 2026 16:51:49 +0800 Hao Jia <jiahao.kernel@gmail.com> 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/20260717085151.22822-1-jiahao.kernel@gmail.com

We discussed this one in the previous version, it's a theoretical
scenario that can already happen today.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 0/2] mm/zswap: Fixes and improves the zswap global shrinker
  2026-07-18  1:18 ` [PATCH v2 0/2] mm/zswap: Fixes and improves the zswap global shrinker Andrew Morton
  2026-07-18  1:22   ` Yosry Ahmed
@ 2026-07-18  1:28   ` Yosry Ahmed
  2026-07-18  4:40     ` Andrew Morton
  1 sibling, 1 reply; 15+ messages in thread
From: Yosry Ahmed @ 2026-07-18  1:28 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Hao Jia, tj, hannes, shakeel.butt, mhocko, mkoutny, nphamcs,
	chengming.zhou, muchun.song, roman.gushchin, linux-mm,
	linux-kernel, linux-doc, Hao Jia

On Fri, Jul 17, 2026 at 6:18 PM Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Fri, 17 Jul 2026 16:51:49 +0800 Hao Jia <jiahao.kernel@gmail.com> 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.

Thank you!

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 0/2] mm/zswap: Fixes and improves the zswap global shrinker
  2026-07-18  1:28   ` Yosry Ahmed
@ 2026-07-18  4:40     ` Andrew Morton
  2026-07-20  1:26       ` Hao Jia
  0 siblings, 1 reply; 15+ messages in thread
From: Andrew Morton @ 2026-07-18  4:40 UTC (permalink / raw)
  To: Yosry Ahmed
  Cc: Hao Jia, tj, hannes, shakeel.butt, mhocko, mkoutny, nphamcs,
	chengming.zhou, muchun.song, roman.gushchin, linux-mm,
	linux-kernel, linux-doc, Hao Jia

On Fri, 17 Jul 2026 18:28:04 -0700 Yosry Ahmed <yosry@kernel.org> wrote:

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



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 0/2] mm/zswap: Fixes and improves the zswap global shrinker
  2026-07-18  4:40     ` Andrew Morton
@ 2026-07-20  1:26       ` Hao Jia
  2026-07-23  1:21         ` Hao Jia
  0 siblings, 1 reply; 15+ messages in thread
From: Hao Jia @ 2026-07-20  1:26 UTC (permalink / raw)
  To: Andrew Morton, Yosry Ahmed
  Cc: tj, hannes, shakeel.butt, mhocko, mkoutny, nphamcs,
	chengming.zhou, muchun.song, roman.gushchin, linux-mm,
	linux-kernel, linux-doc, Hao Jia



On 2026/7/18 12:40, Andrew Morton wrote:
> On Fri, 17 Jul 2026 18:28:04 -0700 Yosry Ahmed <yosry@kernel.org> wrote:
> 
>>>
>>> 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?
> 

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 0/2] mm/zswap: Fixes and improves the zswap global shrinker
  2026-07-20  1:26       ` Hao Jia
@ 2026-07-23  1:21         ` Hao Jia
  2026-07-23  4:49           ` Yosry Ahmed
  0 siblings, 1 reply; 15+ messages in thread
From: Hao Jia @ 2026-07-23  1:21 UTC (permalink / raw)
  To: Andrew Morton, Yosry Ahmed, nphamcs
  Cc: tj, hannes, shakeel.butt, mhocko, mkoutny, chengming.zhou,
	muchun.song, roman.gushchin, linux-mm, linux-kernel, linux-doc,
	Hao Jia



On 2026/7/20 09:26, Hao Jia wrote:
> 
> 
> On 2026/7/18 12:40, Andrew Morton wrote:
>> On Fri, 17 Jul 2026 18:28:04 -0700 Yosry Ahmed <yosry@kernel.org> wrote:
>>
>>>>
>>>> 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?
>>
> 
> 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?

Thanks,
Hao
> 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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 1/2] mm/zswap: Fix global shrinker when memory cgroup is disabled
  2026-07-17  8:51 ` [PATCH v2 1/2] mm/zswap: Fix global shrinker when memory cgroup is disabled Hao Jia
@ 2026-07-23  2:13   ` Johannes Weiner
  0 siblings, 0 replies; 15+ messages in thread
From: Johannes Weiner @ 2026-07-23  2:13 UTC (permalink / raw)
  To: Hao Jia
  Cc: akpm, tj, shakeel.butt, mhocko, yosry, mkoutny, nphamcs,
	chengming.zhou, muchun.song, roman.gushchin, linux-mm,
	linux-kernel, linux-doc, Hao Jia, stable

On Fri, Jul 17, 2026 at 04:51:50PM +0800, Hao Jia wrote:
> From: Hao Jia <jiahao1@lixiang.com>
> 
> 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: stable@vger.kernel.org
> Suggested-by: Nhat Pham <nphamcs@gmail.com>
> Acked-by: Nhat Pham <nphamcs@gmail.com>
> Acked-by: Yosry Ahmed <yosry@kernel.org>
> Reported-by: Yosry Ahmed <yosry@kernel.org>
> Closes: https://lore.kernel.org/all/CAO9r8zPVzMKFbCixxD-qgtRrkFxWVrHiZZeLc=eyTPKPVQgX4g@mail.gmail.com
> Signed-off-by: Hao Jia <jiahao1@lixiang.com>

Acked-by: Johannes Weiner <hannes@cmpxchg.org>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 2/2] mm/zswap: Support batch writeback in shrink_memcg()
  2026-07-17  8:51 ` [PATCH v2 2/2] mm/zswap: Support batch writeback in shrink_memcg() Hao Jia
  2026-07-17 16:45   ` Yosry Ahmed
  2026-07-17 16:46   ` Nhat Pham
@ 2026-07-23  2:27   ` Johannes Weiner
  2026-07-23  4:52     ` Yosry Ahmed
  2 siblings, 1 reply; 15+ messages in thread
From: Johannes Weiner @ 2026-07-23  2:27 UTC (permalink / raw)
  To: Hao Jia
  Cc: akpm, tj, shakeel.butt, mhocko, yosry, mkoutny, nphamcs,
	chengming.zhou, muchun.song, roman.gushchin, linux-mm,
	linux-kernel, linux-doc, Hao Jia

On Fri, Jul 17, 2026 at 04:51:51PM +0800, Hao Jia wrote:
> @@ -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);
>  
> @@ -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.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 0/2] mm/zswap: Fixes and improves the zswap global shrinker
  2026-07-23  1:21         ` Hao Jia
@ 2026-07-23  4:49           ` Yosry Ahmed
  0 siblings, 0 replies; 15+ messages in thread
From: Yosry Ahmed @ 2026-07-23  4:49 UTC (permalink / raw)
  To: Hao Jia
  Cc: Andrew Morton, nphamcs, tj, hannes, shakeel.butt, mhocko, mkoutny,
	chengming.zhou, muchun.song, roman.gushchin, linux-mm,
	linux-kernel, linux-doc, Hao Jia

On Wed, Jul 22, 2026 at 6:21 PM Hao Jia <jiahao.kernel@gmail.com> wrote:
>
>
>
> On 2026/7/20 09:26, Hao Jia wrote:
> >
> >
> > On 2026/7/18 12:40, Andrew Morton wrote:
> >> On Fri, 17 Jul 2026 18:28:04 -0700 Yosry Ahmed <yosry@kernel.org> wrote:
> >>
> >>>>
> >>>> 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?
> >>
> >
> > 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).

...

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 2/2] mm/zswap: Support batch writeback in shrink_memcg()
  2026-07-23  2:27   ` Johannes Weiner
@ 2026-07-23  4:52     ` Yosry Ahmed
  0 siblings, 0 replies; 15+ messages in thread
From: Yosry Ahmed @ 2026-07-23  4:52 UTC (permalink / raw)
  To: Johannes Weiner
  Cc: Hao Jia, akpm, tj, shakeel.butt, mhocko, mkoutny, nphamcs,
	chengming.zhou, muchun.song, roman.gushchin, linux-mm,
	linux-kernel, linux-doc, Hao Jia

On Wed, Jul 22, 2026 at 7:27 PM Johannes Weiner <hannes@cmpxchg.org> wrote:
>
> On Fri, Jul 17, 2026 at 04:51:51PM +0800, Hao Jia wrote:
> > @@ -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);
> >
> > @@ -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?

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2026-07-23  4:52 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-17  8:51 [PATCH v2 0/2] mm/zswap: Fixes and improves the zswap global shrinker Hao Jia
2026-07-17  8:51 ` [PATCH v2 1/2] mm/zswap: Fix global shrinker when memory cgroup is disabled Hao Jia
2026-07-23  2:13   ` Johannes Weiner
2026-07-17  8:51 ` [PATCH v2 2/2] mm/zswap: Support batch writeback in shrink_memcg() Hao Jia
2026-07-17 16:45   ` Yosry Ahmed
2026-07-17 16:46   ` Nhat Pham
2026-07-23  2:27   ` Johannes Weiner
2026-07-23  4:52     ` Yosry Ahmed
2026-07-18  1:18 ` [PATCH v2 0/2] mm/zswap: Fixes and improves the zswap global shrinker Andrew Morton
2026-07-18  1:22   ` Yosry Ahmed
2026-07-18  1:28   ` Yosry Ahmed
2026-07-18  4:40     ` Andrew Morton
2026-07-20  1:26       ` Hao Jia
2026-07-23  1:21         ` Hao Jia
2026-07-23  4:49           ` Yosry Ahmed

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.