mm/zswap: Implement per-cgroup proactive writeback
Subsystems (10)
v5 cover mm/zswap: Implement per-cgroup proactive writeback
From: Hao Jia <[email protected]>
Zswap currently writes back pages to backing swap reactively, triggered either by the shrinker or by the pool reaching its size limit. Although proactive memory reclaim can automatically write back a portion of zswap pages via the shrinker, it cannot explicitly control the amount of writeback for a specific memory cgroup. Moreover, proactive memory reclaim may not always be triggered during a steady state.
In certain scenarios, it is desirable to trigger writeback in advance to free up memory. For example, users may want to prepare for an upcoming memory-intensive workload by flushing cold memory to the backing storage when the system is relatively idle.
This patch series introduces a "source=zswap" key to memory.reclaim cgroup interface, allowing users to proactively write back cold compressed data from zswap to the backing swap device. When specified, this key bypasses standard memory reclaim and exclusively performs proactive zswap writeback up to the requested budget. If omitted, the default reclaim behavior remains unchanged.
Example usage:
# Write back 10MB of compressed data from zswap to the backing swap echo "10M source=zswap" > memory.reclaim
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.
Patch 3: Extract a reusable writeback helper zswap_shrink_one_memcg() from
shrink_worker().
Patch 4: Extend the memory.reclaim cgroup v2 interface with a new
"source=zswap" key, allowing users to trigger proactive zswap writeback up to a requested budget.
Patch 5: Add the zswpwb_proactive_b stat to track the compressed bytes
of proactive writeback for better monitoring and tuning.
Patch 6: Add tests for zswap proactive writeback.
v4->v5:
- Add a new patch to fix missing global shrinker when memory cgroup is disabled.
- Simplify batch writeback logic in shrink_memcg() and improve comments.
- Refactor the writeback retry helper out of shrink_worker().
- Replace the "zswap_writeback_only" memory.reclaim key with a more extensible "source=zswap" key, leaving room for selecting other reclaim sources in the future.
v3->v4:
- Drop the per-memcg cursor and keep the root cgroup cursor (zswap_next_shrink) logic intact.
- Stick to using the zswap_writeback_only key, and change the proactive writeback size to use the compressed size.
- Consolidate and reuse the logic between shrink_worker() and shrink_memcg(). Enable batch writeback in the shrink_worker() path, while maintaining a low writeback budget in the zswap_store() path.
v2->v3:
- Align the return value of zswap_proactive_writeback() with memory.reclaim and update the corresponding documentation accordingly.
- Resolve conflicts in test_zswap.c on the mm-unstable branch.
- Enhance the zswap proactive writeback selftests to guard against potential future regressions.
v1->v2:
- As suggested by Yosry and Nhat, extend the memory.reclaim cgroup v2 interface with a "zswap_writeback_only" key instead of adding a new dedicated cgroup interface.
- Update the zswap documentation and add selftests for proactive writeback.
[v4] https://lore.kernel.org/all/[email protected]
[v3] https://lore.kernel.org/all/[email protected]
[v2] https://lore.kernel.org/all/[email protected]
[v1] https://lore.kernel.org/all/[email protected]
Hao Jia (6):
mm/zswap: Fix global shrinker when memory cgroup is disabled mm/zswap: Support batch writeback in shrink_memcg() mm/zswap: Extract a reusable writeback helper from shrink_worker() mm/zswap: Implement proactive writeback
mm/zswap: Add per-memcg stat for proactive writeback selftests/cgroup: Add tests for zswap proactive writeback
Documentation/admin-guide/cgroup-v2.rst | 22 +- Documentation/admin-guide/mm/zswap.rst | 11 +- include/linux/memcontrol.h | 1 + include/linux/zswap.h | 7 + mm/memcontrol.c | 3 + mm/vmscan.c | 24 +- mm/zswap.c | 239 +++++++++++++++----- tools/testing/selftests/cgroup/test_zswap.c | 161 ++++++++++++- 8 files changed, 410 insertions(+), 58 deletions(-)
Signature · 1 line
2.34.1
PATCH v5 1/6 mm/zswap: Fix global shrinker when memory cgroup is disabled
From: Hao Jia <[email protected]>
When memory cgroup is disabled, 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. Stop the loop once shrink_memcg() reports -ENOENT, since the root LRU is the only target and -ENOENT means it has been exhausted.
--- mm/zswap.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)
mm/zswap.c +14 -2
@@ -1356,7 +1356,12 @@ static void shrink_worker(struct work_struct *w) }while (memcg && !mem_cgroup_tryget_online (memcg));spin_unlock (&zswap_shrink_lock); -if (!memcg) { +/* +* Reaching a NULL memcg means a full hierarchy pass completed. +* Exclude the memcg-disabled case, where it is always NULL, and +* fall through to shrink the root LRU directly. +*/ +if (!memcg && !mem_cgroup_disabled ()) {/* * Continue shrinking without incrementing failures if * we found candidate memcgs in the last tree walk.
@@ -1378,8 +1383,15 @@ static void shrink_worker(struct work_struct *w)* with pages in zswap. Skip this without incrementing attempts * and failures. */ -if (ret == -ENOENT ) +if (ret == -ENOENT ) { +/* +* With memcg disabled the root LRU is the only target, so +* we should abort if it has no writeback-candidate pages. +*/ +if (mem_cgroup_disabled ()) +break ;continue ; + } ++attempts;if (ret && ++failures ==MAX_RECLAIM_RETRIES )
Signature · 1 line
2.34.1
Nhat Pham
maintainerRe: [PATCH v5 1/6] mm/zswap: Fix global shrinker when memory cgroup is disabled
Quoted 13 lines · Hao Jia
From: Hao Jia <[email protected]>
When memory cgroup is disabled,
mem_cgroup_iter()always returns NULL. Therefore, the global shrinkershrink_worker()always takes the !memcg branch. AfterMAX_RECLAIM_RETRIESempty 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. Stop the loop once
shrink_memcg()reports-ENOENT, since the root LRU is the only target and-ENOENTmeans it has been exhausted.
Quoted 5 lines
Fixes a65b0e7607cc ("zswap: make shrinking memcg-aware")Reported-by Yosry Ahmed <[email protected]>Closes https://lore.kernel.org/all/CAO9r8zPVzMKFbCixxD-qgtRrkFxWVrHiZZeLc=eyTPKPVQgX4g@mail.gmail.com open hereSigned-off-by Hao Jia <[email protected]>
Ah good catch.
Quoted 12 lines
--- mm/zswap.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)mm/zswap.c +0
-0@@ -1356,7 +1356,12 @@ static void shrink_worker(struct work_struct *w) }while (memcg && !mem_cgroup_tryget_online (memcg));spin_unlock (&zswap_shrink_lock);
mm/zswap.c static void shrink_worker(struct work_struct *w) · line 1367
Quoted 10 lines
mm/zswap.c +6
-1-if (!memcg) { +/* +* Reaching a NULL memcg means a full hierarchy pass completed. +* Exclude the memcg-disabled case, where it is always NULL, and +* fall through to shrink the root LRU directly. +*/ +if (!memcg && !mem_cgroup_disabled ()) {/* * Continue shrinking without incrementing failures if * we found candidate memcgs in the last tree walk.
nit: I wonder if we can just merge this comment with the new comment you just added.
mm/zswap.c static void shrink_worker(struct work_struct *w) · line 1392
Quoted 12 lines
mm/zswap.c +7
-1@@ -1378,8 +1383,15 @@ static void shrink_worker(struct work_struct *w)* with pages in zswap. Skip this without incrementing attempts * and failures. */ -if (ret == -ENOENT ) +if (ret == -ENOENT ) { +/* +* With memcg disabled the root LRU is the only target, so +* we should abort if it has no writeback-candidate pages. +*/ +if (mem_cgroup_disabled ()) +break ;
Hmm do we need to do this? Consider a system with cgroup enabled but with just one cgroup (root?). The behavior would just be trying that cgroup for MAX_RECLAIM_RETRIES failure attempts, correct?
In that case, we don't need to do this check, and we would get the same behavior. The loop would terminate after MAX_RECLAIM_RETRIES :)
Could you fact-check me? :)
Re: [PATCH v5 1/6] mm/zswap: Fix global shrinker when memory cgroup is disabled
Quoted 36 lines · Nhat Pham
Quoted 13 lines · Hao Jia
From: Hao Jia <[email protected]>
When memory cgroup is disabled,
mem_cgroup_iter()always returns NULL. Therefore, the global shrinkershrink_worker()always takes the !memcg branch. AfterMAX_RECLAIM_RETRIESempty 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. Stop the loop once
shrink_memcg()reports-ENOENT, since the root LRU is the only target and-ENOENTmeans it has been exhausted.Quoted 5 lines
Fixes a65b0e7607cc ("zswap: make shrinking memcg-aware")Reported-by Yosry Ahmed <[email protected]>Closes https://lore.kernel.org/all/CAO9r8zPVzMKFbCixxD-qgtRrkFxWVrHiZZeLc=eyTPKPVQgX4g@mail.gmail.com open hereSigned-off-by Hao Jia <[email protected]>Ah good catch.
Quoted 12 lines
--- mm/zswap.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)mm/zswap.c +0
-0@@ -1356,7 +1356,12 @@ static void shrink_worker(struct work_struct *w) }while (memcg && !mem_cgroup_tryget_online (memcg));spin_unlock (&zswap_shrink_lock);
Quoted 13 lines
mm/zswap.c static void shrink_worker(struct work_struct *w) · line 1367
Quoted 10 lines
mm/zswap.c +6
-1-if (!memcg) { +/* +* Reaching a NULL memcg means a full hierarchy pass completed. +* Exclude the memcg-disabled case, where it is always NULL, and +* fall through to shrink the root LRU directly. +*/ +if (!memcg && !mem_cgroup_disabled ()) {/* * Continue shrinking without incrementing failures if * we found candidate memcgs in the last tree walk. nit: I wonder if we can just merge this comment with the new comment you just added.
Updated. Please see below.
Quoted 18 lines
mm/zswap.c static void shrink_worker(struct work_struct *w) · line 1392
Quoted 13 lines
mm/zswap.c +7
-1@@ -1378,8 +1383,15 @@ static void shrink_worker(struct work_struct *w)* with pages in zswap. Skip this without incrementing attempts * and failures. */ -if (ret == -ENOENT ) +if (ret == -ENOENT ) { +/* +* With memcg disabled the root LRU is the only target, so +* we should abort if it has no writeback-candidate pages. +*/ +if (mem_cgroup_disabled ()) +break ;Hmm do we need to do this? Consider a system with cgroup enabled but with just one cgroup (root?). The behavior would just be trying that cgroup for
MAX_RECLAIM_RETRIESfailure attempts, correct?
Quoted 4 lines
In that case, we don't need to do this check, and we would get the same behavior. The loop would terminate after
MAX_RECLAIM_RETRIES:)Could you fact-check me? :)
Exactly. When memcg is disabled, shrink_memcg() returns -ENOENT only if the root LRU is empty. An empty root LRU implies that the total pages have already dropped below the threshold (thr). At this point, the loop safely terminates because of the zswap_total_pages() <= thr check. In all other cases (where shrink_memcg() returns anything other than -ENOENT), the loop will eventually exit either by hitting the MAX_RECLAIM_RETRIES limit or when zswap_total_pages() <= thr.
How about something like this? If there are no objections, I'll fold this into the next version.
mm/zswap: Fix global shrinker when memory cgroup is disabled
When memory cgroup is disabled, 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.
The loop
then safely bails out via the zswap_total_pages() <= thr 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.
Closes:
https://lore.kernel.org/all/CAO9r8zPVzMKFbCixxD-qgtRrkFxWVrHiZZeLc=eyTPKPVQgX4g@mail.gmail.com
mm/zswap.c +6 -5
@@ -1361,11 +1361,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 pass found no candidates. +*/ +if (!memcg && !mem_cgroup_disabled ()) {if (!attempts && ++failures ==MAX_RECLAIM_RETRIES )break ;
Thanks,
Hao
Re: [PATCH v5 1/6] mm/zswap: Fix global shrinker when memory cgroup is disabled
Quoted 22 lines
How about something like this? If there are no objections, I'll fold this into the next version.
mm/zswap: Fix global shrinker when memory cgroup is disabled
When memory cgroup is disabled,
mem_cgroup_iter()always returns NULL. Therefore, the global shrinkershrink_worker()always takes the !memcg branch. AfterMAX_RECLAIM_RETRIESempty 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-ENOENTwhen the root LRU is empty, which means the total pages are already below thr.
The loop
then safely bails out via thezswap_total_pages()<= thr 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 6 lines
Fixes a65b0e7607cc ("zswap: make shrinking memcg-aware")Reported-by Yosry Ahmed <[email protected]>Closes:
https://lore.kernel.org/all/CAO9r8zPVzMKFbCixxD-qgtRrkFxWVrHiZZeLc=eyTPKPVQgX4g@mail.gmail.comSigned-off-by Hao Jia <[email protected]>
Feel free to add:
A small nit below.
Quoted 17 lines
mm/zswap.c +0
-3@@ -1361,11 +1361,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
mm/zswap.c +2
-2-* we found candidate memcgs in the last tree walk. -*/ +/* +* A NULL memcg ends a full hierarchy pass (except when
Quoted 4 lines
memcg is
+ * disabled, where it is always NULL: fall through to
the root LRU).
+ * Count a failure only if the pass found no candidates.
I think "last pass" is clearer than just "pass" here?
Quoted 7 lines
mm/zswap.c +2
-0+*/ +if (!memcg && !mem_cgroup_disabled ()) {if (!attempts && ++failures ==MAX_RECLAIM_RETRIES )break ;Thanks,
Hao
Re: [PATCH v5 1/6] mm/zswap: Fix global shrinker when memory cgroup is disabled
Quoted 29 lines · Yosry Ahmed
Quoted 22 lines
How about something like this? If there are no objections, I'll fold this into the next version.
mm/zswap: Fix global shrinker when memory cgroup is disabled
When memory cgroup is disabled,
mem_cgroup_iter()always returns NULL. Therefore, the global shrinkershrink_worker()always takes the !memcg branch. AfterMAX_RECLAIM_RETRIESempty 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-ENOENTwhen the root LRU is empty, which means the total pages are already below thr.
The loop
then safely bails out via thezswap_total_pages()<= thr 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 6 lines
Fixes a65b0e7607cc ("zswap: make shrinking memcg-aware")Reported-by Yosry Ahmed <[email protected]>Closes:
https://lore.kernel.org/all/CAO9r8zPVzMKFbCixxD-qgtRrkFxWVrHiZZeLc=eyTPKPVQgX4g@mail.gmail.comSigned-off-by Hao Jia <[email protected]>
Quoted 3 lines
Feel free to add:
Acked-by Yosry Ahmed <[email protected]>
Thank you for taking the time to review this.
Quoted 12 lines
A small nit below.
Quoted 9 lines
mm/zswap.c +0
-0@@ -1361,11 +1361,12 @@ static void shrink_worker(struct work_struct *w) }while (memcg && !mem_cgroup_tryget_online (memcg));spin_unlock (&zswap_shrink_lock);
Quoted 14 lines
Quoted 8 lines
mm/zswap.c +0
-3-if (!memcg) { -/* -* Continue shrinking without incrementing failures if
mm/zswap.c +2
-2-* we found candidate memcgs in the last tree walk. -*/ +/* +* A NULL memcg ends a full hierarchy pass (except when Quoted 4 lines
memcg is
+ * disabled, where it is always NULL: fall through to
the root LRU).
+ * Count a failure only if the pass found no candidates.I think "last pass" is clearer than just "pass" here?
Will do.
Thanks,
Hao
Re: [PATCH v5 1/6] mm/zswap: Fix global shrinker when memory cgroup is disabled
In reply to Hao JiaQuoted 23 lines · Hao Jia
mm/zswap.c +0
-3@@ -1361,11 +1361,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
mm/zswap.c +2
-2-* 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).
mm/zswap.c static void shrink_worker(struct work_struct *w) · line 1371
Quoted 5 lines
mm/zswap.c +3
-0+* Count a failure only if the pass found no candidates. +*/ +if (!memcg && !mem_cgroup_disabled ()) {if (!attempts && ++failures ==MAX_RECLAIM_RETRIES )break ;
With Yosry's suggestion:
PATCH v5 2/6 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.
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.
Additionally, to prepare for future proactive writeback, update the return value semantics of shrink_memcg(): a positive value now represents the actual number of compressed bytes written back, 0 indicates that candidates existed but no writeback succeeded, and a negative value represents an error code.
--- mm/zswap.c | 89 ++++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 69 insertions(+), 20 deletions(-)
mm/zswap.c +69 -20
@@ -160,6 +160,11 @@ struct zswap_pool {char tfm_name[CRYPTO_MAX_ALG_NAME ]; }; +struct zswap_shrink_walk_arg { +unsigned long bytes_written; +bool encountered_page_in_swapcache; +}; +/* Global LRU lists shared by all zswap pools. */ static struct list_lru zswap_list_lru;
@@ -1089,8 +1094,9 @@ static enum lru_status shrink_memcg_cb(struct list_head *item, struct list_lru_o void *arg) { struct zswap_entry *entry = container_of(item, struct zswap_entry, lru); -bool *encountered_page_in_swapcache = (bool *)arg; +struct zswap_shrink_walk_arg *walk_arg = arg;swp_entry_t swpentry; +unsigned int length;enum lru_status ret =LRU_REMOVED_RETRY ;int writeback_result;
@@ -1133,10 +1139,11 @@ static enum lru_status shrink_memcg_cb(struct list_head *item, struct list_lru_o/* * Once the lru lock is dropped, the entry might get freed. The -* swpentry is copied to the stack, and entry isn't deref'd again -* until the entry is verified to still be alive in the tree. +* needed fields are copied to the stack, and entry isn't deref'd +* again until it is verified to still be alive in the tree. */ swpentry = entry->swpentry; + length = entry->length;/* * It's safe to drop the lock here because we return either
@@ -1155,12 +1162,13 @@ static enum lru_status shrink_memcg_cb(struct list_head *item, struct list_lru_o* into the warmer region. We should terminate shrinking (if we're in the dynamic * shrinker context). */ -if (writeback_result == -EEXIST && encountered_page_in_swapcache) { +if (writeback_result == -EEXIST ) { ret =LRU_STOP ; - *encountered_page_in_swapcache = true; + walk_arg->encountered_page_in_swapcache = true; } }else { zswap_written_back_pages++; + walk_arg->bytes_written += length; }return ret;
@@ -1169,8 +1177,11 @@ static enum lru_status shrink_memcg_cb(struct list_head *item, struct list_lru_ostatic unsigned long zswap_shrinker_scan (struct shrinker *shrinker,struct shrink_control *sc) { +struct zswap_shrink_walk_arg walk_arg = { + .bytes_written =0 , + .encountered_page_in_swapcache = false, + };unsigned long shrink_ret; -bool encountered_page_in_swapcache = false;if (!zswap_shrinker_enabled || !mem_cgroup_zswap_writeback_enabled (sc->memcg)) {
@@ -1179,9 +1190,9 @@ static unsigned long zswap_shrinker_scan(struct shrinker *shrinker, } shrink_ret =list_lru_shrink_walk (&zswap_list_lru, sc, &shrink_memcg_cb, - &encountered_page_in_swapcache); + &walk_arg); -if (encountered_page_in_swapcache) +if (walk_arg.encountered_page_in_swapcache)return SHRINK_STOP ;return shrink_ret ? shrink_ret :SHRINK_STOP ;
@@ -1275,9 +1286,31 @@ 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: The number of compressed bytes written back (>= 0), or -ENOENT +* if @memcg has writeback disabled, is a zombie cgroup, or has empty +* zswap LRUs. +*/ +static long shrink_memcg (struct mem_cgroup *memcg,unsigned long nr_to_scan) { -int nid, shrunk =0 , scanned =0 ; +struct zswap_shrink_walk_arg walk_arg = { + .bytes_written =0 , + .encountered_page_in_swapcache = false, + }; +unsigned long nr_remaining = nr_to_scan; +int nid;if (!mem_cgroup_zswap_writeback_enabled (memcg))return -ENOENT ;
@@ -1290,24 +1323,40 @@ 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; - shrunk +=list_lru_walk_one (&zswap_list_lru, nid, memcg, - &shrink_memcg_cb,NULL , &nr_to_walk); - scanned +=1 - 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; +list_lru_walk_one (&zswap_list_lru, nid, memcg, &shrink_memcg_cb, + &walk_arg, &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 ; +return walk_arg.bytes_written; }static void shrink_worker (struct work_struct *w) {struct mem_cgroup *memcg; -int ret, failures =0 , attempts =0 ; +int failures =0 , attempts =0 ;unsigned long thr; +long ret;/* Reclaim down to the accept threshold */ thr =zswap_accept_thr_pages ();
@@ -1373,7 +1422,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);
@@ -1394,7 +1443,7 @@ static void shrink_worker(struct work_struct *w) } ++attempts; -if (ret && ++failures ==MAX_RECLAIM_RETRIES ) +if (ret <=0 && ++failures ==MAX_RECLAIM_RETRIES )break ; resched:cond_resched ();
@@ -1504,7 +1553,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,num_node_state (N_NORMAL_MEMORY )) <=0 ) {mem_cgroup_put (memcg); goto put_objcg; }
Signature · 1 line
2.34.1
Yosry Ahmed
maintainerRe: [PATCH v5 2/6] mm/zswap: Support batch writeback in shrink_memcg()
Quoted 207 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.To address this, extend
shrink_memcg()and rewrite its LRU iteration logic to support batch writeback. Introduce thenr_to_scanparameter to bound how many pages are scanned per call. This enables batch writeback in theshrink_worker()path, while maintaining a low scan budget in thezswap_store()path.Additionally, to prepare for future proactive writeback, update the return value semantics of
shrink_memcg(): a positive value now represents the actual number of compressed bytes written back, 0 indicates that candidates existed but no writeback succeeded, and a negative value represents an error code.Suggested-by Yosry Ahmed <[email protected]>Signed-off-by Hao Jia <[email protected]>--- mm/zswap.c | 89 ++++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 69 insertions(+), 20 deletions(-)mm/zswap.c +68
-19@@ -160,6 +160,11 @@ struct zswap_pool {char tfm_name[CRYPTO_MAX_ALG_NAME ]; }; +struct zswap_shrink_walk_arg { +unsigned long bytes_written; +bool encountered_page_in_swapcache; +}; +/* Global LRU lists shared by all zswap pools. */ static struct list_lru zswap_list_lru;@@ -1089,8 +1094,9 @@ static enum lru_status shrink_memcg_cb(struct list_head *item, struct list_lru_o void *arg) { struct zswap_entry *entry = container_of(item, struct zswap_entry, lru); -bool *encountered_page_in_swapcache = (bool *)arg; +struct zswap_shrink_walk_arg *walk_arg = arg;swp_entry_t swpentry; +unsigned int length;enum lru_status ret =LRU_REMOVED_RETRY ;int writeback_result;@@ -1133,10 +1139,11 @@ static enum lru_status shrink_memcg_cb(struct list_head *item, struct list_lru_o/* * Once the lru lock is dropped, the entry might get freed. The -* swpentry is copied to the stack, and entry isn't deref'd again -* until the entry is verified to still be alive in the tree. +* needed fields are copied to the stack, and entry isn't deref'd +* again until it is verified to still be alive in the tree. */ swpentry = entry->swpentry; + length = entry->length;/* * It's safe to drop the lock here because we return either @@ -1155,12 +1162,13 @@ static enum lru_status shrink_memcg_cb(struct list_head *item, struct list_lru_o* into the warmer region. We should terminate shrinking (if we're in the dynamic * shrinker context). */ -if (writeback_result == -EEXIST && encountered_page_in_swapcache) { +if (writeback_result == -EEXIST ) { ret =LRU_STOP ; - *encountered_page_in_swapcache = true; + walk_arg->encountered_page_in_swapcache = true; } }else { zswap_written_back_pages++; + walk_arg->bytes_written += length; }return ret;@@ -1169,8 +1177,11 @@ static enum lru_status shrink_memcg_cb(struct list_head *item, struct list_lru_ostatic unsigned long zswap_shrinker_scan (struct shrinker *shrinker,struct shrink_control *sc) { +struct zswap_shrink_walk_arg walk_arg = { + .bytes_written =0 , + .encountered_page_in_swapcache = false, + };unsigned long shrink_ret; -bool encountered_page_in_swapcache = false;if (!zswap_shrinker_enabled || !mem_cgroup_zswap_writeback_enabled (sc->memcg)) {@@ -1179,9 +1190,9 @@ static unsigned long zswap_shrinker_scan(struct shrinker *shrinker, } shrink_ret =list_lru_shrink_walk (&zswap_list_lru, sc, &shrink_memcg_cb, - &encountered_page_in_swapcache); + &walk_arg); -if (encountered_page_in_swapcache) +if (walk_arg.encountered_page_in_swapcache)return SHRINK_STOP ;return shrink_ret ? shrink_ret :SHRINK_STOP ;@@ -1275,9 +1286,31 @@ 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: The number of compressed bytes written back (>= 0), or -ENOENT +* if @memcg has writeback disabled, is a zombie cgroup, or has empty +* zswap LRUs. +*/ +static long shrink_memcg (struct mem_cgroup *memcg,unsigned long nr_to_scan) { -int nid, shrunk =0 , scanned =0 ; +struct zswap_shrink_walk_arg walk_arg = { + .bytes_written =0 , + .encountered_page_in_swapcache = false, + }; +unsigned long nr_remaining = nr_to_scan; +int nid;if (!mem_cgroup_zswap_writeback_enabled (memcg))return -ENOENT ;@@ -1290,24 +1323,40 @@ 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; - shrunk +=list_lru_walk_one (&zswap_list_lru, nid, memcg, - &shrink_memcg_cb,NULL , &nr_to_walk); - scanned +=1 - 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; +list_lru_walk_one (&zswap_list_lru, nid, memcg, &shrink_memcg_cb, + &walk_arg, &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 ; +return walk_arg.bytes_written; }static void shrink_worker (struct work_struct *w) {struct mem_cgroup *memcg; -int ret, failures =0 , attempts =0 ; +int failures =0 , attempts =0 ;unsigned long thr; +long ret;/* Reclaim down to the accept threshold */ thr =zswap_accept_thr_pages ();@@ -1373,7 +1422,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);@@ -1394,7 +1443,7 @@ static void shrink_worker(struct work_struct *w) } ++attempts; -if (ret && ++failures ==MAX_RECLAIM_RETRIES ) +if (ret <=0 && ++failures ==MAX_RECLAIM_RETRIES )break ; resched:cond_resched ();
mm/zswap.c bool zswap_store(struct folio *folio) · line 1556
Quoted 5 lines
mm/zswap.c +1
-1@@ -1504,7 +1553,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,num_node_state (N_NORMAL_MEMORY )) <=0 ) {
Why not just 1?
I guess the current behavior will try each node. But this doesn't really match it, as we may reclaim everything from the first node. Right?
I think it's probably fine to just do 1 here, fairness is not the main concern in this code path, we're really just trying to free up some space to free up some space for the incoming page. I doubt these limits are actually being used extensively anyway, so we can revisit this later if needed.
Quoted 6 lines
mm/zswap.c +0
-0mem_cgroup_put (memcg); goto put_objcg; }Signature · 1 line
2.34.1
Re: [PATCH v5 2/6] mm/zswap: Support batch writeback in shrink_memcg()
Quoted 40 lines · Yosry Ahmed
Quoted 30 lines · Hao Jia
static void shrink_worker(struct work_struct *w) { struct mem_cgroup *memcg;mm/zswap.c +4
-3-int ret, failures =0 , attempts =0 ; +int failures =0 , attempts =0 ;unsigned long thr; +long ret;/* Reclaim down to the accept threshold */ thr =zswap_accept_thr_pages ();@@ -1373,7 +1422,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);@@ -1394,7 +1443,7 @@ static void shrink_worker(struct work_struct *w) } ++attempts; -if (ret && ++failures ==MAX_RECLAIM_RETRIES ) +if (ret <=0 && ++failures ==MAX_RECLAIM_RETRIES )break ; resched:cond_resched ();mm/zswap.c bool zswap_store(struct folio *folio) · line 1556
Quoted 5 lines
mm/zswap.c +1
-1@@ -1504,7 +1553,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,num_node_state (N_NORMAL_MEMORY )) <=0 ) {Why not just 1?
Quoted 3 lines
I guess the current behavior will try each node. But this doesn't really match it, as we may reclaim everything from the first node. Right?
Yes, it just keeps the number the same as before, but the behavior is not exactly the same.
Quoted 5 lines
I think it's probably fine to just do 1 here, fairness is not the main concern in this code path, we're really just trying to free up some space to free up some space for the incoming page. I doubt these limits are actually being used extensively anyway, so we can revisit this later if needed.
Okay, I'll do this in the next version.
Thanks,
Hao
Quoted 7 lines
Quoted 7 lines
mm/zswap.c +0
-0mem_cgroup_put (memcg); goto put_objcg; }Signature · 1 line
2.34.1
PATCH v5 3/6 mm/zswap: Extract a reusable writeback helper from shrink_worker()
In reply to Hao JiaFrom: Hao Jia <[email protected]>
Extract a reusable writeback helper zswap_shrink_one_memcg() from shrink_worker(). This helper will be reused by the upcoming proactive writeback feature.
zswap_shrink_one_memcg() takes one step of a memcg-tree writeback walk driven by the caller's iterator. Consequently, shrink_worker() now only needs to calculate the acceptance threshold, drive its own iteration based on this helper, and abort the walk when zswap_shrink_one_memcg() returns -EBUSY.
--- mm/zswap.c | 118 +++++++++++++++++++++++++++++++---------------------- 1 file changed, 69 insertions(+), 49 deletions(-)
mm/zswap.c +69 -49
@@ -1351,12 +1351,70 @@ static long shrink_memcg(struct mem_cgroup *memcg, unsigned long nr_to_scan)return walk_arg.bytes_written; } +/* Track progress of a memcg-tree writeback walk. */ +struct zswap_shrink_state { +int scans; +int failures; +}; + +/* +* Take one step of a memcg-tree writeback walk driven by the caller's +* iterator, and fold the result into @s, the retry bookkeeping shared +* across steps. @memcg is the iterator's current memcg, or NULL once +* it has wrapped around after a full pass over the tree. +* +* The function returns -EBUSY to signal the caller to abort the walk after +* encountering either of the following MAX_RECLAIM_RETRIES times: +* - No writeback-candidate memcgs were found in a memcg tree walk. +* - Shrinking a writeback-candidate memcg failed. +* +* Return: The number of compressed bytes written back (>= 0), or -EBUSY +* when the caller should abort the walk. +*/ +static long zswap_shrink_one_memcg (struct mem_cgroup *memcg, +struct zswap_shrink_state *s) +{ +long shrunk; + +/* +* Reaching a NULL memcg means a full hierarchy pass completed. +* Exclude the memcg-disabled case, where it is always NULL, and +* fall through to shrink the root LRU directly. +*/ +if (!memcg && !mem_cgroup_disabled ()) { +/* +* Continue shrinking without incrementing failures if we found +* candidate memcgs in the last tree walk. +*/ +if (!s->scans && ++s->failures ==MAX_RECLAIM_RETRIES ) +return -EBUSY ; + s->scans =0 ; +return 0 ; + } + + shrunk =shrink_memcg (memcg,NR_ZSWAP_WB_BATCH ); + +/* +* There are no writeback-candidate pages in the memcg. With memcg +* enabled this is not an issue as long as we can find another memcg +* with pages in zswap, so skip without counting it as a candidate. +* With memcg disabled the root LRU is the only target, so we should +* abort if it has no writeback-candidate pages. +*/ +if (shrunk == -ENOENT ) +return mem_cgroup_disabled () ? -EBUSY :0 ; + s->scans++; + +if (shrunk <=0 && ++s->failures ==MAX_RECLAIM_RETRIES ) +return -EBUSY ; + +return shrunk; +} +static void shrink_worker (struct work_struct *w) { -struct mem_cgroup *memcg; -int failures =0 , attempts =0 ; +struct zswap_shrink_state s = {};unsigned long thr; -long ret;/* Reclaim down to the accept threshold */ thr =zswap_accept_thr_pages ();
@@ -1367,11 +1425,6 @@ static void shrink_worker(struct work_struct *w)* writeback-disabled memcgs (memory.zswap.writeback=0) are not * candidates for shrinking. * -* Shrinking will be aborted if we encounter the following -* MAX_RECLAIM_RETRIES times: -* - No writeback-candidate memcgs found in a memcg tree walk. -* - Shrinking a writeback-candidate memcg failed. -* * We save iteration cursor memcg into zswap_next_shrink, * which can be modified by the offline memcg cleaner * zswap_memcg_offline_cleanup().
@@ -1386,7 +1439,11 @@ static void shrink_worker(struct work_struct *w)* offline memcg left in zswap_next_shrink will hold the reference * until the next run of shrink_worker(). */ - do { +while (zswap_total_pages () > thr) { +struct mem_cgroup *memcg; +long ret; + +cond_resched ();/* * Start shrinking from the next memcg after zswap_next_shrink. * When the offline cleaner has already advanced the cursor,
@@ -1405,49 +1462,12 @@ static void shrink_worker(struct work_struct *w) }while (memcg && !mem_cgroup_tryget_online (memcg));spin_unlock (&zswap_shrink_lock); -/* -* Reaching a NULL memcg means a full hierarchy pass completed. -* Exclude the memcg-disabled case, where it is always NULL, and -* fall through to shrink the root LRU directly. -*/ -if (!memcg && !mem_cgroup_disabled ()) { -/* -* Continue shrinking without incrementing failures if -* we found candidate memcgs in the last tree walk. -*/ -if (!attempts && ++failures ==MAX_RECLAIM_RETRIES ) -break ; - - attempts =0 ; - goto resched; - } - - ret =shrink_memcg (memcg,NR_ZSWAP_WB_BATCH ); + ret =zswap_shrink_one_memcg (memcg, &s);/* drop the extra reference */ mem_cgroup_put (memcg); - -/* -* There are no writeback-candidate pages in the memcg. -* This is not an issue as long as we can find another memcg -* with pages in zswap. Skip this without incrementing attempts -* and failures. -*/ -if (ret == -ENOENT ) { -/* -* With memcg disabled the root LRU is the only target, so -* we should abort if it has no writeback-candidate pages. -*/ -if (mem_cgroup_disabled ()) -break ; -continue ; - } - ++attempts; - -if (ret <=0 && ++failures ==MAX_RECLAIM_RETRIES ) +if (ret == -EBUSY )break ; -resched: -cond_resched (); - }while (zswap_total_pages () > thr); + } }/*********************************
Signature · 1 line
2.34.1
PATCH v5 4/6 mm/zswap: Implement proactive writeback
In reply to Hao JiaFrom: Hao Jia <[email protected]>
Zswap currently writes back pages to backing swap reactively, triggered either by the shrinker or when the pool reaches its size limit. There is no mechanism to control the amount of writeback for a specific memory cgroup. However, users may want to proactively write back zswap pages, e.g., to free up memory for other applications or to prepare for memory-intensive workloads.
Introduce a "source=" key to the memory.reclaim cgroup interface, currently accepting the single value "zswap". When set to "zswap", it bypasses standard memory reclaim and exclusively performs proactive zswap writeback up to the requested budget. If omitted, the default reclaim behavior remains unchanged.
Example usage:
# Write back 10MB of compressed data from zswap to the backing swap echo "10M source=zswap" > memory.reclaim
Note that the actual amount of compressed data written back may be less than requested due to the zswap second-chance algorithm: referenced entries are rotated on the LRU on the first encounter and only written back on a second pass. If fewer bytes are written back than requested, -EAGAIN is returned, matching the existing memory.reclaim semantics.
Internally, extend user_proactive_reclaim() to parse the new "source=" key and invoke the dedicated handler zswap_proactive_writeback() when it is set to "zswap". This handler walks the target memcg subtree in a round-robin fashion and drains each memcg's per-node zswap LRUs through shrink_memcg(), accumulating the compressed bytes written back until the requested budget is met.
--- Documentation/admin-guide/cgroup-v2.rst | 18 ++++++++- Documentation/admin-guide/mm/zswap.rst | 11 +++++- include/linux/zswap.h | 7 ++++ mm/vmscan.c | 24 +++++++++++- mm/zswap.c | 50 +++++++++++++++++++++++++ 5 files changed, 106 insertions(+), 4 deletions(-)
Documentation/admin-guide/cgroup-v2.rst +16 -2
@@ -1425,9 +1425,10 @@ PAGE_SIZE multiple when read back. The following nested keys are defined. - ========== ================================ + ==================== ================================================== swappiness Swappiness value to reclaim with - ========== ================================ + source=zswap Only perform proactive zswap writeback + ==================== ================================================== Specifying a swappiness value instructs the kernel to perform the reclaim with that swappiness value. Note that this has the
@@ -1437,6 +1438,19 @@ The following nested keys are defined. The valid range for swappiness is [0-200, max], setting swappiness=max exclusively reclaims anonymous memory. + The source=zswap key skips ordinary memory reclaim and + writes back pages from zswap to the backing swap device until + the requested amount has been written or no further candidates + are found. This is useful to proactively offload cold compressed + data from the zswap pool to the swap device. It is only available + if zswap writeback is enabled. source=zswap cannot be + combined with swappiness; specifying both returns -EINVAL. + + Example:: + + # Writeback up to 10MB of compressed data from zswap to the backing swap + echo "10M source=zswap" > memory.reclaim + memory.peak A read-write single value file which exists on non-root cgroups.
Documentation/admin-guide/mm/zswap.rst +10 -1
@@ -131,7 +131,16 @@ User can enable it as follows:: echo Y > /sys/module/zswap/parameters/shrinker_enabled This can be enabled at the boot time if ``CONFIG_ZSWAP_SHRINKER_DEFAULT_ON`` is -selected. +selected. Once enabled, the shrinker automatically writes back zswap pages to +backing swap during memory reclaim. + +If users want to explicitly trigger proactive zswap writeback for a specific +memory cgroup without invoking standard page reclaim, it can be done as follows:: + + echo "10M source=zswap" > /sys/fs/cgroup/<cgroup-name>/memory.reclaim + +Both of the methods mentioned above are subject to the ``memory.zswap.writeback`` +control. This means that ``memory.zswap.writeback`` can prevent all zswap writeback. A debugfs interface is provided for various statistic about pool size, number of pages stored, same-value filled pages and various counters for the reasons
include/linux/zswap.h +7 -0
@@ -35,6 +35,7 @@ void zswap_lruvec_state_init(struct lruvec *lruvec);void zswap_folio_swapin (struct folio *folio);bool zswap_is_enabled (void );bool zswap_never_enabled (void ); +int zswap_proactive_writeback (struct mem_cgroup *memcg,u64 bytes_to_writeback);#else struct zswap_lruvec_state {};
@@ -69,6 +70,12 @@ static inline bool zswap_never_enabled(void)return true; } +static inline int zswap_proactive_writeback (struct mem_cgroup *memcg, +u64 bytes_to_writeback) +{ +return -EOPNOTSUPP ; +} +#endif #endif /* _LINUX_ZSWAP_H */
mm/vmscan.c +23 -1
@@ -64,6 +64,7 @@#include <linux/swapops.h> #include <linux/sched/sysctl.h> +#include <linux/zswap.h> #include "internal.h" #include "swap.h"
@@ -7855,11 +7856,13 @@ static unsigned long __node_reclaim(struct pglist_data *pgdat, gfp_t gfp_mask,enum {MEMORY_RECLAIM_SWAPPINESS =0 ,MEMORY_RECLAIM_SWAPPINESS_MAX , +MEMORY_RECLAIM_SOURCE ,MEMORY_RECLAIM_NULL , };static const match_table_t tokens = { {MEMORY_RECLAIM_SWAPPINESS ,"swappiness=%d" }, {MEMORY_RECLAIM_SWAPPINESS_MAX ,"swappiness=max" }, + {MEMORY_RECLAIM_SOURCE ,"source=%s" }, {MEMORY_RECLAIM_NULL ,NULL }, };
@@ -7869,9 +7872,12 @@ int user_proactive_reclaim(char *buf,unsigned int nr_retries =MAX_RECLAIM_RETRIES ;unsigned long nr_to_reclaim, nr_reclaimed =0 ;int swappiness =-1 ; +bool zswap_writeback_only = false;char *old_buf, *start; +char source[16 ];substring_t args[MAX_OPT_ARGS ];gfp_t gfp_mask =GFP_KERNEL ; +u64 nr_bytes;if (!buf || (!memcg && !pgdat) || (memcg && pgdat))return -EINVAL ;
@@ -7879,7 +7885,8 @@ int user_proactive_reclaim(char *buf, buf =strstrip (buf); old_buf = buf; - nr_to_reclaim =memparse (buf, &buf) /PAGE_SIZE ; + nr_bytes =memparse (buf, &buf); + nr_to_reclaim = nr_bytes /PAGE_SIZE ;if (buf == old_buf)return -EINVAL ;
@@ -7899,11 +7906,26 @@ int user_proactive_reclaim(char *buf,case MEMORY_RECLAIM_SWAPPINESS_MAX : swappiness =SWAPPINESS_ANON_ONLY ;break ; +case MEMORY_RECLAIM_SOURCE : +if (match_strlcpy (source, &args[0 ],sizeof (source)) >=sizeof (source)) +return -EINVAL ; +/* Only zswap is supported as a reclaim source for now. */ +if (strcmp (source,"zswap" )) +return -EINVAL ; + zswap_writeback_only = true; +break ;default :return -EINVAL ; } } +if (zswap_writeback_only) { +/* source=zswap and swappiness are mutually exclusive. */ +if (swappiness !=-1 ) +return -EINVAL ; +return zswap_proactive_writeback (memcg, nr_bytes); + } +while (nr_reclaimed < nr_to_reclaim) {/* Will converge on zero, but reclaim enforces a minimum */ unsigned long batch_size = (nr_to_reclaim - nr_reclaimed) /4 ;
mm/zswap.c +50 -0
56 lines not on this page · raw message
Signature · 1 line
2.34.1
Yosry Ahmed
maintainerRe: [PATCH v5 4/6] mm/zswap: Implement proactive writeback
Quoted 32 lines · Hao Jia
From: Hao Jia <[email protected]>
Zswap currently writes back pages to backing swap reactively, triggered either by the shrinker or when the pool reaches its size limit. There is no mechanism to control the amount of writeback for a specific memory cgroup. However, users may want to proactively write back zswap pages, e.g., to free up memory for other applications or to prepare for memory-intensive workloads.
Introduce a "source=" key to the memory.reclaim cgroup interface, currently accepting the single value "zswap". When set to "zswap", it bypasses standard memory reclaim and exclusively performs proactive zswap writeback up to the requested budget. If omitted, the default reclaim behavior remains unchanged.
Example usage:
# Write back 10MB of compressed data from zswap to the backing swap echo "10M source=zswap" > memory.reclaimNote that the actual amount of compressed data written back may be less than requested due to the zswap second-chance algorithm: referenced entries are rotated on the LRU on the first encounter and only written back on a second pass. If fewer bytes are written back than requested,
-EAGAINis returned, matching the existing memory.reclaim semantics.Internally, extend
user_proactive_reclaim()to parse the new "source=" key and invoke the dedicated handlerzswap_proactive_writeback()when it is set to "zswap". This handler walks the target memcg subtree in a round-robin fashion and drains each memcg's per-node zswap LRUs throughshrink_memcg(), accumulating the compressed bytes written back until the requested budget is met.
Quoted 4 lines
Suggested-by Yosry Ahmed <[email protected]>Suggested-by Nhat Pham <[email protected]>Signed-off-by Hao Jia <[email protected]>---
Before going through more versions we need to figure out if this will pivot to be a proactive demotion interfcae for swap tiering.
Quoted 14 lines
mm/vmscan.c +3
-0@@ -7869,9 +7872,12 @@ int user_proactive_reclaim(char *buf,unsigned int nr_retries =MAX_RECLAIM_RETRIES ;unsigned long nr_to_reclaim, nr_reclaimed =0 ;int swappiness =-1 ; +bool zswap_writeback_only = false;char *old_buf, *start; +char source[16 ];substring_t args[MAX_OPT_ARGS ];gfp_t gfp_mask =GFP_KERNEL ; +u64 nr_bytes;if (!buf || (!memcg && !pgdat) || (memcg && pgdat))return -EINVAL ;
mm/vmscan.c int user_proactive_reclaim(char *buf, · line 7889
Quoted 6 lines
mm/vmscan.c +2
-1@@ -7879,7 +7885,8 @@ int user_proactive_reclaim(char *buf, buf =strstrip (buf); old_buf = buf; - nr_to_reclaim =memparse (buf, &buf) /PAGE_SIZE ; + nr_bytes =memparse (buf, &buf); + nr_to_reclaim = nr_bytes /PAGE_SIZE ;
Nit: if we keep this as part of memory.reclaim, we probably want to choose clearer names (e.g. pages_to_reclaim and bytes_to_reclaim).
Quoted 37 lines
mm/vmscan.c +15
-0if (buf == old_buf)return -EINVAL ;@@ -7899,11 +7906,26 @@ int user_proactive_reclaim(char *buf,case MEMORY_RECLAIM_SWAPPINESS_MAX : swappiness =SWAPPINESS_ANON_ONLY ;break ; +case MEMORY_RECLAIM_SOURCE : +if (match_strlcpy (source, &args[0 ],sizeof (source)) >=sizeof (source)) +return -EINVAL ; +/* Only zswap is supported as a reclaim source for now. */ +if (strcmp (source,"zswap" )) +return -EINVAL ; + zswap_writeback_only = true; +break ;default :return -EINVAL ; } } +if (zswap_writeback_only) { +/* source=zswap and swappiness are mutually exclusive. */ +if (swappiness !=-1 ) +return -EINVAL ; +return zswap_proactive_writeback (memcg, nr_bytes); + } +while (nr_reclaimed < nr_to_reclaim) {/* Will converge on zero, but reclaim enforces a minimum */ unsigned long batch_size = (nr_to_reclaim - nr_reclaimed) /4 ;mm/zswap.c +0
-0@@ -1713,6 +1713,56 @@ int zswap_load(struct folio *folio)return 0 ; }
mm/zswap.c int zswap_load(struct folio *folio) · line 1724
Quoted 10 lines
mm/zswap.c +9
-0+int zswap_proactive_writeback (struct mem_cgroup *memcg,u64 bytes_to_writeback) +{ +struct zswap_shrink_state s = {}; +struct mem_cgroup *iter =NULL ; +u64 bytes_written =0 ; +int ret =0 ; + +if (!memcg) +return -EINVAL ;
Can this ever happen? It would be a bug in the caller.
mm/zswap.c int zswap_load(struct folio *folio) · line 1728
Quoted 4 lines
mm/zswap.c +4
-0+if (!mem_cgroup_zswap_writeback_enabled (memcg)) +return -EINVAL ; +if (!bytes_to_writeback) +return 0 ;
Do we need this? I think the loop will just never enter and mem_cgroup_iter_break() will do nothing.
Quoted 16 lines
mm/zswap.c +16
-0+ +while (bytes_written < bytes_to_writeback) { +long shrunk; + +cond_resched (); + +if (signal_pending (current)) { + ret = -EINTR ; +break ; + } + +/* +* Use a local iterator to walk the memcg and its online descendants +* in a round-robin manner. Upon exiting the loop, mem_cgroup_iter_break() +* must be called to drop the iterator reference. +*/
mm/zswap.c int zswap_load(struct folio *folio) · line 1754
Quoted 10 lines
mm/zswap.c +10
-0+do { + iter =mem_cgroup_iter (memcg, iter,NULL ); + }while (iter && !mem_cgroup_tryget_online (iter)); + + shrunk =zswap_shrink_one_memcg (iter, &s); +if (shrunk >0 ) + bytes_written += shrunk; + +/* drop the extra reference taken by mem_cgroup_tryget_online() */ +mem_cgroup_put (iter);
Can we just use mem_cgroup_online() instead since mem_cgroup_iter() already graps a ref?
Quoted 17 lines
mm/zswap.c +11
-0+ +if (shrunk == -EBUSY ) { + ret = -EAGAIN ; +break ; + } + } + +mem_cgroup_iter_break (memcg, iter); +return ret; +} +void zswap_invalidate (swp_entry_t swp) {pgoff_t offset =swp_offset (swp);Signature · 1 line
2.34.1
Re: [PATCH v5 4/6] mm/zswap: Implement proactive writeback
Quoted 33 lines · Yosry Ahmed
Quoted 32 lines · Hao Jia
From: Hao Jia <[email protected]>
Zswap currently writes back pages to backing swap reactively, triggered either by the shrinker or when the pool reaches its size limit. There is no mechanism to control the amount of writeback for a specific memory cgroup. However, users may want to proactively write back zswap pages, e.g., to free up memory for other applications or to prepare for memory-intensive workloads.
Introduce a "source=" key to the memory.reclaim cgroup interface, currently accepting the single value "zswap". When set to "zswap", it bypasses standard memory reclaim and exclusively performs proactive zswap writeback up to the requested budget. If omitted, the default reclaim behavior remains unchanged.
Example usage:
# Write back 10MB of compressed data from zswap to the backing swap echo "10M source=zswap" > memory.reclaimNote that the actual amount of compressed data written back may be less than requested due to the zswap second-chance algorithm: referenced entries are rotated on the LRU on the first encounter and only written back on a second pass. If fewer bytes are written back than requested,
-EAGAINis returned, matching the existing memory.reclaim semantics.Internally, extend
user_proactive_reclaim()to parse the new "source=" key and invoke the dedicated handlerzswap_proactive_writeback()when it is set to "zswap". This handler walks the target memcg subtree in a round-robin fashion and drains each memcg's per-node zswap LRUs throughshrink_memcg(), accumulating the compressed bytes written back until the requested budget is met.
Quoted 8 lines
Quoted 4 lines
Suggested-by Yosry Ahmed <[email protected]>Suggested-by Nhat Pham <[email protected]>Signed-off-by Hao Jia <[email protected]>---
Before going through more versions we need to figure out if this will pivot to be a proactive demotion interfcae for swap tiering.
Yes. Should I drop patches 4-6 in the next version and wait for swap tiering to be finalized?
We can try to get the non-memcg parts (patches 1-3) merged upstream first. This would also give them plenty of time to bake and catch any potential regressions. Thoughts?
Quoted 16 lines
Quoted 16 lines
mm/vmscan.c +3
-0@@ -7869,9 +7872,12 @@ int user_proactive_reclaim(char *buf,unsigned int nr_retries =MAX_RECLAIM_RETRIES ;unsigned long nr_to_reclaim, nr_reclaimed =0 ;int swappiness =-1 ; +bool zswap_writeback_only = false;char *old_buf, *start; +char source[16 ];substring_t args[MAX_OPT_ARGS ];gfp_t gfp_mask =GFP_KERNEL ; +u64 nr_bytes;if (!buf || (!memcg && !pgdat) || (memcg && pgdat))return -EINVAL ;@@ -7879,7 +7885,8 @@ int user_proactive_reclaim(char *buf, buf =strstrip (buf);
Quoted 7 lines
mm/vmscan.c int user_proactive_reclaim(char *buf, · line 7889
Quoted 4 lines
mm/vmscan.c +2
-1old_buf = buf; - nr_to_reclaim =memparse (buf, &buf) /PAGE_SIZE ; + nr_bytes =memparse (buf, &buf); + nr_to_reclaim = nr_bytes /PAGE_SIZE ;Nit: if we keep this as part of memory.reclaim, we probably want to choose clearer names (e.g.
pages_to_reclaimandbytes_to_reclaim).
Will do.
Quoted 39 lines
Quoted 39 lines
mm/vmscan.c +15
-0if (buf == old_buf)return -EINVAL ;@@ -7899,11 +7906,26 @@ int user_proactive_reclaim(char *buf,case MEMORY_RECLAIM_SWAPPINESS_MAX : swappiness =SWAPPINESS_ANON_ONLY ;break ; +case MEMORY_RECLAIM_SOURCE : +if (match_strlcpy (source, &args[0 ],sizeof (source)) >=sizeof (source)) +return -EINVAL ; +/* Only zswap is supported as a reclaim source for now. */ +if (strcmp (source,"zswap" )) +return -EINVAL ; + zswap_writeback_only = true; +break ;default :return -EINVAL ; } } +if (zswap_writeback_only) { +/* source=zswap and swappiness are mutually exclusive. */ +if (swappiness !=-1 ) +return -EINVAL ; +return zswap_proactive_writeback (memcg, nr_bytes); + } +while (nr_reclaimed < nr_to_reclaim) {/* Will converge on zero, but reclaim enforces a minimum */ unsigned long batch_size = (nr_to_reclaim - nr_reclaimed) /4 ;mm/zswap.c +0
-0@@ -1713,6 +1713,56 @@ int zswap_load(struct folio *folio)return 0 ; }
Quoted 11 lines
mm/zswap.c int zswap_load(struct folio *folio) · line 1724
Quoted 9 lines
mm/zswap.c +9
-0+int zswap_proactive_writeback (struct mem_cgroup *memcg,u64 bytes_to_writeback) +{ +struct zswap_shrink_state s = {}; +struct mem_cgroup *iter =NULL ; +u64 bytes_written =0 ; +int ret =0 ; + +if (!memcg) +return -EINVAL ;Can this ever happen? It would be a bug in the caller.
IIRC，Writing the following to the NUMA node sysfs entry triggers this check:
echo "10M source=zswap" > /sys/devices/system/node/nodeN/reclaim
Quoted 8 lines
mm/zswap.c int zswap_load(struct folio *folio) · line 1728
Quoted 5 lines
mm/zswap.c +4
-0+if (!mem_cgroup_zswap_writeback_enabled (memcg)) +return -EINVAL ; +if (!bytes_to_writeback) +return 0 ;Do we need this? I think the loop will just never enter and
mem_cgroup_iter_break()will do nothing.
Will do.
Quoted 23 lines
Quoted 23 lines
mm/zswap.c +22
-0+ +while (bytes_written < bytes_to_writeback) { +long shrunk; + +cond_resched (); + +if (signal_pending (current)) { + ret = -EINTR ; +break ; + } + +/* +* Use a local iterator to walk the memcg and its online descendants +* in a round-robin manner. Upon exiting the loop, mem_cgroup_iter_break() +* must be called to drop the iterator reference. +*/ +do { + iter =mem_cgroup_iter (memcg, iter,NULL ); + }while (iter && !mem_cgroup_tryget_online (iter)); + + shrunk =zswap_shrink_one_memcg (iter, &s); +if (shrunk >0 )
Quoted 9 lines
mm/zswap.c int zswap_load(struct folio *folio) · line 1754
Quoted 4 lines
mm/zswap.c +4
-0+ bytes_written += shrunk; + +/* drop the extra reference taken by mem_cgroup_tryget_online() */ +mem_cgroup_put (iter);Can we just use
mem_cgroup_online()instead sincemem_cgroup_iter()already graps a ref?
Will do.
Thanks,
Hao
Yosry Ahmed
maintainerRe: [PATCH v5 4/6] mm/zswap: Implement proactive writeback
Quoted 9 lines
Quoted 3 lines
Before going through more versions we need to figure out if this will pivot to be a proactive demotion interfcae for swap tiering.
Yes. Should I drop patches 4-6 in the next version and wait for swap tiering to be finalized?
We can try to get the non-memcg parts (patches 1-3) merged upstream first. This would also give them plenty of time to bake and catch any potential regressions. Thoughts?
Patches 1-2 can be sent and merged separately, yes. For patch 2, please include some numbers for the writeback performance before and after batching.
Patch 3 does refactoring in preparation for patch 4, so I don't think it makes sense on its own.
Quoted 12 lines
Quoted 11 lines
mm/zswap.c int zswap_load(struct folio *folio) · line 1724
Quoted 9 lines
mm/zswap.c +9
-0+int zswap_proactive_writeback (struct mem_cgroup *memcg,u64 bytes_to_writeback) +{ +struct zswap_shrink_state s = {}; +struct mem_cgroup *iter =NULL ; +u64 bytes_written =0 ; +int ret =0 ; + +if (!memcg) +return -EINVAL ;Can this ever happen? It would be a bug in the caller.
Quoted 3 lines
IIRC，Writing the following to the NUMA node sysfs entry triggers this check:
echo "10M source=zswap" >/sys/devices/system/node/nodeN/reclaim
Oh yeah, I forgot about that one :)
If we keep this, probably combine the !memcg and writeback check below.
Quoted 43 lines
Quoted 9 lines
mm/zswap.c int zswap_load(struct folio *folio) · line 1728
Quoted 6 lines
mm/zswap.c +4
-0+if (!mem_cgroup_zswap_writeback_enabled (memcg)) +return -EINVAL ; +if (!bytes_to_writeback) +return 0 ;Do we need this? I think the loop will just never enter and
mem_cgroup_iter_break()will do nothing.Will do.
Quoted 23 lines
Quoted 23 lines
mm/zswap.c +22
-0+ +while (bytes_written < bytes_to_writeback) { +long shrunk; + +cond_resched (); + +if (signal_pending (current)) { + ret = -EINTR ; +break ; + } + +/* +* Use a local iterator to walk the memcg and its online descendants +* in a round-robin manner. Upon exiting the loop, mem_cgroup_iter_break() +* must be called to drop the iterator reference. +*/ +do { + iter =mem_cgroup_iter (memcg, iter,NULL ); + }while (iter && !mem_cgroup_tryget_online (iter)); + + shrunk =zswap_shrink_one_memcg (iter, &s); +if (shrunk >0 )Quoted 9 lines
mm/zswap.c int zswap_load(struct folio *folio) · line 1754
Quoted 4 lines
mm/zswap.c +4
-0+ bytes_written += shrunk; + +/* drop the extra reference taken by mem_cgroup_tryget_online() */ +mem_cgroup_put (iter);Can we just use
mem_cgroup_online()instead sincemem_cgroup_iter()already graps a ref?
Quoted 1 line
Will do.
If you're looking for another cleanup to do, shrink_worker() should probably also use mem_cgroup_online() and avoid taking/dropping an extra ref :)
Re: [PATCH v5 4/6] mm/zswap: Implement proactive writeback
Quoted 10 lines · Yosry Ahmed
Quoted 9 lines
Quoted 3 lines
Before going through more versions we need to figure out if this will pivot to be a proactive demotion interfcae for swap tiering.
Yes. Should I drop patches 4-6 in the next version and wait for swap tiering to be finalized?
We can try to get the non-memcg parts (patches 1-3) merged upstream first. This would also give them plenty of time to bake and catch any potential regressions. Thoughts?
Quoted 6 lines
Patches 1-2 can be sent and merged separately, yes. For patch 2, please include some numbers for the writeback performance before and after batching.
Patch 3 does refactoring in preparation for patch 4, so I don't think it makes sense on its own.
Will do.
Quoted 13 lines
Quoted 13 lines
Quoted 12 lines
mm/zswap.c int zswap_load(struct folio *folio) · line 1724
Quoted 10 lines
mm/zswap.c +9
-0+int zswap_proactive_writeback (struct mem_cgroup *memcg,u64 bytes_to_writeback) +{ +struct zswap_shrink_state s = {}; +struct mem_cgroup *iter =NULL ; +u64 bytes_written =0 ; +int ret =0 ; + +if (!memcg) +return -EINVAL ;Can this ever happen? It would be a bug in the caller.
Quoted 7 lines
Quoted 3 lines
IIRC，Writing the following to the NUMA node sysfs entry triggers this check:
echo "10M source=zswap" >/sys/devices/system/node/nodeN/reclaimOh yeah, I forgot about that one :)
If we keep this, probably combine the !memcg and writeback check below.
Will do.
Quoted 44 lines
Quoted 44 lines
Quoted 10 lines
mm/zswap.c int zswap_load(struct folio *folio) · line 1728
Quoted 7 lines
mm/zswap.c +4
-0+if (!mem_cgroup_zswap_writeback_enabled (memcg)) +return -EINVAL ; +if (!bytes_to_writeback) +return 0 ;Do we need this? I think the loop will just never enter and
mem_cgroup_iter_break()will do nothing.Will do.
Quoted 32 lines
Quoted 17 lines
mm/zswap.c +16
-0+ +while (bytes_written < bytes_to_writeback) { +long shrunk; + +cond_resched (); + +if (signal_pending (current)) { + ret = -EINTR ; +break ; + } + +/* +* Use a local iterator to walk the memcg and its online descendants +* in a round-robin manner. Upon exiting the loop, mem_cgroup_iter_break() +* must be called to drop the iterator reference. +*/ mm/zswap.c int zswap_load(struct folio *folio) · line 1754
Quoted 10 lines
mm/zswap.c +10
-0+do { + iter =mem_cgroup_iter (memcg, iter,NULL ); + }while (iter && !mem_cgroup_tryget_online (iter)); + + shrunk =zswap_shrink_one_memcg (iter, &s); +if (shrunk >0 ) + bytes_written += shrunk; + +/* drop the extra reference taken by mem_cgroup_tryget_online() */ +mem_cgroup_put (iter);Can we just use
mem_cgroup_online()instead sincemem_cgroup_iter()already graps a ref?
Quoted 5 lines
Quoted 1 line
Will do.
If you're looking for another cleanup to do,
shrink_worker()should probably also usemem_cgroup_online()and avoid taking/dropping an extra ref :)
IIRC, this might not work because zswap_next_shrink is a global variable and is accessed outside the lock during reclamation.
Consider the following race condition between shrink_worker on CPU0 and zswap_memcg_offline_cleanup on CPU1:
CPU0 CPU1spin_lock(zswap_shrink_lock)
memcg1 = mem_cgroup_iter()
memcg1.ref = 1
zswap_next_shrink = memcg1spin_unlock(zswap_shrink_lock)
zswap_memcg_offline_cleanup()
spin_lock(zswap_shrink_lock)
css_put(zswap_next_shrink)
memcg1.ref = 0 <--shrink_memcg(memcg1) *maybe UAF*
Thanks,
Hao
Yosry Ahmed
maintainerRe: [PATCH v5 4/6] mm/zswap: Implement proactive writeback
Quoted 17 lines
Quoted 3 lines
If you're looking for another cleanup to do,
shrink_worker()should probably also usemem_cgroup_online()and avoid taking/dropping an extra ref :)IIRC, this might not work because
zswap_next_shrinkis a global variable and is accessed outside the lock during reclamation.Consider the following race condition between
shrink_workeron CPU0 andzswap_memcg_offline_cleanupon CPU1:CPU0 CPU1
spin_lock(zswap_shrink_lock)
memcg1 =mem_cgroup_iter()
memcg1.ref = 1
zswap_next_shrink= memcg1spin_unlock(zswap_shrink_lock)
Quoted 6 lines
zswap_memcg_offline_cleanup() spin_lock(zswap_shrink_lock) css_put(zswap_next_shrink) memcg1.ref = 0 <--
shrink_memcg(memcg1)*maybe UAF*
You're right, thanks for double-checking. I lost track of why the extra ref was needed.
Re: [PATCH v5 4/6] mm/zswap: Implement proactive writeback
In reply to Yosry AhmedQuoted 10 lines · Yosry Ahmed
Quoted 9 lines
Quoted 3 lines
Before going through more versions we need to figure out if this will pivot to be a proactive demotion interfcae for swap tiering.
Yes. Should I drop patches 4-6 in the next version and wait for swap tiering to be finalized?
We can try to get the non-memcg parts (patches 1-3) merged upstream first. This would also give them plenty of time to bake and catch any potential regressions. Thoughts?
Quoted 3 lines
Patches 1-2 can be sent and merged separately, yes. For patch 2, please include some numbers for the writeback performance before and after batching.
I'd love to collect some performance data. Do you have any recommended benchmarks for this?
Thanks,
Hao
Quoted 4 lines
Patch 3 does refactoring in preparation for patch 4, so I don't think it makes sense on its own.
Re: [PATCH v5 4/6] mm/zswap: Implement proactive writeback
Quoted 13 lines · Hao Jia
Quoted 10 lines · Yosry Ahmed
Quoted 9 lines
Quoted 3 lines
Before going through more versions we need to figure out if this will pivot to be a proactive demotion interfcae for swap tiering.
Yes. Should I drop patches 4-6 in the next version and wait for swap tiering to be finalized?
We can try to get the non-memcg parts (patches 1-3) merged upstream first. This would also give them plenty of time to bake and catch any potential regressions. Thoughts?
Quoted 6 lines
Quoted 3 lines
Patches 1-2 can be sent and merged separately, yes. For patch 2, please include some numbers for the writeback performance before and after batching.
I'd love to collect some performance data. Do you have any recommended benchmarks for this?
Perhaps the following test case could work?
Test Setup:
- Total memory: 32 GB
- zswap settings: max_pool_percent=1, accept_threshold_percent=50, shrinker_enabled=N
- cgroup constraint: memory.max=1G
- Workload: Run the following stress-ng command inside the cgroup for 120s to continuously force zswap store failures and trigger shrink_worker():
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 following comparison results were collected over multiple runs via bpftrace and the 'written_back_pages' sysfs interface:
Baseline Patched
---------------------------------------------------shrink_worker wakeups 5,587 878shrink_memcg calls 7,823,853 2,347,320written_back 257 781,214
Conclusion:
Under the same workload and duration, the patched kernel shows a significant reduction in both shrink_worker wakeups and shrink_memcg calls, while successfully executing a much higher volume of page writebacks.
Thanks，
Hao
Yosry Ahmed
maintainerRe: [PATCH v5 4/6] mm/zswap: Implement proactive writeback
Quoted 50 lines · Hao Jia
Quoted 13 lines · Hao Jia
Quoted 10 lines · Yosry Ahmed
Quoted 9 lines
Quoted 3 lines
Before going through more versions we need to figure out if this will pivot to be a proactive demotion interfcae for swap tiering.
Yes. Should I drop patches 4-6 in the next version and wait for swap tiering to be finalized?
We can try to get the non-memcg parts (patches 1-3) merged upstream first. This would also give them plenty of time to bake and catch any potential regressions. Thoughts?Quoted 7 lines
Quoted 3 lines
Patches 1-2 can be sent and merged separately, yes. For patch 2, please include some numbers for the writeback performance before and after batching.
I'd love to collect some performance data. Do you have any recommended benchmarks for this?
Perhaps the following test case could work?
Test Setup:
- Total memory: 32 GB
- zswap settings: max_pool_percent=1, accept_threshold_percent=50, shrinker_enabled=N
- cgroup constraint: memory.max=1G
- Workload: Run the following stress-ng command inside the cgroup for 120s to
continuously force zswap store failures and triggershrink_worker():bash -c 'echo $$ > /sys/fs/cgroup/zswaptest/cgroup.procs ; \ exec stress-ng --vm 4 --vm-bytes 4G --vm-keep --vm-method rand-set -t120s -q'
The following comparison results were collected over multiple runs via bpftrace
and the 'written_back_pages' sysfs interface:Baseline Patched
---------------------------------------------------shrink_workerwakeups 5,587 878shrink_memcgcalls 7,823,853 2,347,320written_back257 781,214
Quoted 6 lines
Conclusion:
Under the same workload and duration, the patched kernel shows a significant reduction
in bothshrink_workerwakeups andshrink_memcgcalls, while successfully executing a
much higher volume of page writebacks.
Hmm this is actually a bit concerning. Yes, we are invoking the shrinker less, but we're writing back *a lot* more memory, orders of magnitude more. We are using a batch size of 64, and making ~1/3 of the calls to shrink_memcg(), so the number of written back pages should be ~20x more, not 3000x more? I think I am missing something.
Also, ideally, the batching wouldn't result in significantly more writeback, but a similar amount of writeback over less shrinker invocations. If we are writing back significantly more pages then the batching logic is probably too aggressive?
Re: [PATCH v5 4/6] mm/zswap: Implement proactive writeback
Quoted 64 lines · Yosry Ahmed
Quoted 50 lines · Hao Jia
Quoted 13 lines · Hao Jia
Quoted 10 lines · Yosry Ahmed
Quoted 9 lines
Quoted 3 lines
Before going through more versions we need to figure out if this will pivot to be a proactive demotion interfcae for swap tiering.
Yes. Should I drop patches 4-6 in the next version and wait for swap tiering to be finalized?
We can try to get the non-memcg parts (patches 1-3) merged upstream first. This would also give them plenty of time to bake and catch any potential regressions. Thoughts?Quoted 7 lines
Quoted 3 lines
Patches 1-2 can be sent and merged separately, yes. For patch 2, please include some numbers for the writeback performance before and after batching.
I'd love to collect some performance data. Do you have any recommended benchmarks for this?
Perhaps the following test case could work?
Test Setup:
- Total memory: 32 GB
- zswap settings: max_pool_percent=1, accept_threshold_percent=50, shrinker_enabled=N
- cgroup constraint: memory.max=1G
- Workload: Run the following stress-ng command inside the cgroup for 120s tocontinuously force zswap store failures and trigger shrink_worker(): bash -c 'echo $$ > /sys/fs/cgroup/zswaptest/cgroup.procs ; \ exec stress-ng --vm 4 --vm-bytes 4G --vm-keep --vm-method rand-set -t120s -q'
The following comparison results were collected over multiple runs via bpftrace
and the 'written_back_pages' sysfs interface:Baseline Patched
---------------------------------------------------shrink_workerwakeups 5,587 878shrink_memcgcalls 7,823,853 2,347,320written_back257 781,214Quoted 6 lines
Conclusion:
Under the same workload and duration, the patched kernel shows a significant reduction
in bothshrink_workerwakeups andshrink_memcgcalls, while successfully executing a
much higher volume of page writebacks.Hmm this is actually a bit concerning. Yes, we are invoking the shrinker less, but we're writing back *a lot* more memory, orders of magnitude more. We are using a batch size of 64, and making ~1/3 of the calls to
shrink_memcg(), so the number of written back pages should be ~20x more, not 3000x more? I think I am missing something.
Quoted 4 lines
Also, ideally, the batching wouldn't result in significantly more writeback, but a similar amount of writeback over less shrinker invocations. If we are writing back significantly more pages then the batching logic is probably too aggressive?
Apologies, I think the test I constructed has a bit of a problem. This test has very, very heavy memory pressure and is already a very abnormal case.
The zswap entry returns the first time because of "second chance" after setting referenced to false. For the baseline, it scans 1 page per node each time for 16 loops. During the test, shrink_worker() basically exits at about 16 pages each time.
Since stress-ng periodically and randomly writes to this 4G memory, it keeps triggering zswapin and then waiting to zswapout new zswap entries after falling below the pool threshold. When the speed of zswapin/out is far greater than the scanning speed of shrink_worker(), a large number of zswap entries cannot wait until the second scan for writeback. New entries are stored on the zswap LRU list again, and the referenced of the new zswap entries is set to true again. During the test, it was found that 99.21% of the return values of shrink_memcg_cb() in the baseline kernel were LRU_ROTATE.
For the patched kernel, shrink_worker() scans at least 64 * 16 entries in a single pass, so it can trigger writeback during the second scan.
I have re-tested using a more common, reproducible scenario:
Test Setup:
- Total memory: 32 GB
- zswap settings: max_pool_percent=1, accept_threshold_percent=50, shrinker_enabled=N
Test Procedure:
Allocate 512MB anonymous pages and fill the data with random numbers (to avoid compression), then use cgroup memory.reclaim to force 512M anonymous pages into zswap. Allocate a 4K anonymous page at an interval of 2ms, and then trigger a reclamation of a 4K anonymous page through cgroup memory.reclaim. When the pool threshold is reached, shrink_memcg() will be triggered.
The test data after running for 300s is as follows: (it will eventually stop writing back because the pool threshold is reached)
Baseline Patched
---------------------------------------------------shrink_worker wakeups 5126 85shrink_memcg calls 11,003,734 173,420written_back 40200 40233
Thanks,
Hao
Yosry Ahmed
maintainerRe: [PATCH v5 4/6] mm/zswap: Implement proactive writeback
Quoted 82 lines · Hao Jia
Quoted 64 lines · Yosry Ahmed
Quoted 50 lines · Hao Jia
Quoted 13 lines · Hao Jia
Quoted 10 lines · Yosry Ahmed
Quoted 9 lines
Quoted 3 lines
Before going through more versions we need to figure out if this will pivot to be a proactive demotion interfcae for swap tiering.
Yes. Should I drop patches 4-6 in the next version and wait for swap tiering to be finalized?
We can try to get the non-memcg parts (patches 1-3) merged upstream first. This would also give them plenty of time to bake and catch any potential regressions. Thoughts?Quoted 7 lines
Quoted 3 lines
Patches 1-2 can be sent and merged separately, yes. For patch 2, please include some numbers for the writeback performance before and after batching.
I'd love to collect some performance data. Do you have any recommended benchmarks for this?
Perhaps the following test case could work?
Test Setup:
- Total memory: 32 GB
- zswap settings: max_pool_percent=1, accept_threshold_percent=50, shrinker_enabled=N
- cgroup constraint: memory.max=1G
- Workload: Run the following stress-ng command inside the cgroup for 120s tocontinuously force zswap store failures and trigger shrink_worker(): bash -c 'echo $$ > /sys/fs/cgroup/zswaptest/cgroup.procs ; \ exec stress-ng --vm 4 --vm-bytes 4G --vm-keep --vm-method rand-set -t120s -q'
The following comparison results were collected over multiple runs via bpftrace
and the 'written_back_pages' sysfs interface:Baseline Patched
---------------------------------------------------shrink_workerwakeups 5,587 878shrink_memcgcalls 7,823,853 2,347,320written_back257 781,214Quoted 6 lines
Conclusion:
Under the same workload and duration, the patched kernel shows a significant reduction
in bothshrink_workerwakeups andshrink_memcgcalls, while successfully executing a
much higher volume of page writebacks.Hmm this is actually a bit concerning. Yes, we are invoking the shrinker less, but we're writing back *a lot* more memory, orders of magnitude more. We are using a batch size of 64, and making ~1/3 of the calls to
shrink_memcg(), so the number of written back pages should be ~20x more, not 3000x more? I think I am missing something.Quoted 4 lines
Also, ideally, the batching wouldn't result in significantly more writeback, but a similar amount of writeback over less shrinker invocations. If we are writing back significantly more pages then the batching logic is probably too aggressive?
Apologies, I think the test I constructed has a bit of a problem. This test has very, very heavy memory pressure and is already a very abnormal case.
The zswap entry returns the first time because of "second chance" after setting referenced to false. For the baseline, it scans 1 page per node each time for 16 loops. During the test,
shrink_worker()basically exits at about 16 pages each time.
Quoted 9 lines
Since stress-ng periodically and randomly writes to this 4G memory, it keeps triggering zswapin and then waiting to zswapout new zswap entries after falling below the pool threshold. When the speed of zswapin/out is far greater than the scanning speed of
shrink_worker(), a large number of zswap entries cannot wait until the second scan for writeback. New entries are stored on the zswap LRU list again, and the referenced of the new zswap entries is set to true again. During the test, it was found that 99.21% of the return values ofshrink_memcg_cb()in the baseline kernel wereLRU_ROTATE.
Hmm if I understand correctly, you are saying that the current upstream code is actually failing to writeback when it should in the previous test case with very high memory pressure, but it is with batching? If that's the case, I think it's actually really good data to include. However, we should make sure that's what's actually happening. If the current shrinker is not keeping up and failing to writeback, we should observe:
1. shrink_worker() hitting MAX_RECLAIM_RETRIES continuously and bailing.
2. zswap usage consistently remains at/near the limit, and not going down to the acceptance threshold.
3. zswap_store() failing to accept pages and the pages going directly to disk, causing an LRU inversion (hotter pages on disk, colder pages in zswap).
Can you confirm that this is what's observed with the high pressure test case?
Quoted 20 lines
For the patched kernel,
shrink_worker()scans at least 64 * 16 entries in a single pass, so it can trigger writeback during the second scan.I have re-tested using a more common, reproducible scenario:
Test Setup:
- Total memory: 32 GB
- zswap settings: max_pool_percent=1, accept_threshold_percent=50, shrinker_enabled=NTest Procedure:
Allocate 512MB anonymous pages and fill the data with random numbers (to avoid compression), then use cgroup memory.reclaim to force 512M anonymous pages into zswap.
Allocate a 4K anonymous page at an interval of 2ms, and then trigger a reclamation of a 4K anonymous page through cgroup memory.reclaim. When the pool threshold is reached,shrink_memcg()will be triggered.
Quoted 8 lines
The test data after running for 300s is as follows: (it will eventually stop writing back because the pool threshold is reached)
Baseline Patched
---------------------------------------------------shrink_workerwakeups 5126 85shrink_memcgcalls 11,003,734 173,420written_back40200 40233
This data looks good and makes sense, but I would also want to understand the high pressure test case as well.
Re: [PATCH v5 4/6] mm/zswap: Implement proactive writeback
Quoted 107 lines · Yosry Ahmed
Quoted 82 lines · Hao Jia
Quoted 64 lines · Yosry Ahmed
Quoted 50 lines · Hao Jia
Quoted 13 lines · Hao Jia
Quoted 10 lines · Yosry Ahmed
Quoted 9 lines
Quoted 3 lines
Before going through more versions we need to figure out if this will pivot to be a proactive demotion interfcae for swap tiering.
Yes. Should I drop patches 4-6 in the next version and wait for swap tiering to be finalized?
We can try to get the non-memcg parts (patches 1-3) merged upstream first. This would also give them plenty of time to bake and catch any potential regressions. Thoughts?Quoted 7 lines
Quoted 3 lines
Patches 1-2 can be sent and merged separately, yes. For patch 2, please include some numbers for the writeback performance before and after batching.
I'd love to collect some performance data. Do you have any recommended benchmarks for this?
Perhaps the following test case could work?
Test Setup:
- Total memory: 32 GB
- zswap settings: max_pool_percent=1, accept_threshold_percent=50, shrinker_enabled=N
- cgroup constraint: memory.max=1G
- Workload: Run the following stress-ng command inside the cgroup for 120s tocontinuously force zswap store failures and trigger shrink_worker(): bash -c 'echo $$ > /sys/fs/cgroup/zswaptest/cgroup.procs ; \ exec stress-ng --vm 4 --vm-bytes 4G --vm-keep --vm-method rand-set -t120s -q'
The following comparison results were collected over multiple runs via bpftrace
and the 'written_back_pages' sysfs interface:Baseline Patched
---------------------------------------------------shrink_workerwakeups 5,587 878shrink_memcgcalls 7,823,853 2,347,320written_back257 781,214Quoted 6 lines
Conclusion:
Under the same workload and duration, the patched kernel shows a significant reduction
in bothshrink_workerwakeups andshrink_memcgcalls, while successfully executing a
much higher volume of page writebacks.Hmm this is actually a bit concerning. Yes, we are invoking the shrinker less, but we're writing back *a lot* more memory, orders of magnitude more. We are using a batch size of 64, and making ~1/3 of the calls to
shrink_memcg(), so the number of written back pages should be ~20x more, not 3000x more? I think I am missing something.Quoted 4 lines
Also, ideally, the batching wouldn't result in significantly more writeback, but a similar amount of writeback over less shrinker invocations. If we are writing back significantly more pages then the batching logic is probably too aggressive?
Apologies, I think the test I constructed has a bit of a problem. This test has very, very heavy memory pressure and is already a very abnormal case.
The zswap entry returns the first time because of "second chance" after setting referenced to false. For the baseline, it scans 1 page per node each time for 16 loops. During the test,
shrink_worker()basically exits at about 16 pages each time.Quoted 9 lines
Since stress-ng periodically and randomly writes to this 4G memory, it keeps triggering zswapin and then waiting to zswapout new zswap entries after falling below the pool threshold. When the speed of zswapin/out is far greater than the scanning speed of
shrink_worker(), a large number of zswap entries cannot wait until the second scan for writeback. New entries are stored on the zswap LRU list again, and the referenced of the new zswap entries is set to true again. During the test, it was found that 99.21% of the return values ofshrink_memcg_cb()in the baseline kernel wereLRU_ROTATE.Hmm if I understand correctly, you are saying that the current upstream code is actually failing to writeback when it should in the previous test case with very high memory pressure, but it is with batching? If that's the case, I think it's actually really good data to include. However, we should make sure that's what's actually happening. If the current shrinker is not keeping up and failing to writeback, we should observe:
1.shrink_worker()hittingMAX_RECLAIM_RETRIEScontinuously and bailing.
2. zswap usage consistently remains at/near the limit, and not going down to the acceptance threshold.
3.zswap_store()failing to accept pages and the pages going directly to disk, causing an LRU inversion (hotter pages on disk, colder pages in zswap).
Quoted 2 lines
Can you confirm that this is what's observed with the high pressure test case?
Apologies, my previous explanation might not have been very clear.
For an entry to be written back, the shrinker must scan the *same* entry twice: the first scan sets referenced to false and returns ROTATE, and only during the second scan can it be written back.
If a swap entry is zswapin'd between the first and second scan (meaning the entry is no longer on the zswap LRU), then this swap entry will not be written back by the shrinker. Therefore, the second scan must occur before this entry is zswapin'd for it to be possible to be written back.
So, if the baseline scanning speed is far slower than the lifecycle speed of the swap entries, it results in only scanning once. In the baseline kernel, 99.21% of the return values of shrink_memcg_cb() are LRU_ROTATE, while the patched kernel's shrink_worker() scans at least 64 * 16 entries in a single pass, resulting in only 58.7% of the return values of shrink_memcg_cb() being LRU_ROTATE.
Baseline Patched
---------------------------------------------------
zswapin 929,096 281,196 <----
zswapout 982,731 1,058,746
After modifying the kernel to remove the "second chance" algorithm, the test data of the baseline kernel roughly conforms to our expectations.
Therefore, the constructed test has a bit of a problem. The second test case in my previous email can ensure that the entries on the zswap LRU are relatively stable.
It is worth noting that the capacity of the zswap pool will not remain constant all the time. Since stress-ng periodically writes to and reads from this 4G memory, accessing the pages in the zswap pool will trigger zswapin, causing the zswap pool size to decrease. Other pages attempting to zswapout into zswap via zswap_store() will fail (until the zswap pool size becomes 160M (accept_threshold_percent=50)), but the zswap_store() path will wake up shrink_worker() to try to perform scanning and writeback. Then stress-ng will continue to read and write memory, continuously triggering zswapin to further reduce the zswap pool size until the threshold, at which point shrink_memcg() will also stop. Then, new zswap entries (with referenced as true) are added to the zswap LRU again via zswap_store(), until the zswap pool size reaches 320MB (32 GB (Total memory) * 1%). This process repeats continuously during the test.
Thanks,
Hao
Yosry Ahmed
maintainerRe: [PATCH v5 4/6] mm/zswap: Implement proactive writeback
Quoted 120 lines · Hao Jia
Quoted 107 lines · Yosry Ahmed
Quoted 82 lines · Hao Jia
Quoted 64 lines · Yosry Ahmed
Quoted 50 lines · Hao Jia
Quoted 13 lines · Hao Jia
Quoted 10 lines · Yosry Ahmed
Quoted 9 lines
Quoted 3 lines
Before going through more versions we need to figure out if this will pivot to be a proactive demotion interfcae for swap tiering.
Yes. Should I drop patches 4-6 in the next version and wait for swap tiering to be finalized?
We can try to get the non-memcg parts (patches 1-3) merged upstream first. This would also give them plenty of time to bake and catch any potential regressions. Thoughts?Quoted 7 lines
Quoted 3 lines
Patches 1-2 can be sent and merged separately, yes. For patch 2, please include some numbers for the writeback performance before and after batching.
I'd love to collect some performance data. Do you have any recommended benchmarks for this?
Perhaps the following test case could work?
Test Setup:
- Total memory: 32 GB
- zswap settings: max_pool_percent=1, accept_threshold_percent=50, shrinker_enabled=N
- cgroup constraint: memory.max=1G
- Workload: Run the following stress-ng command inside the cgroup for 120s tocontinuously force zswap store failures and trigger shrink_worker(): bash -c 'echo $$ > /sys/fs/cgroup/zswaptest/cgroup.procs ; \ exec stress-ng --vm 4 --vm-bytes 4G --vm-keep --vm-method rand-set -t120s -q'
The following comparison results were collected over multiple runs via bpftrace
and the 'written_back_pages' sysfs interface:Baseline Patched
---------------------------------------------------shrink_workerwakeups 5,587 878shrink_memcgcalls 7,823,853 2,347,320written_back257 781,214Quoted 6 lines
Conclusion:
Under the same workload and duration, the patched kernel shows a significant reduction
in bothshrink_workerwakeups andshrink_memcgcalls, while successfully executing a
much higher volume of page writebacks.Hmm this is actually a bit concerning. Yes, we are invoking the shrinker less, but we're writing back *a lot* more memory, orders of magnitude more. We are using a batch size of 64, and making ~1/3 of the calls to
shrink_memcg(), so the number of written back pages should be ~20x more, not 3000x more? I think I am missing something.Quoted 4 lines
Also, ideally, the batching wouldn't result in significantly more writeback, but a similar amount of writeback over less shrinker invocations. If we are writing back significantly more pages then the batching logic is probably too aggressive?
Apologies, I think the test I constructed has a bit of a problem. This test has very, very heavy memory pressure and is already a very abnormal case.
The zswap entry returns the first time because of "second chance" after setting referenced to false. For the baseline, it scans 1 page per node each time for 16 loops. During the test,
shrink_worker()basically exits at about 16 pages each time.Quoted 9 lines
Since stress-ng periodically and randomly writes to this 4G memory, it keeps triggering zswapin and then waiting to zswapout new zswap entries after falling below the pool threshold. When the speed of zswapin/out is far greater than the scanning speed of
shrink_worker(), a large number of zswap entries cannot wait until the second scan for writeback. New entries are stored on the zswap LRU list again, and the referenced of the new zswap entries is set to true again. During the test, it was found that 99.21% of the return values ofshrink_memcg_cb()in the baseline kernel wereLRU_ROTATE.Hmm if I understand correctly, you are saying that the current upstream code is actually failing to writeback when it should in the previous test case with very high memory pressure, but it is with batching? If that's the case, I think it's actually really good data to include. However, we should make sure that's what's actually happening. If the current shrinker is not keeping up and failing to writeback, we should observe:
1.shrink_worker()hittingMAX_RECLAIM_RETRIEScontinuously and bailing.
2. zswap usage consistently remains at/near the limit, and not going down to the acceptance threshold.
3.zswap_store()failing to accept pages and the pages going directly to disk, causing an LRU inversion (hotter pages on disk, colder pages in zswap).Quoted 2 lines
Can you confirm that this is what's observed with the high pressure test case?
Apologies, my previous explanation might not have been very clear.
For an entry to be written back, the shrinker must scan the *same* entry
twice: the first scan sets referenced to false and returns ROTATE, and only during the second scan can it be written back.
Quoted 10 lines
If a swap entry is zswapin'd between the first and second scan (meaning the entry is no longer on the zswap LRU), then this swap entry will not be written back by the shrinker. Therefore, the second scan must occur before this entry is zswapin'd for it to be possible to be written back. So, if the baseline scanning speed is far slower than the lifecycle speed of the swap entries, it results in only scanning once. In the baseline kernel, 99.21% of the return values of
shrink_memcg_cb()areLRU_ROTATE, while the patched kernel'sshrink_worker()scans at least 64
* 16 entries in a single pass, resulting in only 58.7% of the return values ofshrink_memcg_cb()beingLRU_ROTATE.
Right, my question is, is the high rate of LRU_ROTATE leading to failure to writeback in a way that causes zswap store failures (and pages skipping zswap and going directly to swap)?
Quoted 30 lines
Baseline Patched
---------------------------------------------------
zswapin 929,096 281,196 <----
zswapout 982,731 1,058,746After modifying the kernel to remove the "second chance" algorithm, the test data of the baseline kernel roughly conforms to our expectations.
Therefore, the constructed test has a bit of a problem. The second test case in my previous email can ensure that the entries on the zswap LRU are relatively stable.
It is worth noting that the capacity of the zswap pool will not remain constant all the time. Since stress-ng periodically writes to and reads from this 4G memory, accessing the pages in the zswap pool will trigger zswapin, causing the zswap pool size to decrease. Other pages attempting to zswapout into zswap via
zswap_store()will fail (until the zswap pool size becomes 160M (accept_threshold_percent=50)), but thezswap_store()path will wake upshrink_worker()to try to perform scanning and writeback. Then stress-ng will continue to read and write memory, continuously triggering zswapin to further reduce the zswap pool size until the threshold, at which pointshrink_memcg()will also stop. Then, new zswap entries (with referenced as true) are added to the zswap LRU again viazswap_store(), until the zswap pool size reaches 320MB (32 GB (Total memory) * 1%). This process repeats continuously during the test.Thanks,
Hao
Re: [PATCH v5 4/6] mm/zswap: Implement proactive writeback
Quoted 132 lines · Yosry Ahmed
Quoted 120 lines · Hao Jia
Quoted 107 lines · Yosry Ahmed
Quoted 82 lines · Hao Jia
Quoted 64 lines · Yosry Ahmed
Quoted 50 lines · Hao Jia
Quoted 13 lines · Hao Jia
Quoted 10 lines · Yosry Ahmed
Quoted 9 lines
Quoted 3 lines
Before going through more versions we need to figure out if this will pivot to be a proactive demotion interfcae for swap tiering.
Yes. Should I drop patches 4-6 in the next version and wait for swap tiering to be finalized?
We can try to get the non-memcg parts (patches 1-3) merged upstream first. This would also give them plenty of time to bake and catch any potential regressions. Thoughts?Quoted 7 lines
Quoted 3 lines
Patches 1-2 can be sent and merged separately, yes. For patch 2, please include some numbers for the writeback performance before and after batching.
I'd love to collect some performance data. Do you have any recommended benchmarks for this?
Perhaps the following test case could work?
Test Setup:
- Total memory: 32 GB
- zswap settings: max_pool_percent=1, accept_threshold_percent=50, shrinker_enabled=N
- cgroup constraint: memory.max=1G
- Workload: Run the following stress-ng command inside the cgroup for 120s tocontinuously force zswap store failures and trigger shrink_worker(): bash -c 'echo $$ > /sys/fs/cgroup/zswaptest/cgroup.procs ; \ exec stress-ng --vm 4 --vm-bytes 4G --vm-keep --vm-method rand-set -t120s -q'
The following comparison results were collected over multiple runs via bpftrace
and the 'written_back_pages' sysfs interface:Baseline Patched
---------------------------------------------------shrink_workerwakeups 5,587 878shrink_memcgcalls 7,823,853 2,347,320written_back257 781,214Quoted 6 lines
Conclusion:
Under the same workload and duration, the patched kernel shows a significant reduction
in bothshrink_workerwakeups andshrink_memcgcalls, while successfully executing a
much higher volume of page writebacks.Hmm this is actually a bit concerning. Yes, we are invoking the shrinker less, but we're writing back *a lot* more memory, orders of magnitude more. We are using a batch size of 64, and making ~1/3 of the calls to
shrink_memcg(), so the number of written back pages should be ~20x more, not 3000x more? I think I am missing something.Quoted 4 lines
Also, ideally, the batching wouldn't result in significantly more writeback, but a similar amount of writeback over less shrinker invocations. If we are writing back significantly more pages then the batching logic is probably too aggressive?
Apologies, I think the test I constructed has a bit of a problem. This test has very, very heavy memory pressure and is already a very abnormal case.
The zswap entry returns the first time because of "second chance" after setting referenced to false. For the baseline, it scans 1 page per node each time for 16 loops. During the test,
shrink_worker()basically exits at about 16 pages each time.Quoted 9 lines
Since stress-ng periodically and randomly writes to this 4G memory, it keeps triggering zswapin and then waiting to zswapout new zswap entries after falling below the pool threshold. When the speed of zswapin/out is far greater than the scanning speed of
shrink_worker(), a large number of zswap entries cannot wait until the second scan for writeback. New entries are stored on the zswap LRU list again, and the referenced of the new zswap entries is set to true again. During the test, it was found that 99.21% of the return values ofshrink_memcg_cb()in the baseline kernel wereLRU_ROTATE.Hmm if I understand correctly, you are saying that the current upstream code is actually failing to writeback when it should in the previous test case with very high memory pressure, but it is with batching? If that's the case, I think it's actually really good data to include. However, we should make sure that's what's actually happening. If the current shrinker is not keeping up and failing to writeback, we should observe:
1.shrink_worker()hittingMAX_RECLAIM_RETRIEScontinuously and bailing.
2. zswap usage consistently remains at/near the limit, and not going down to the acceptance threshold.
3.zswap_store()failing to accept pages and the pages going directly to disk, causing an LRU inversion (hotter pages on disk, colder pages in zswap).Quoted 2 lines
Can you confirm that this is what's observed with the high pressure test case?
Apologies, my previous explanation might not have been very clear.
For an entry to be written back, the shrinker must scan the *same* entry
twice: the first scan sets referenced to false and returns ROTATE, and only during the second scan can it be written back.Quoted 10 lines
If a swap entry is zswapin'd between the first and second scan (meaning the entry is no longer on the zswap LRU), then this swap entry will not be written back by the shrinker. Therefore, the second scan must occur before this entry is zswapin'd for it to be possible to be written back. So, if the baseline scanning speed is far slower than the lifecycle speed of the swap entries, it results in only scanning once. In the baseline kernel, 99.21% of the return values of
shrink_memcg_cb()areLRU_ROTATE, while the patched kernel'sshrink_worker()scans at least 64
* 16 entries in a single pass, resulting in only 58.7% of the return values ofshrink_memcg_cb()beingLRU_ROTATE.
Quoted 3 lines
Right, my question is, is the high rate of
LRU_ROTATEleading to failure to writeback in a way that causes zswap store failures (and pages skipping zswap and going directly to swap)?
Yes, I observed that the number of zswap_store() failures (returning false) is very high.
Thanks，
Hao
PATCH v5 5/6 mm/zswap: Add per-memcg stat for proactive writeback
In reply to Hao JiaFrom: Hao Jia <[email protected]>
Add a new stat zswpwb_proactive_b to memory.stat. This counter is incremented by entry->length during proactive writebacks triggered via the source=zswap key in memory.reclaim. It tracks the compressed size (in bytes) of pages proactively written back from zswap to swap, allowing users to better monitor and tune the proactive writeback mechanism.
--- Documentation/admin-guide/cgroup-v2.rst | 4 ++++ include/linux/memcontrol.h | 1 + mm/memcontrol.c | 3 +++ mm/zswap.c | 4 +++- 4 files changed, 11 insertions(+), 1 deletion(-)
Documentation/admin-guide/cgroup-v2.rst +4 -0
@@ -1748,6 +1748,10 @@ The following nested keys are defined. zswpwb Number of pages written from zswap to swap. + zswpwb_proactive_b + Bytes of compressed data proactively written back from + zswap to swap via the memory.reclaim source=zswap key. + zswap_incomp Number of incompressible pages currently stored in zswap without compression. These pages could not be compressed to
include/linux/memcontrol.h +1 -0
@@ -40,6 +40,7 @@ enum memcg_stat_item {MEMCG_ZSWAP_B ,MEMCG_ZSWAPPED ,MEMCG_ZSWAP_INCOMP , +MEMCG_ZSWPWB_PROACTIVE_B , MEMCG_NR_STAT, };
mm/memcontrol.c +3 -0
@@ -433,6 +433,7 @@ static const unsigned int memcg_stat_items[] = {MEMCG_ZSWAP_B ,MEMCG_ZSWAPPED ,MEMCG_ZSWAP_INCOMP , + MEMCG_ZSWPWB_PROACTIVE_B, };#define NR_MEMCG_NODE_STAT_ITEMS ARRAY_SIZE(memcg_node_stat_items)
@@ -1558,6 +1559,7 @@ static const struct memory_stat memory_stats[] = { { "zswap", MEMCG_ZSWAP_B }, { "zswapped", MEMCG_ZSWAPPED }, { "zswap_incomp", MEMCG_ZSWAP_INCOMP }, + { "zswpwb_proactive_b", MEMCG_ZSWPWB_PROACTIVE_B }, #endif { "file_mapped", NR_FILE_MAPPED }, { "file_dirty", NR_FILE_DIRTY },
@@ -1614,6 +1616,7 @@ static int memcg_page_state_unit(int item)switch (item) {case MEMCG_PERCPU_B :case MEMCG_ZSWAP_B : +case MEMCG_ZSWPWB_PROACTIVE_B :case NR_SLAB_RECLAIMABLE_B :case NR_SLAB_UNRECLAIMABLE_B :return 1 ;
mm/zswap.c +3 -1
@@ -1747,8 +1747,10 @@ int zswap_proactive_writeback(struct mem_cgroup *memcg, u64 bytes_to_writeback) }while (iter && !mem_cgroup_tryget_online (iter)); shrunk =zswap_shrink_one_memcg (iter, &s); -if (shrunk >0 ) +if (shrunk >0 ) { bytes_written += shrunk; +mod_memcg_state (iter,MEMCG_ZSWPWB_PROACTIVE_B , shrunk); + }/* drop the extra reference taken by mem_cgroup_tryget_online() */ mem_cgroup_put (iter);
Signature · 1 line
2.34.1
PATCH v5 6/6 selftests/cgroup: Add tests for zswap proactive writeback
In reply to Hao JiaFrom: Hao Jia <[email protected]>
Add test_zswap_proactive_writeback() to cover the new memory.reclaim "source=zswap" key. The test populates a memory cgroup zswap pool, triggers proactive writeback, and verifies the behavior by observing the change in zswpwb_proactive_b. Invalid input combinations are also covered.
Extend test_zswap_writeback_one() to assert that the existing non-proactive writeback path leaves zswpwb_proactive_b at zero.
--- tools/testing/selftests/cgroup/test_zswap.c | 161 +++++++++++++++++++- 1 file changed, 160 insertions(+), 1 deletion(-)
tools/testing/selftests/cgroup/test_zswap.c +160 -1
@@ -60,7 +60,12 @@ static int get_zswap_stored_pages(size_t *value)static long get_cg_wb_count (const char *cg) { -return cg_read_key_long (cg,"memory.stat" ,"zswpwb" ); +return cg_read_key_long (cg,"memory.stat" ,"zswpwb " ); +} + +static long get_cg_pwb_bytes (const char *cg) +{ +return cg_read_key_long (cg,"memory.stat" ,"zswpwb_proactive_b " ); }static long get_zswpout (const char *cgroup)
@@ -355,6 +360,7 @@ static int attempt_writeback(const char *cgroup, void *arg)static int test_zswap_writeback_one (const char *cgroup,bool wb) {long zswpwb_before, zswpwb_after; +long pwb_bytes; zswpwb_before =get_cg_wb_count (cgroup);if (zswpwb_before !=0 ) {
@@ -362,6 +368,12 @@ static int test_zswap_writeback_one(const char *cgroup, bool wb)return -1 ; } + pwb_bytes =get_cg_pwb_bytes (cgroup); +if (pwb_bytes >0 ) { +ksft_print_msg ("zswpwb_proactive_b_before = %ld instead of 0\n" , pwb_bytes); +return -1 ; + } +if (cg_run (cgroup, attempt_writeback, (void *) &wb))return -1 ;
@@ -379,6 +391,17 @@ static int test_zswap_writeback_one(const char *cgroup, bool wb)return -1 ; } +/* +* attempt_writeback() does not use the proactive writeback path, so +* zswpwb_proactive_b must stay at zero regardless of whether +* writeback was enabled. +*/ + pwb_bytes =get_cg_pwb_bytes (cgroup); +if (pwb_bytes >0 ) { +ksft_print_msg ("zswpwb_proactive_b_after is %ld, expected 0\n" , pwb_bytes); +return -1 ; + } +return 0 ; }
@@ -770,6 +793,141 @@ static int test_zswap_incompressible(const char *root)return ret; } +/* +* Trigger proactive zswap writeback with the following steps: +* 1. Allocate memory. +* 2. Push allocated memory into zswap. +* 3. Proactively write back zswap pages to swap +* using "source=zswap". +*/ +static int proactive_writeback_workload (const char *cgroup,void *arg) +{ +long pagesize =sysconf (_SC_PAGESIZE); +size_t memsize = pagesize *1024 ; +char reclaim_cmd[64 ]; +char buf[pagesize]; +long zswap_usage; +int ret =-1 ; +int rc; +char *mem; + + mem = (char *)malloc (memsize); +if (!mem) +return ret; + +for (int i =0 ; i < pagesize; i++) + buf[i] = i < pagesize /2 ? (char )i :0 ; +for (int i =0 ; i < memsize; i += pagesize) +memcpy (&mem[i], buf, pagesize); + +/* Evict allocated memory into zswap. */ +if (cg_write_numeric (cgroup,"memory.reclaim" , memsize)) { +ksft_print_msg ("Failed to push pages into zswap\n" ); + goto out; + } + + zswap_usage =cg_read_long (cgroup,"memory.zswap.current" ); +if (zswap_usage <=0 ) { +ksft_print_msg ("no zswap pool to write back\n" ); + goto out; + } + +/* Trigger proactive zswap writeback. */ +snprintf (reclaim_cmd,sizeof (reclaim_cmd),"%ld source=zswap" , zswap_usage); + rc =cg_write (cgroup,"memory.reclaim" , reclaim_cmd); +if (rc && rc != -EAGAIN ) { +ksft_print_msg ("proactive zswap writeback failed: %d\n" , rc); + goto out; + } + + ret =0 ; +out: +free (mem); +return ret; +} + +static int check_writeback_invalid_inputs (const char *cgroup) +{ +static char *const bad_inputs[] = { +"source=zswap" , +"1M source=zswap swappiness=60" , +"1M swappiness=60 source=zswap" , +"1M source=zswap swappiness=max" , +"1M swappiness=max source=zswap" , + }; +int i, rc; + +for (i =0 ; i <ARRAY_SIZE (bad_inputs); i++) { + rc =cg_write (cgroup,"memory.reclaim" , bad_inputs[i]); +if (rc != -EINVAL ) { +ksft_print_msg ("memory.reclaim '%s': returned %d, expected %d\n" , + bad_inputs[i], rc, -EINVAL ); +return -1 ; + } + } +return 0 ; +} + +static int test_zswap_proactive_writeback (const char *root) +{ +long wb_before, wb_after; +long pwb_b_before, pwb_b_after; +long wb_delta, pwb_b_delta; +int ret =KSFT_FAIL ; +char *test_group; + +if (cg_read_strcmp (root,"memory.zswap.writeback" ,"1" )) +return KSFT_SKIP ; + + test_group =cg_name (root,"zswap_proactive_test" ); +if (!test_group) +return KSFT_FAIL ; +if (cg_create (test_group)) + goto out; +/* +* A missing zswpwb_proactive_b stat means the kernel lacks proactive +* writeback support, so skip rather than fail. +*/ +if (get_cg_pwb_bytes (test_group) <0 ) { + ret =KSFT_SKIP ; + goto out; + } +if (check_writeback_invalid_inputs (test_group)) + goto out; + + pwb_b_before =get_cg_pwb_bytes (test_group); + wb_before =get_cg_wb_count (test_group); +if (pwb_b_before <0 || wb_before <0 ) + goto out; + +if (cg_run (test_group, proactive_writeback_workload,NULL )) + goto out; + + pwb_b_after =get_cg_pwb_bytes (test_group); + wb_after =get_cg_wb_count (test_group); +if (pwb_b_after <0 || wb_after <0 ) + goto out; + + pwb_b_delta = pwb_b_after - pwb_b_before; + wb_delta = wb_after - wb_before; + +if (pwb_b_delta <=0 ) { +ksft_print_msg ("zswpwb_proactive_b did not increase: delta=%ld\n" , + pwb_b_delta); + goto out; + } +if (wb_delta <=0 ) { +ksft_print_msg ("zswpwb did not increase: delta=%ld\n" , wb_delta); + goto out; + } + + ret =KSFT_PASS ; +out: +cg_destroy (test_group); +free (test_group); +return ret; +} +#define T (x) { x, #x }struct zswap_test {int (*fn)(const char *root);
@@ -783,6 +941,7 @@ struct zswap_test {T (test_no_kmem_bypass),T (test_no_invasive_cgroup_shrink),T (test_zswap_incompressible), + T(test_zswap_proactive_writeback), };#undef T
Signature · 1 line
2.34.1