mm/zswap: Fixes and improves the zswap shrink
v3 cover mm/zswap: Fixes and improves the zswap shrink
From: Hao Jia <[email protected]>
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 thereby improving
the writeback efficiency in the shrink_worker() and zswap_store() paths.
v2->v3:
- Added user impact to the commit 1 message.
- Updated writeback batch size to SWAP_CLUSTER_MAX to avoid introducing new magic macros. And enabled batched shrinking in the zswap_store() path as well.
- Re-benchmarked performance data with the updated patch.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.
[v2] https://lore.kernel.org/all/[email protected]
[v1] https://lore.kernel.org/all/[email protected]
Hao Jia (2):
mm/zswap: Fix global shrinker when memory cgroup is disabled mm/zswap: Support batch writeback in shrink_memcg()
mm/zswap.c | 58 ++++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 45 insertions(+), 13 deletions(-)
Signature · 1 line
2.34.1
PATCH v3 1/2 mm/zswap: Fix global shrinker when memory cgroup is disabled
From: Hao Jia <[email protected]>
Zswap writeback when the global pool limit is hit fails when memory cgroup is 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).
This happens because mem_cgroup_iter() always returns NULL when memory cgroups are disabled. As a result, the global shrinker shrink_worker() repeatedly takes empty walks. After MAX_RECLAIM_RETRIES failed attempts, the worker gives up without writing back any pages.
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.
--- mm/zswap.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-)
mm/zswap.c +7 -6
@@ -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 )
Signature · 1 line
2.34.1
Andrew Morton
maintainerRe: [PATCH v3 1/2] mm/zswap: Fix global shrinker when memory cgroup is disabled
Quoted 22 lines · On Wed, 29 Jul 2026 16:42:05 +0800 Hao Jia <[email protected]> wrote:
Zswap writeback when the global pool limit is hit fails when memory cgroup is 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).
This happens because
mem_cgroup_iter()always returns NULL when memory cgroups are disabled. As a result, the global shrinkershrink_worker()repeatedly takes empty walks. AfterMAX_RECLAIM_RETRIESfailed attempts, the worker gives up without writing back any pages.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-ENOENTwhen 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 thezswap_total_pages()<= thr check; otherwise, it will resume shrinking the memcg after processing the reschedule check. For any other return value fromshrink_memcg(), the loop is guaranteed to terminate, either afterMAX_RECLAIM_RETRIESfailures or once the threshold is met.
Quoted 2 lines
Fixes a65b0e7607cc ("zswap: make shrinking memcg-aware")
How does this affect users? What behavior do they observe when it occurs?
Quoted 1 line
hm, that isn't really a bug report and doesn't answer the above question.
AI review asked a couple of questions:
https://sashiko.dev/#/patchset/[email protected]
Thanks.
Yosry Ahmed
maintainerRe: [PATCH v3 1/2] mm/zswap: Fix global shrinker when memory cgroup is disabled
Quoted 25 lines · Andrew Morton
Quoted 22 lines · On Wed, 29 Jul 2026 16:42:05 +0800 Hao Jia <[email protected]> wrote:
Zswap writeback when the global pool limit is hit fails when memory cgroup is 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).
This happens because
mem_cgroup_iter()always returns NULL when memory cgroups are disabled. As a result, the global shrinkershrink_worker()repeatedly takes empty walks. AfterMAX_RECLAIM_RETRIESfailed attempts, the worker gives up without writing back any pages.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-ENOENTwhen 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 thezswap_total_pages()<= thr check; otherwise, it will resume shrinking the memcg after processing the reschedule check. For any other return value fromshrink_memcg(), the loop is guaranteed to terminate, either afterMAX_RECLAIM_RETRIESfailures or once the threshold is met.
Quoted 5 lines
Quoted 2 lines
Fixes a65b0e7607cc ("zswap: make shrinking memcg-aware")How does this affect users? What behavior do they observe when it occurs?
I think the first paragraph sums it up pretty well, especially the last sentence "hotter pages with higher fault latency".
Quoted 5 lines
Quoted 2 lines
hm, that isn't really a bug report and doesn't answer the above question.
Yeah, it isn't. Probably we should drop "Closes". I assume Hao added it because checkpatch annoyingly complains if you add "Reported-by" without "Closes", so Hao just linked to the thread where I pointed out the bug.
Quoted 3 lines
AI review asked a couple of questions:
https://sashiko.dev/#/patchset/[email protected]
The review on patch #1 is something theoretical, we discussed it at length in previous versions.
For patch #2:
Quoted 11 lines
Does this batching logic break NUMA fairness?
Because
for_each_node_state()always starts from the lowest node ID and breaks when the scan budget is exhausted, subsequent calls toshrink_memcg()will restart at the lowest node ID again.If the lowest node (typically Node 0) consistently has enough items to exhaust the scan budget, wouldn't we exclusively evict pages from it while ignoring older pages on other nodes? Could this cause LRU inversion across nodes, keeping older pages in memory on Node 1 while hot pages on Node 0 are evicted?
Yes, unfairness is possible.
For global shrinking, it's probably not an issue. We reclaim until we hit the acceptance threshold and it's very unlikely this will happen before iterating all nodes (given that the batch size is 32 pages). However, with the shrink_memcg() path, we only reclaim one batch, so there's a chance we'll always reclaim it from node 0.
Maybe we should just drop the early bailout and accept potentially doing more writeback than needed. Hao, WDYT?
If you respin, please also drop the batch size argument to shrink_memcg() as it's now always SWAP_CLUSTER_MAX.
Andrew Morton
maintainerRe: [PATCH v3 1/2] mm/zswap: Fix global shrinker when memory cgroup is disabled
Quoted 26 lines · On Wed, 29 Jul 2026 17:30:56 -0700 Yosry Ahmed <[email protected]> wrote:
Quoted 25 lines · Andrew Morton
Quoted 22 lines · On Wed, 29 Jul 2026 16:42:05 +0800 Hao Jia <[email protected]> wrote:
Zswap writeback when the global pool limit is hit fails when memory cgroup is 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).
This happens because
mem_cgroup_iter()always returns NULL when memory cgroups are disabled. As a result, the global shrinkershrink_worker()repeatedly takes empty walks. AfterMAX_RECLAIM_RETRIESfailed attempts, the worker gives up without writing back any pages.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-ENOENTwhen 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 thezswap_total_pages()<= thr check; otherwise, it will resume shrinking the memcg after processing the reschedule check. For any other return value fromshrink_memcg(), the loop is guaranteed to terminate, either afterMAX_RECLAIM_RETRIESfailures or once the threshold is met.
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
Fixes a65b0e7607cc ("zswap: make shrinking memcg-aware")How does this affect users? What behavior do they observe when it occurs?
I think the first paragraph sums it up pretty well, especially the last sentence "hotter pages with higher fault latency".
How do users observe that?
See, what I'm looking for here is an explanation for why we're proposing a backport. How are our users harmed by the current code and how does this change benefit them?
Quoted 10 lines
Quoted 5 lines
Quoted 2 lines
hm, that isn't really a bug report and doesn't answer the above question.
Yeah, it isn't. Probably we should drop "Closes". I assume Hao added it because checkpatch annoyingly complains if you add "Reported-by" without "Closes", so Hao just linked to the thread where I pointed out the bug.
OK.
Yosry Ahmed
maintainerRe: [PATCH v3 1/2] mm/zswap: Fix global shrinker when memory cgroup is disabled
Quoted 35 lines · Andrew Morton
Quoted 32 lines · On Wed, 29 Jul 2026 17:30:56 -0700 Yosry Ahmed <[email protected]> wrote:
Quoted 30 lines · Andrew Morton
Quoted 22 lines · On Wed, 29 Jul 2026 16:42:05 +0800 Hao Jia <[email protected]> wrote:
Zswap writeback when the global pool limit is hit fails when memory cgroup is 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).
This happens because
mem_cgroup_iter()always returns NULL when memory cgroups are disabled. As a result, the global shrinkershrink_worker()repeatedly takes empty walks. AfterMAX_RECLAIM_RETRIESfailed attempts, the worker gives up without writing back any pages.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-ENOENTwhen 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 thezswap_total_pages()<= thr check; otherwise, it will resume shrinking the memcg after processing the reschedule check. For any other return value fromshrink_memcg(), the loop is guaranteed to terminate, either afterMAX_RECLAIM_RETRIESfailures or once the threshold is met.Quoted 2 lines
Fixes a65b0e7607cc ("zswap: make shrinking memcg-aware")How does this affect users? What behavior do they observe when it occurs?
Quoted 8 lines
Quoted 2 lines
I think the first paragraph sums it up pretty well, especially the last sentence "hotter pages with higher fault latency".
How do users observe that?
See, what I'm looking for here is an explanation for why we're proposing a backport. How are our users harmed by the current code and how does this change benefit them?
It's a potential performance regression when using zswap with memcg disabled. Sorry I just realized you were looking for a high level description not detailed symptoms :)
Andrew Morton
maintainerRe: [PATCH v3 1/2] mm/zswap: Fix global shrinker when memory cgroup is disabled
Quoted 9 lines · On Thu, 30 Jul 2026 09:52:38 -0700 Yosry Ahmed <[email protected]> wrote:
Quoted 9 lines
Quoted 6 lines
Quoted 3 lines
How does this affect users? What behavior do they observe when it occurs?
I think the first paragraph sums it up pretty well, especially the last sentence "hotter pages with higher fault latency".
How do users observe that?
Quoted 7 lines
Quoted 3 lines
See, what I'm looking for here is an explanation for why we're proposing a backport. How are our users harmed by the current code and how does this change benefit them?
It's a potential performance regression when using zswap with memcg disabled. Sorry I just realized you were looking for a high level description not detailed symptoms :)
Are we able to guess how significant this is? It should have significant impact to justify a backport.
Sorry to push - I'm forever trying to train people to describe the userspace-visible effects so we can make these backporting decisions!
And to justify those decisions to -stable maintainers
And so that someone who is experiencing a slowdown in some private kernel branch can look at the changelog and decide whether this might fix it for them.
Yosry Ahmed
maintainerRe: [PATCH v3 1/2] mm/zswap: Fix global shrinker when memory cgroup is disabled
Quoted 16 lines · Andrew Morton
Quoted 13 lines · On Thu, 30 Jul 2026 09:52:38 -0700 Yosry Ahmed <[email protected]> wrote:
Quoted 12 lines
Quoted 6 lines
Quoted 3 lines
How does this affect users? What behavior do they observe when it occurs?
I think the first paragraph sums it up pretty well, especially the last sentence "hotter pages with higher fault latency".
How do users observe that?
See, what I'm looking for here is an explanation for why we're proposing a backport. How are our users harmed by the current code and how does this change benefit them?
Quoted 6 lines
Quoted 3 lines
It's a potential performance regression when using zswap with memcg disabled. Sorry I just realized you were looking for a high level description not detailed symptoms :)
Are we able to guess how significant this is? It should have significant impact to justify a backport.
It depends on the workload. I think a lot of workloads may not notice tbh, I don't think it's common to use zswap without cgroups.
Quoted 8 lines
Sorry to push - I'm forever trying to train people to describe the userspace-visible effects so we can make these backporting decisions!
And to justify those decisions to -stable maintainers
And so that someone who is experiencing a slowdown in some private kernel branch can look at the changelog and decide whether this might fix it for them.
I don't think it's super important to backport, but it is also something that we broke at some point and the fix is straightforward and simple, so imo the risk/reward function is leaning toward backporting it just in case. That being said, I don't feel strongly.
Re: [PATCH v3 1/2] mm/zswap: Fix global shrinker when memory cgroup is disabled
In reply to Yosry AhmedQuoted 38 lines · Yosry Ahmed
Quoted 25 lines · Andrew Morton
Quoted 22 lines · On Wed, 29 Jul 2026 16:42:05 +0800 Hao Jia <[email protected]> wrote:
Zswap writeback when the global pool limit is hit fails when memory cgroup is 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).
This happens because
mem_cgroup_iter()always returns NULL when memory cgroups are disabled. As a result, the global shrinkershrink_worker()repeatedly takes empty walks. AfterMAX_RECLAIM_RETRIESfailed attempts, the worker gives up without writing back any pages.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-ENOENTwhen 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 thezswap_total_pages()<= thr check; otherwise, it will resume shrinking the memcg after processing the reschedule check. For any other return value fromshrink_memcg(), the loop is guaranteed to terminate, either afterMAX_RECLAIM_RETRIESfailures or once the threshold is met.Quoted 5 lines
Quoted 2 lines
Fixes a65b0e7607cc ("zswap: make shrinking memcg-aware")How does this affect users? What behavior do they observe when it occurs?
I think the first paragraph sums it up pretty well, especially the last sentence "hotter pages with higher fault latency".
Quoted 3 lines
Quoted 7 lines
Quoted 2 lines
hm, that isn't really a bug report and doesn't answer the above question.
Yeah, it isn't. Probably we should drop "Closes". I assume Hao added it because checkpatch annoyingly complains if you add "Reported-by" without "Closes", so Hao just linked to the thread where I pointed out the bug.
Yeah, checkpatch will complain if it's missing.
Quoted 24 lines
Quoted 4 lines
AI review asked a couple of questions:
https://sashiko.dev/#/patchset/[email protected]The review on patch #1 is something theoretical, we discussed it at length in previous versions.
For patch #2:
Quoted 11 lines
Does this batching logic break NUMA fairness?
Because
for_each_node_state()always starts from the lowest node ID and breaks when the scan budget is exhausted, subsequent calls toshrink_memcg()will restart at the lowest node ID again.If the lowest node (typically Node 0) consistently has enough items to exhaust the scan budget, wouldn't we exclusively evict pages from it while ignoring older pages on other nodes? Could this cause LRU inversion across nodes, keeping older pages in memory on Node 1 while hot pages on Node 0 are evicted?
Yes, unfairness is possible.
Quoted 9 lines
For global shrinking, it's probably not an issue. We reclaim until we hit the acceptance threshold and it's very unlikely this will happen before iterating all nodes (given that the batch size is 32 pages). However, with the
shrink_memcg()path, we only reclaim one batch, so there's a chance we'll always reclaim it from node 0.Maybe we should just drop the early bailout and accept potentially doing more writeback than needed. Hao, WDYT?
If we scan and attempt to write back SWAP_CLUSTER_MAX zswap entries per node, it might lead to excessive writeback on machines with many NUMA nodes. Furthermore, I'm concerned about introducing higher latency in synchronous shrink paths like zswap_store()—especially on systems with a large number of NUMA nodes, where it could end up writing back hundreds of pages in a single call.
Maybe we could do something like this instead? That way, in the worst-case scenario, it falls back to the baseline behavior without introducing any extra latency risks.
static int shrink_memcg(struct mem_cgroup *memcg) {
- int nid, shrunk = 0, scanned = 0; + unsigned long node_batch, scanned = 0; + int nid, shrunk = 0;
if (!mem_cgroup_zswap_writeback_enabled(memcg))
return -ENOENT;
@@ -1289,14 +1313,26 @@ static int shrink_memcg(struct mem_cgroup *memcg)
if (memcg && !mem_cgroup_online(memcg))
return -ENOENT;+ node_batch = max(1UL, SWAP_CLUSTER_MAX / num_node_state(N_NORMAL_MEMORY));
for_each_node_state(nid, N_NORMAL_MEMORY) {- unsigned long nr_to_walk = 1;
+ unsigned long nr_to_walk, budget;
+
+ /*
+ * Cap the scan at the per-node LRU length so each entry is + * scanned at most once per call. + */
+ budget = min(node_batch,
+ list_lru_count_one(&zswap_list_lru, nid, memcg));
+ if (!budget)
+ continue;
+ nr_to_walk = budget;
shrunk += list_lru_walk_one(&zswap_list_lru, nid, memcg,
&shrink_memcg_cb, NULL,
&nr_to_walk);
- scanned += 1 - nr_to_walk;
+ scanned += budget - nr_to_walk;
}+ /* Nothing was scanned: every LRU under @memcg was empty. */
if (!scanned)
return -ENOENT;Thanks,
Hao
Quoted 2 lines
If you respin, please also drop the batch size argument to
shrink_memcg()as it's now alwaysSWAP_CLUSTER_MAX.
Yosry Ahmed
maintainerRe: [PATCH v3 1/2] mm/zswap: Fix global shrinker when memory cgroup is disabled
Quoted 6 lines
Quoted 4 lines
Yeah, it isn't. Probably we should drop "Closes". I assume Hao added it because checkpatch annoyingly complains if you add "Reported-by" without "Closes", so Hao just linked to the thread where I pointed out the bug.
Yeah, checkpatch will complain if it's missing.
We often ignore checkpatch if it's unreasonable.
Quoted 38 lines
Quoted 22 lines
Quoted 2 lines
AI review asked a couple of questions:
https://sashiko.dev/#/patchset/[email protected]The review on patch #1 is something theoretical, we discussed it at length in previous versions.
For patch #2:
Quoted 11 lines
Does this batching logic break NUMA fairness?
Because
for_each_node_state()always starts from the lowest node ID and breaks when the scan budget is exhausted, subsequent calls toshrink_memcg()will restart at the lowest node ID again.If the lowest node (typically Node 0) consistently has enough items to exhaust the scan budget, wouldn't we exclusively evict pages from it while ignoring older pages on other nodes? Could this cause LRU inversion across nodes, keeping older pages in memory on Node 1 while hot pages on Node 0 are evicted?
Yes, unfairness is possible.
Quoted 9 lines
For global shrinking, it's probably not an issue. We reclaim until we hit the acceptance threshold and it's very unlikely this will happen before iterating all nodes (given that the batch size is 32 pages). However, with the
shrink_memcg()path, we only reclaim one batch, so there's a chance we'll always reclaim it from node 0.Maybe we should just drop the early bailout and accept potentially doing more writeback than needed. Hao, WDYT?
If we scan and attempt to write back
SWAP_CLUSTER_MAXzswap entries per node, it might lead to excessive writeback on machines with many NUMA nodes. Furthermore, I'm concerned about introducing higher latency in synchronous shrink paths like zswap_store()—especially on systems with a large number of NUMA nodes, where it could end up writing back hundreds of pages in a single call.
Quoted 3 lines
Maybe we could do something like this instead? That way, in the worst-case scenario, it falls back to the baseline behavior without introducing any extra latency risks.
This basically errs on the side of honouring the batch size (under-reclaim) instead of doing more writeback, right?
I think I prefer erring on the side of doing more writeback, as it would ultimately result in less LRU inversion, and we are already doing writeback proactively during reclaim through the shrinker.
The other option we can explore is keeping the existing logic with the bailout, but start iterating the nodes at the folio's node in the zswap_store() path. Basically pass an nid to shrink_memcg() and always start there. This will avoid always reclaiming from node 0, and the node of the folio being stored is more likely to be under pressure and need the writeback. I am not sure how complex this would be and whether it's worth doing.
Johannes, Nhats, any thoughts on this?
Quoted 45 lines
static int shrink_memcg(struct mem_cgroup *memcg) {(quoted diff) +16
-3- int nid, shrunk = 0, scanned = 0; + unsigned long node_batch, scanned = 0; + int nid, shrunk = 0; if (!mem_cgroup_zswap_writeback_enabled(memcg)) return -ENOENT;@@ -1289,14 +1313,26 @@ static int shrink_memcg(struct mem_cgroup *memcg) if (memcg && !mem_cgroup_online(memcg)) return -ENOENT; + node_batch = max(1UL, SWAP_CLUSTER_MAX / num_node_state(N_NORMAL_MEMORY)); for_each_node_state(nid, N_NORMAL_MEMORY) { - unsigned long nr_to_walk = 1; + unsigned long nr_to_walk, budget; + + /* + * Cap the scan at the per-node LRU length so each entry is + * scanned at most once per call. + */ + budget = min(node_batch, + list_lru_count_one(&zswap_list_lru, nid, memcg)); + if (!budget) + continue; + nr_to_walk = budget; shrunk += list_lru_walk_one(&zswap_list_lru, nid, memcg, &shrink_memcg_cb, NULL, &nr_to_walk); - scanned += 1 - nr_to_walk; + scanned += budget - nr_to_walk; } + /* Nothing was scanned: every LRU under @memcg was empty. */if (!scanned) return -ENOENT;Thanks,
HaoQuoted 2 lines
If you respin, please also drop the batch size argument to
shrink_memcg()as it's now alwaysSWAP_CLUSTER_MAX.
Johannes Weiner
maintainerRe: [PATCH v3 1/2] mm/zswap: Fix global shrinker when memory cgroup is disabled
Quoted 51 lines · Yosry Ahmed
Quoted 6 lines
Quoted 4 lines
Yeah, it isn't. Probably we should drop "Closes". I assume Hao added it because checkpatch annoyingly complains if you add "Reported-by" without "Closes", so Hao just linked to the thread where I pointed out the bug.
Yeah, checkpatch will complain if it's missing.
We often ignore checkpatch if it's unreasonable.
Quoted 38 lines
Quoted 22 lines
Quoted 2 lines
AI review asked a couple of questions:
https://sashiko.dev/#/patchset/[email protected]The review on patch #1 is something theoretical, we discussed it at length in previous versions.
For patch #2:
Quoted 11 lines
Does this batching logic break NUMA fairness?
Because
for_each_node_state()always starts from the lowest node ID and breaks when the scan budget is exhausted, subsequent calls toshrink_memcg()will restart at the lowest node ID again.If the lowest node (typically Node 0) consistently has enough items to exhaust the scan budget, wouldn't we exclusively evict pages from it while ignoring older pages on other nodes? Could this cause LRU inversion across nodes, keeping older pages in memory on Node 1 while hot pages on Node 0 are evicted?
Yes, unfairness is possible.
Quoted 9 lines
For global shrinking, it's probably not an issue. We reclaim until we hit the acceptance threshold and it's very unlikely this will happen before iterating all nodes (given that the batch size is 32 pages). However, with the
shrink_memcg()path, we only reclaim one batch, so there's a chance we'll always reclaim it from node 0.Maybe we should just drop the early bailout and accept potentially doing more writeback than needed. Hao, WDYT?
If we scan and attempt to write back
SWAP_CLUSTER_MAXzswap entries per node, it might lead to excessive writeback on machines with many NUMA nodes. Furthermore, I'm concerned about introducing higher latency in synchronous shrink paths like zswap_store()—especially on systems with a large number of NUMA nodes, where it could end up writing back hundreds of pages in a single call.Quoted 3 lines
Maybe we could do something like this instead? That way, in the worst-case scenario, it falls back to the baseline behavior without introducing any extra latency risks.
Quoted 6 lines
This basically errs on the side of honouring the batch size (under-reclaim) instead of doing more writeback, right?
I think I prefer erring on the side of doing more writeback, as it would ultimately result in less LRU inversion, and we are already doing writeback proactively during reclaim through the shrinker.
Yes, LRU inversions are worse than reclaiming a few extra pages from the cold tail of the LRU. It's not cumulative after all, it just means we take a bigger bite and can take more stores before the next shrink.
Quoted 9 lines
The other option we can explore is keeping the existing logic with the bailout, but start iterating the nodes at the folio's node in the
zswap_store()path. Basically pass an nid toshrink_memcg()and always start there. This will avoid always reclaiming from node 0, and the node of the folio being stored is more likely to be under pressure and need the writeback. I am not sure how complex this would be and whether it's worth doing.Johannes, Nhats, any thoughts on this?
What happens if simply we do a hard SWAP_CLUSTER_MAX on each node?
With cgroups, we put the zswap entry on the list_lru head of the node and memcg where the incoming page was.
So if you're going after one specific cgroup, it's not like you *actually* reclaim 32 entries on each node of the system. You just write back from nodes where that cgroup has entries.
The global shrinker also does one cgroup at a time (zswap_next_shrink) before checking the limit again, so that's fairly fine-grained too.
Without cgroups, you could have zswap entries on all nodes. But if you have so many nodes that this would constitute hundreds of pages, presumably that's still a drop in the bucket compared to overall memory capacity. SWAP_CLUSTER_MAX isn't that big a number.
So I don't think we need to get fancy. Just do SWAP_CLUSTER_MAX on each node. Delete that nr_to_walk carry-over between them.
PATCH v3 2/2 mm/zswap: Support batch writeback in shrink_memcg()
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, enabling batch writeback for both the shrink_worker() and zswap_store() paths. To prevent shrink unfairness across NUMA nodes caused by a shared global scan quota, limit scanning to up to SWAP_CLUSTER_MAX pages per node and write back any reclaimable entries found.
Test Setup:
- Total memory: 32 GB, 1 NUMA node.
- zswap settings: accept_threshold_percent=50, shrinker_enabled=N.
Test Case 1:
Set max_pool_percent=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 reclamation of this 4K 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 Patchedshrink_worker wakeups 5,363 169shrink_memcg calls 11,373,201 350,703written_back pages 40,212 40,241
zswap_store calls 161,190 163,753 store succeeded (ret=1) 102,743 117,183 store rejected (ret=0) 58,447 46,570 store reject rate ~36% ~28% pool_limit_hit delta 55,826 33,760 pswpout 98,659 86,811
pswpin 2 0
Test Case 2:
We evaluated the following two sub-configurations using stress-ng inside a cgroup capped at memory.max=1G for 120 seconds:
Test Case 2a (max_pool_percent=1): Continuously triggers the global zswap pool limit, thereby waking up shrink_worker() to perform asynchronous shrinking.
Test Case 2b (zswap.max=320M, max_pool_percent=50): Continuously triggers the cgroup's zswap.max limit, thereby invoking synchronous shrinking.
Command executed for both setups:
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'
Test Case 2a (max_pool_percent=1):
Baseline Patchedshrink_worker wakeups 5,640 1,308shrink_memcg calls 8,481,500 3,140,972written_back pages 260 468,216zswap_store calls 2,742,756 2,011,269
store succeeded (ret=1) 934,640 947,988
store rejected (ret=0) 1,808,116 1,063,281
store reject rate ~66% ~52%
pool_limit_hit delta 1,181,310 196,882 pswpout 1,808,376 1,531,497 pswpin 4,288,497 3,635,365
Test Case 2b (zswap.max=320M, max_pool_percent=50):
Baseline Patchedshrink_worker wakeups 0 0shrink_memcg calls 687,608 54,002written_back pages 639,176 846,663zswap_store calls 1,224,222 1,228,548
store succeeded (ret=1) 992,816 1,208,123
store rejected (ret=0) 231,431 20,425
store reject rate ~19% ~2%pool_limit_hit delta 0 0
pswpout 870,745 867,360
pswpin 1,707,823 1,216,814
Under identical workloads and runtimes, batched zswap shrinking exhibits a significant reduction in both shrink_worker() wakeups and shrink_memcg() calls. Furthermore, the sharp drop in both pswpin and zswap_store() rejections demonstrates that batching zswap shrink operations 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.
--- mm/zswap.c | 30 ++++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-)
mm/zswap.c +28 -2
@@ -1275,6 +1275,21 @@ static struct shrinker *zswap_alloc_shrinker(void)return shrinker; } +/* +* Scan up to SWAP_CLUSTER_MAX pages on each per-node zswap LRU 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) {int nid, shrunk =0 , scanned =0 ;
@@ -1290,13 +1305,24 @@ 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, node_budget; + +/* +* Cap the scan at the per-node LRU length so each entry is +* scanned at most once per call. +*/ + node_budget =min (SWAP_CLUSTER_MAX , +list_lru_count_one (&zswap_list_lru, nid, memcg)); +if (!node_budget) +continue ; + nr_to_walk = node_budget; shrunk +=list_lru_walk_one (&zswap_list_lru, nid, memcg, &shrink_memcg_cb,NULL , &nr_to_walk); - scanned +=1 - nr_to_walk; + scanned += node_budget - nr_to_walk; } +/* Nothing was scanned: every LRU under @memcg was empty. */ if (!scanned)return -ENOENT ;
Signature · 1 line
2.34.1
Johannes Weiner
maintainerRe: [PATCH v3 2/2] mm/zswap: Support batch writeback in shrink_memcg()
Quoted 124 lines · Hao Jia
From: Hao Jia <[email protected]>
Currently,
shrink_memcg()writes back at most one entry per-node during its traversal. This makesshrink_worker()inefficient, as it must repeatedly re-entershrink_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, enabling batch writeback for both theshrink_worker()andzswap_store()paths. To prevent shrink unfairness across NUMA nodes caused by a shared global scan quota, limit scanning to up toSWAP_CLUSTER_MAXpages per node and write back any reclaimable entries found.Test Setup:
- Total memory: 32 GB, 1 NUMA node.
- zswap settings: accept_threshold_percent=50, shrinker_enabled=N.Test Case 1:
Set max_pool_percent=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 reclamation of this 4K 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 Patchedshrink_workerwakeups 5,363 169shrink_memcgcalls 11,373,201 350,703written_backpages 40,212 40,241zswap_store calls 161,190 163,753 store succeeded (ret=1) 102,743 117,183 store rejected (ret=0) 58,447 46,570 store reject rate ~36% ~28% pool_limit_hit delta 55,826 33,760 pswpout 98,659 86,811pswpin 2 0
Test Case 2:
We evaluated the following two sub-configurations using stress-ng inside a cgroup capped at memory.max=1G for 120 seconds:
Test Case 2a (max_pool_percent=1): Continuously triggers the global zswap pool limit, thereby waking upshrink_worker()to perform asynchronous shrinking.
Test Case 2b (zswap.max=320M, max_pool_percent=50): Continuously triggers the cgroup's zswap.max limit, thereby invoking synchronous shrinking.
Command executed for both setups: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'
Test Case 2a (max_pool_percent=1):
Baseline Patchedshrink_workerwakeups 5,640 1,308shrink_memcgcalls 8,481,500 3,140,972written_backpages 260 468,216zswap_storecalls 2,742,756 2,011,269
store succeeded (ret=1) 934,640 947,988
store rejected (ret=0) 1,808,116 1,063,281
store reject rate ~66% ~52%pool_limit_hit delta 1,181,310 196,882 pswpout 1,808,376 1,531,497 pswpin 4,288,497 3,635,365Test Case 2b (zswap.max=320M, max_pool_percent=50):
Baseline Patchedshrink_workerwakeups 0 0shrink_memcgcalls 687,608 54,002written_backpages 639,176 846,663zswap_storecalls 1,224,222 1,228,548
store succeeded (ret=1) 992,816 1,208,123
store rejected (ret=0) 231,431 20,425
store reject rate ~19% ~2%pool_limit_hitdelta 0 0
pswpout 870,745 867,360
pswpin 1,707,823 1,216,814Under identical workloads and runtimes, batched zswap shrinking exhibits a significant reduction in both
shrink_worker()wakeups andshrink_memcg()calls. Furthermore, the sharp drop in both pswpin andzswap_store()rejections demonstrates that batching zswap shrink operations effectively mitigateszswap_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]>Acked-by Yosry Ahmed <[email protected]>Acked-by Nhat Pham <[email protected]>Signed-off-by Hao Jia <[email protected]>--- mm/zswap.c | 30 ++++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-)mm/zswap.c +15
-0@@ -1275,6 +1275,21 @@ static struct shrinker *zswap_alloc_shrinker(void)return shrinker; } +/* +* Scan up to SWAP_CLUSTER_MAX pages on each per-node zswap LRU 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) {int nid, shrunk =0 , scanned =0 ;@@ -1290,13 +1305,24 @@ static int shrink_memcg(struct mem_cgroup *memcg)return -ENOENT ;
mm/zswap.c static int shrink_memcg(struct mem_cgroup *memcg) · line 1315
Quoted 10 lines
mm/zswap.c +8
-1for_each_node_state (nid,N_NORMAL_MEMORY ) { -unsigned long nr_to_walk =1 ; +unsigned long nr_to_walk, node_budget; + +/* +* Cap the scan at the per-node LRU length so each entry is +* scanned at most once per call. +*/ + node_budget =min (SWAP_CLUSTER_MAX , +list_lru_count_one (&zswap_list_lru, nid, memcg));
AFAICS you can just do unsigned long nr_to_walk = SWAP_CLUSTER_MAX.
__list_lru_walk_one() does a list_for_each_safe() that will exit the same way whether you hit !nr_to_walk or run out of items.
mm/zswap.c static int shrink_memcg(struct mem_cgroup *memcg) · line 1322
Quoted 8 lines
mm/zswap.c +4
-1+if (!node_budget) +continue ; + nr_to_walk = node_budget; shrunk +=list_lru_walk_one (&zswap_list_lru, nid, memcg, &shrink_memcg_cb,NULL , &nr_to_walk); - scanned +=1 - nr_to_walk; + scanned += node_budget - nr_to_walk;
scanned += SWAP_CLUSTER_MAX - nr_to_walk;
Re: [PATCH v3 2/2] mm/zswap: Support batch writeback in shrink_memcg()
Quoted 138 lines · Johannes Weiner
Quoted 124 lines · Hao Jia
From: Hao Jia <[email protected]>
Currently,
shrink_memcg()writes back at most one entry per-node during its traversal. This makesshrink_worker()inefficient, as it must repeatedly re-entershrink_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, enabling batch writeback for both theshrink_worker()andzswap_store()paths. To prevent shrink unfairness across NUMA nodes caused by a shared global scan quota, limit scanning to up toSWAP_CLUSTER_MAXpages per node and write back any reclaimable entries found.Test Setup:
- Total memory: 32 GB, 1 NUMA node.
- zswap settings: accept_threshold_percent=50, shrinker_enabled=N.Test Case 1:
Set max_pool_percent=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 reclamation of this 4K 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 Patchedshrink_workerwakeups 5,363 169shrink_memcgcalls 11,373,201 350,703written_backpages 40,212 40,241zswap_storecalls 161,190 163,753store succeeded (ret=1) 102,743 117,183 store rejected (ret=0) 58,447 46,570 store reject rate ~36% ~28% pool_limit_hit delta 55,826 33,760 pswpout 98,659 86,811pswpin 2 0
Test Case 2:
We evaluated the following two sub-configurations using stress-ng inside a cgroup capped at memory.max=1G for 120 seconds:
Test Case 2a (max_pool_percent=1): Continuously triggers the global zswap pool limit, thereby waking upshrink_worker()to perform asynchronous shrinking.
Test Case 2b (zswap.max=320M, max_pool_percent=50): Continuously triggers the cgroup's zswap.max limit, thereby invoking synchronous shrinking.
Command executed for both setups: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'
Test Case 2a (max_pool_percent=1):
Baseline Patchedshrink_workerwakeups 5,640 1,308shrink_memcgcalls 8,481,500 3,140,972written_backpages 260 468,216zswap_storecalls 2,742,756 2,011,269store succeeded (ret=1) 934,640 947,988 store rejected (ret=0) 1,808,116 1,063,281 store reject rate ~66% ~52% pool_limit_hit delta 1,181,310 196,882 pswpout 1,808,376 1,531,497 pswpin 4,288,497 3,635,365Test Case 2b (zswap.max=320M, max_pool_percent=50):
Baseline Patchedshrink_workerwakeups 0 0shrink_memcgcalls 687,608 54,002written_backpages 639,176 846,663zswap_storecalls 1,224,222 1,228,548store succeeded (ret=1) 992,816 1,208,123 store rejected (ret=0) 231,431 20,425 store reject rate ~19% ~2%
pool_limit_hitdelta 0 0
pswpout 870,745 867,360
pswpin 1,707,823 1,216,814Under identical workloads and runtimes, batched zswap shrinking exhibits a significant reduction in both
shrink_worker()wakeups andshrink_memcg()calls. Furthermore, the sharp drop in both pswpin andzswap_store()rejections demonstrates that batching zswap shrink operations effectively mitigateszswap_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]>Acked-by Yosry Ahmed <[email protected]>Acked-by Nhat Pham <[email protected]>Signed-off-by Hao Jia <[email protected]>--- mm/zswap.c | 30 ++++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-)mm/zswap.c +15
-0@@ -1275,6 +1275,21 @@ static struct shrinker *zswap_alloc_shrinker(void)return shrinker; } +/* +* Scan up to SWAP_CLUSTER_MAX pages on each per-node zswap LRU 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) {int nid, shrunk =0 , scanned =0 ;@@ -1290,13 +1305,24 @@ static int shrink_memcg(struct mem_cgroup *memcg)return -ENOENT ;mm/zswap.c static int shrink_memcg(struct mem_cgroup *memcg) · line 1315
Quoted 10 lines
mm/zswap.c +8
-1for_each_node_state (nid,N_NORMAL_MEMORY ) { -unsigned long nr_to_walk =1 ; +unsigned long nr_to_walk, node_budget; + +/* +* Cap the scan at the per-node LRU length so each entry is +* scanned at most once per call. +*/ + node_budget =min (SWAP_CLUSTER_MAX , +list_lru_count_one (&zswap_list_lru, nid, memcg));AFAICS you can just do unsigned long
nr_to_walk=SWAP_CLUSTER_MAX.
Quoted 2 lines
__list_lru_walk_one()does alist_for_each_safe()that will exit the same way whether you hit !nr_to_walk or run out of items.
Wouldn't it be better to ensure that each entry is scanned at most once per call, particularly when list_lru_count_one(nid) < SWAP_CLUSTER_MAX?
On one hand, this avoids scanning the same entry multiple times within a single pass. Since the second-chance algorithm rotates referenced entries to the tail of the LRU, entries on nodes with a large number of zswap entries require at least two shrink_memcg() calls to be written back, whereas entries on nodes with fewer entries might get written back in a single shrink_memcg() call instead. On the other hand, we also avoid spinning repeatedly on entries that fail writeback.
Thanks,
Hao
Quoted 11 lines
mm/zswap.c static int shrink_memcg(struct mem_cgroup *memcg) · line 1322
Quoted 9 lines
mm/zswap.c +4
-1+if (!node_budget) +continue ; + nr_to_walk = node_budget; shrunk +=list_lru_walk_one (&zswap_list_lru, nid, memcg, &shrink_memcg_cb,NULL , &nr_to_walk); - scanned +=1 - nr_to_walk; + scanned += node_budget - nr_to_walk;scanned +=
SWAP_CLUSTER_MAX-nr_to_walk;
Re: [PATCH v3 2/2] mm/zswap: Support batch writeback in shrink_memcg()
Quoted 148 lines · Hao Jia
Quoted 142 lines · Johannes Weiner
Quoted 128 lines · Hao Jia
From: Hao Jia <[email protected]>
Currently,
shrink_memcg()writes back at most one entry per-node during its traversal. This makesshrink_worker()inefficient, as it must repeatedly re-entershrink_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, enabling batch writeback for both theshrink_worker()andzswap_store()paths. To prevent shrink unfairness across NUMA nodes caused by a shared global scan quota, limit scanning to up toSWAP_CLUSTER_MAXpages per node and write back any reclaimable entries found.Test Setup:
- Total memory: 32 GB, 1 NUMA node.
- zswap settings: accept_threshold_percent=50, shrinker_enabled=N.Test Case 1:
Set max_pool_percent=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 reclamation of this 4K 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 Patchedshrink_workerwakeups 5,363 169shrink_memcgcalls 11,373,201 350,703written_backpages 40,212 40,241zswap_storecalls 161,190 163,753
store succeeded (ret=1) 102,743 117,183
store rejected (ret=0) 58,447 46,570
store reject rate ~36% ~28%pool_limit_hitdelta 55,826 33,760
pswpout 98,659 86,811
pswpin 2 0Test Case 2:
We evaluated the following two sub-configurations using stress-ng inside a cgroup capped at memory.max=1G for 120 seconds:
Test Case 2a (max_pool_percent=1): Continuously triggers the global zswap pool limit, thereby waking upshrink_worker()to perform asynchronous shrinking.
Test Case 2b (zswap.max=320M, max_pool_percent=50): Continuously triggers the cgroup's zswap.max limit, thereby invoking synchronous shrinking. Command executed for both setups:
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'Test Case 2a (max_pool_percent=1):
Baseline Patchedshrink_workerwakeups 5,640 1,308shrink_memcgcalls 8,481,500 3,140,972written_backpages 260 468,216zswap_storecalls 2,742,756 2,011,269 store succeeded (ret=1) 934,640 947,988 store rejected (ret=0) 1,808,116 1,063,281 store reject rate ~66% ~52%pool_limit_hitdelta 1,181,310 196,882 pswpout 1,808,376 1,531,497 pswpin 4,288,497 3,635,365 Test Case 2b (zswap.max=320M, max_pool_percent=50):
Baseline Patchedshrink_workerwakeups 0 0shrink_memcgcalls 687,608 54,002written_backpages 639,176 846,663zswap_storecalls 1,224,222 1,228,548 store succeeded (ret=1) 992,816 1,208,123 store rejected (ret=0) 231,431 20,425 store reject rate ~19% ~2%pool_limit_hitdelta 0 0 pswpout 870,745 867,360 pswpin 1,707,823 1,216,814Under identical workloads and runtimes, batched zswap shrinking exhibits a significant reduction in both
shrink_worker()wakeups andshrink_memcg()calls. Furthermore, the sharp drop in both pswpin andzswap_store()rejections demonstrates that batching zswap shrink operations effectively mitigateszswap_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]>Acked-by Yosry Ahmed <[email protected]>Acked-by Nhat Pham <[email protected]>Signed-off-by Hao Jia <[email protected]>---
mm/zswap.c| 30 ++++++++++++++++++++++++++++--1 file changed, 28 insertions(+), 2 deletions(-)mm/zswap.c +2
-0@@ -1275,6 +1275,21 @@ static struct shrinker *zswap_alloc_shrinker(void) return shrinker; } +/* +* Scan up to SWAP_CLUSTER_MAX pages on each per-node zswap LRU 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,-EAGAINif entries
+ * were scanned but none could be written back, or-ENOENTif @memcg has
+ * writeback disabled, is a zombie cgroup, or has empty zswap LRUs.
+ */
static intshrink_memcg(struct mem_cgroup *memcg)
{
int nid, shrunk = 0, scanned = 0;mm/zswap.c +0
-0@@ -1290,13 +1305,24 @@ static int shrink_memcg(struct mem_cgroup *memcg) return -ENOENT;mm/zswap.c static int shrink_memcg(struct mem_cgroup · line 1315
Quoted 10 lines
mm/zswap.c +8
-1for_each_node_state(nid, N_NORMAL_MEMORY) { - unsigned long nr_to_walk = 1; + unsigned long nr_to_walk, node_budget; + + /* + * Cap the scan at the per-node LRU length so each entry is + * scanned at most once per call. + */ + node_budget = min(SWAP_CLUSTER_MAX, + list_lru_count_one(&zswap_list_lru, nid, memcg));AFAICS you can just do unsigned long
nr_to_walk=SWAP_CLUSTER_MAX.Quoted 2 lines
__list_lru_walk_one()does alist_for_each_safe()that will exit the same way whether you hit !nr_to_walk or run out of items.
Quoted 7 lines
Wouldn't it be better to ensure that each entry is scanned at most once per call, particularly when
list_lru_count_one(nid)<SWAP_CLUSTER_MAX?On one hand, this avoids scanning the same entry multiple times within a single pass. Since the second-chance algorithm rotates referenced entries to the tail of the LRU, entries on nodes with a large number of zswap entries require at least two
shrink_memcg()calls to be written back, whereas entries on nodes with fewer entries might get written back in a singleshrink_memcg()call instead. On the other hand, we also avoid spinning repeatedly on entries that fail writeback.Thanks,
Hao
Johannes, Yosry, Nhats, any thoughts on this?
Quoted 10 lines
Quoted 10 lines
mm/zswap.c static int shrink_memcg(struct mem_cgroup *memcg) · line 1322
Quoted 8 lines
mm/zswap.c +4
-1+ if (!node_budget) + continue; + nr_to_walk = node_budget; shrunk += list_lru_walk_one(&zswap_list_lru, nid, memcg, &shrink_memcg_cb, NULL, &nr_to_walk); - scanned += 1 - nr_to_walk; + scanned += node_budget - nr_to_walk;scanned +=
SWAP_CLUSTER_MAX-nr_to_walk;
Nhat Pham
maintainerRe: [PATCH v3 2/2] mm/zswap: Support batch writeback in shrink_memcg()
Quoted 6 lines · Hao Jia
Quoted 2 lines · Hao Jia
Quoted 6 lines
Quoted 6 lines
On one hand, this avoids scanning the same entry multiple times within a single pass. Since the second-chance algorithm rotates referenced entries to the tail of the LRU, entries on nodes with a large number of zswap entries require at least two
shrink_memcg()calls to be written back, whereas entries on nodes with fewer entries might get written back in a singleshrink_memcg()call instead. On the other hand, we also
Same thing happens if you have more entries, no? Say you have 64 entries and 96 entries in node 1 and node 2, respectively. Then you need to go to round 3 to reclaim an entry in node 1, and round 4 for node 2. What's special about 1/2 rounds vs 3/4 rounds?
I think it's probably fine to just pass SWAP_CLUSTER_MAX as nr_to_walk
- we're applying equal amount of reclaim pressure to 2 nodes. Without prior information, seems like a reasonable strategy to me. I think what you did is already a pretty big improvements to status quo - we can always add more fine tuning on top of it later if needs be.
Quoted 1 line
Quoted 1 line
avoid spinning repeatedly on entries that fail writeback.
SWAP_CLUSTER_MAX is 32, so I assume it's not going to spin for a terribly long amount of time?
Re: [PATCH v3 1/2] mm/zswap: Fix global shrinker when memory cgroup is disabled
In reply to Johannes WeinerQuoted 85 lines · Johannes Weiner
Quoted 51 lines · Yosry Ahmed
Quoted 6 lines
Quoted 4 lines
Yeah, it isn't. Probably we should drop "Closes". I assume Hao added it because checkpatch annoyingly complains if you add "Reported-by" without "Closes", so Hao just linked to the thread where I pointed out the bug.
Yeah, checkpatch will complain if it's missing.
We often ignore checkpatch if it's unreasonable.
Quoted 38 lines
Quoted 22 lines
Quoted 2 lines
AI review asked a couple of questions:
https://sashiko.dev/#/patchset/[email protected]The review on patch #1 is something theoretical, we discussed it at length in previous versions.
For patch #2:
Quoted 11 lines
Does this batching logic break NUMA fairness?
Because
for_each_node_state()always starts from the lowest node ID and breaks when the scan budget is exhausted, subsequent calls toshrink_memcg()will restart at the lowest node ID again.If the lowest node (typically Node 0) consistently has enough items to exhaust the scan budget, wouldn't we exclusively evict pages from it while ignoring older pages on other nodes? Could this cause LRU inversion across nodes, keeping older pages in memory on Node 1 while hot pages on Node 0 are evicted?
Yes, unfairness is possible.
Quoted 9 lines
For global shrinking, it's probably not an issue. We reclaim until we hit the acceptance threshold and it's very unlikely this will happen before iterating all nodes (given that the batch size is 32 pages). However, with the
shrink_memcg()path, we only reclaim one batch, so there's a chance we'll always reclaim it from node 0.Maybe we should just drop the early bailout and accept potentially doing more writeback than needed. Hao, WDYT?
If we scan and attempt to write back
SWAP_CLUSTER_MAXzswap entries per node, it might lead to excessive writeback on machines with many NUMA nodes. Furthermore, I'm concerned about introducing higher latency in synchronous shrink paths like zswap_store()—especially on systems with a large number of NUMA nodes, where it could end up writing back hundreds of pages in a single call.Quoted 3 lines
Maybe we could do something like this instead? That way, in the worst-case scenario, it falls back to the baseline behavior without introducing any extra latency risks.
Quoted 6 lines
This basically errs on the side of honouring the batch size (under-reclaim) instead of doing more writeback, right?
I think I prefer erring on the side of doing more writeback, as it would ultimately result in less LRU inversion, and we are already doing writeback proactively during reclaim through the shrinker.
Yes, LRU inversions are worse than reclaiming a few extra pages from the cold tail of the LRU. It's not cumulative after all, it just means we take a bigger bite and can take more stores before the next shrink.
Quoted 9 lines
The other option we can explore is keeping the existing logic with the bailout, but start iterating the nodes at the folio's node in the
zswap_store()path. Basically pass an nid toshrink_memcg()and always start there. This will avoid always reclaiming from node 0, and the node of the folio being stored is more likely to be under pressure and need the writeback. I am not sure how complex this would be and whether it's worth doing.Johannes, Nhats, any thoughts on this?
What happens if simply we do a hard
SWAP_CLUSTER_MAXon each node?With cgroups, we put the zswap entry on the
list_lruhead of the node and memcg where the incoming page was.So if you're going after one specific cgroup, it's not like you *actually* reclaim 32 entries on each node of the system. You just write back from nodes where that cgroup has entries.
The global shrinker also does one cgroup at a time (
zswap_next_shrink) before checking the limit again, so that's fairly fine-grained too.
Quoted 7 lines
Without cgroups, you could have zswap entries on all nodes. But if you have so many nodes that this would constitute hundreds of pages, presumably that's still a drop in the bucket compared to overall memory capacity.
SWAP_CLUSTER_MAXisn't that big a number.So I don't think we need to get fancy. Just do
SWAP_CLUSTER_MAXon each node. Delete thatnr_to_walkcarry-over between them.
Thanks for the input! I've posted the new implementation—please take a look when you have time.
https://lore.kernel.org/all/[email protected]
Thanks,
Hao
PATCH v3 2/2 mm/zswap: Support batch writeback in shrink_memcg()
In reply to Hao JiaFrom: 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 setting the writeback batch size to SWAP_CLUSTER_MAX for both the shrink_worker() and zswap_store() paths.
Test Setup:
- Total memory: 32 GB.
- zswap settings: accept_threshold_percent=50, shrinker_enabled=N.
Test Case 1:
Set max_pool_percent=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 reclamation of this 4K 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 Patchedshrink_worker wakeups 5,363 169shrink_memcg calls 11,373,201 350,703written_back pages 40,212 40,241
zswap_store calls 161,190 163,753 store succeeded (ret=1) 102,743 117,183 store rejected (ret=0) 58,447 46,570 store reject rate ~36% ~28% pool_limit_hit delta 55,826 33,760 pswpout 98,659 86,811
pswpin 2 0
Test Case 2:
We evaluated the following two sub-configurations using stress-ng inside a cgroup capped at memory.max=1G for 120 seconds:
Test Case 2a (max_pool_percent=1): Continuously triggers the global zswap pool limit, thereby waking up shrink_worker() to perform asynchronous shrinking.
Test Case 2b (zswap.max=320M, max_pool_percent=50): Continuously triggers the cgroup's zswap.max limit, thereby invoking synchronous shrinking.
Command executed for both setups:
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'
Test Case 2a (max_pool_percent=1):
Baseline Patchedshrink_worker wakeups 5,640 1,308shrink_memcg calls 8,481,500 3,140,972written_back pages 260 468,216zswap_store calls 2,742,756 2,011,269
store succeeded (ret=1) 934,640 947,988
store rejected (ret=0) 1,808,116 1,063,281
store reject rate ~66% ~52%
pool_limit_hit delta 1,181,310 196,882 pswpout 1,808,376 1,531,497 pswpin 4,288,497 3,635,365
Test Case 2b (zswap.max=320M, max_pool_percent=50):
Baseline Patchedshrink_worker wakeups 0 0shrink_memcg calls 687,608 54,002written_back pages 639,176 846,663zswap_store calls 1,224,222 1,228,548
store succeeded (ret=1) 992,816 1,208,123
store rejected (ret=0) 231,431 20,425
store reject rate ~19% ~2%pool_limit_hit delta 0 0
pswpout 870,745 867,360
pswpin 1,707,823 1,216,814
Under identical workloads and runtimes, batched zswap shrinking exhibits a significant reduction in both shrink_worker() wakeups and shrink_memcg() calls. Furthermore, the sharp drop in both pswpin and zswap_store() rejections demonstrates that batching zswap shrink operations 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.
--- mm/zswap.c | 45 ++++++++++++++++++++++++++++++++++++++------- 1 file changed, 38 insertions(+), 7 deletions(-)
mm/zswap.c +38 -7
@@ -1275,9 +1275,25 @@ static struct shrinker *zswap_alloc_shrinker(void)return shrinker; } -static int shrink_memcg (struct mem_cgroup *memcg) +/* +* 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 +1306,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 +1400,7 @@ static void shrink_worker(struct work_struct *w) goto resched; } - ret =shrink_memcg (memcg); + ret =shrink_memcg (memcg,SWAP_CLUSTER_MAX );/* drop the extra reference */ mem_cgroup_put (memcg);
@@ -1493,7 +1524,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,SWAP_CLUSTER_MAX )) {mem_cgroup_put (memcg); goto put_objcg; }
Signature · 1 line
2.34.1