* [RFC PATCH v4 0/3] mm/zswap: shrink zswap_entry via a fixed pool index
@ 2026-08-30 11:47 Jianyue Wu
2026-08-30 11:47 ` [RFC PATCH v4 1/3] mm/zswap: release retired pools via queue_rcu_work() instead of synchronize_rcu() Jianyue Wu
` (3 more replies)
0 siblings, 4 replies; 21+ messages in thread
From: Jianyue Wu @ 2026-08-30 11:47 UTC (permalink / raw)
To: Johannes Weiner, Yosry Ahmed, Nhat Pham, Chengming Zhou, Andrew Morton
Cc: Jianyue Wu, Chris Li, linux-mm, linux-kernel
Every stored page has a struct zswap_entry, so its size is pure per-page
overhead. On x86_64 it is currently 56 bytes, of which 8 bytes are a
pointer to the owning zswap_pool.
Only a handful of pools are ever live: a new pool is created only when
the compressor is (re)set, and pools are reused across compressor
switches. That makes a per-entry pool pointer more expensive than it
needs to be, and the RCU list that currently tracks pools is more
machinery than this needs once each pool already has a stable slot.
This series:
1. Releases retired pools with queue_rcu_work() instead of
synchronize_rcu() so the last put no longer blocks on an RCU grace
period.
2. Replaces the zswap_pools list with a fixed ZSWAP_MAX_POOLS (16)
array and a separate RCU-protected current-pool pointer, giving
each pool a stable slot index. Slot 0 is left unused, and pool
creation publishes the fully constructed pool directly into a free
slot.
3. Stores that u8 slot index in each zswap_entry instead of the pool
pointer. The u8 fits in padding after the bool referenced field,
so the entry shrinks from 56 to 48 bytes on x86_64 (~2MiB of
metadata saved per 1GiB of data held in zswap).
Runtime compressor switching is preserved, but the fixed array now
bounds the number of simultaneously live distinct compressor pools to 15
(slot 0 is reserved). If all usable slots are full, creating a pool for
another compressor fails and the compressor switch is rejected.
Benchmark (x86_64, compressor=lzo, MADV_PAGEOUT store + fault-in load):
- zswap_entry object_size: 56 -> 48 bytes
- e2e store+load median latency: no measurable regression vs baseline
at matched stored_delta
The extra cost per store/free/decompress is one array-index load
instead of a pointer dereference. With a single (or few) live pool(s)
that does not show up against (de)compression.
Testing
=======
- sizeof_check: 56 -> 48 bytes on x86_64
- Boot with DEBUG_ATOMIC_SLEEP + lockdep/PROVE_RCU + KASAN:
zswap store/load and compressor switch (retire + reuse) pass
This series is based on akpm/mm-unstable as of 2026-08-30
(42d64d4fef83).
Signed-off-by: Jianyue Wu <wujianyue000@gmail.com>
Changes since RFC v3:
- Retire pools with queue_rcu_work() instead of call_rcu().
- Queue the deferred RCU release work on system_percpu_wq.
- Use rcu_dereference_check() with entry->pool_idx != 0 in
zswap_entry_pool() instead of rcu_dereference_protected().
- Simplify pool-slot publishing after confirming compressor parameter
updates are serialized by the module parameter lock.
Link: https://lore.kernel.org/all/20260815-shrink_zswap_entry_0815_v2-v3-3-0171bd86a667@gmail.com/
Link: https://lore.kernel.org/all/20260731-shrink_zswap_entry_v2-0-0-v2-0-e72083aa8734@gmail.com/
Link: https://lore.kernel.org/all/20260726-shrink_zswap_entry_v1-0-0-v1-1-30957e4d0cb6@gmail.com/
Jianyue Wu (3):
mm/zswap: release retired pools via queue_rcu_work() instead of
synchronize_rcu()
mm/zswap: replace the zswap_pools list with a fixed pools array
mm/zswap: reference the pool by index to shrink struct zswap_entry
mm/zswap.c | 140 ++++++++++++++++++++++++++++++++++++++---------------
1 file changed, 101 insertions(+), 39 deletions(-)
base-commit: 42d64d4fef83a241c919c8693fdf0a21b2cb6061
--
2.43.0
^ permalink raw reply [flat|nested] 21+ messages in thread* [RFC PATCH v4 1/3] mm/zswap: release retired pools via queue_rcu_work() instead of synchronize_rcu() 2026-08-30 11:47 [RFC PATCH v4 0/3] mm/zswap: shrink zswap_entry via a fixed pool index Jianyue Wu @ 2026-08-30 11:47 ` Jianyue Wu 2026-08-31 15:20 ` Yosry Ahmed 2026-09-01 15:38 ` Johannes Weiner 2026-08-30 11:47 ` [RFC PATCH v4 2/3] mm/zswap: replace the zswap_pools list with a fixed pools array Jianyue Wu ` (2 subsequent siblings) 3 siblings, 2 replies; 21+ messages in thread From: Jianyue Wu @ 2026-08-30 11:47 UTC (permalink / raw) To: Johannes Weiner, Yosry Ahmed, Nhat Pham, Chengming Zhou, Andrew Morton Cc: Jianyue Wu, Chris Li, linux-mm, linux-kernel When a pool's last reference is dropped, __zswap_pool_empty() removes it from the pool list and schedules __zswap_pool_release(), which calls synchronize_rcu() to wait for readers before tearing the pool down. synchronize_rcu() is a synchronous, potentially long wait. Replace it with queue_rcu_work(): __zswap_pool_empty() hands the pool to queue_rcu_work(), which waits for a grace period asynchronously and then runs __zswap_pool_release() from a worker for the sleepable teardown (__zswap_pool_empty() can run in atomic context and must not block). The grace-period guarantee is unchanged; the retirement path just no longer blocks on it. Suggested-by: Yosry Ahmed <yosry@kernel.org> Signed-off-by: Jianyue Wu <wujianyue000@gmail.com> --- mm/zswap.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/mm/zswap.c b/mm/zswap.c index 37f34e406c8e..0bb30e58950a 100644 --- a/mm/zswap.c +++ b/mm/zswap.c @@ -155,7 +155,7 @@ struct zswap_pool { struct crypto_acomp_ctx __percpu *acomp_ctx; struct percpu_ref ref; struct list_head list; - struct work_struct release_work; + struct rcu_work release_work; struct hlist_node node; char tfm_name[CRYPTO_MAX_ALG_NAME]; }; @@ -379,10 +379,8 @@ static void zswap_pool_destroy(struct zswap_pool *pool) static void __zswap_pool_release(struct work_struct *work) { - struct zswap_pool *pool = container_of(work, typeof(*pool), - release_work); - - synchronize_rcu(); + struct zswap_pool *pool = container_of(to_rcu_work(work), + typeof(*pool), release_work); /* nobody should have been able to get a ref... */ WARN_ON(!percpu_ref_is_zero(&pool->ref)); @@ -406,8 +404,8 @@ static void __zswap_pool_empty(struct percpu_ref *ref) list_del_rcu(&pool->list); - INIT_WORK(&pool->release_work, __zswap_pool_release); - schedule_work(&pool->release_work); + INIT_RCU_WORK(&pool->release_work, __zswap_pool_release); + queue_rcu_work(system_percpu_wq, &pool->release_work); spin_unlock_bh(&zswap_pools_lock); } -- 2.43.0 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC PATCH v4 1/3] mm/zswap: release retired pools via queue_rcu_work() instead of synchronize_rcu() 2026-08-30 11:47 ` [RFC PATCH v4 1/3] mm/zswap: release retired pools via queue_rcu_work() instead of synchronize_rcu() Jianyue Wu @ 2026-08-31 15:20 ` Yosry Ahmed 2026-09-01 14:33 ` Jianyue Wu 2026-09-01 15:38 ` Johannes Weiner 1 sibling, 1 reply; 21+ messages in thread From: Yosry Ahmed @ 2026-08-31 15:20 UTC (permalink / raw) To: Jianyue Wu Cc: Johannes Weiner, Nhat Pham, Chengming Zhou, Andrew Morton, Chris Li, linux-mm, linux-kernel On Sun, Aug 30, 2026 at 4:47 AM Jianyue Wu <wujianyue000@gmail.com> wrote: > > When a pool's last reference is dropped, __zswap_pool_empty() removes it > from the pool list and schedules __zswap_pool_release(), which calls > synchronize_rcu() to wait for readers before tearing the pool down. > > synchronize_rcu() is a synchronous, potentially long wait. Replace it > with queue_rcu_work(): __zswap_pool_empty() hands the pool to > queue_rcu_work(), which waits for a grace period asynchronously and then > runs __zswap_pool_release() from a worker for the sleepable teardown > (__zswap_pool_empty() can run in atomic context and must not block). > The grace-period guarantee is unchanged; the retirement path just no > longer blocks on it. > > Suggested-by: Yosry Ahmed <yosry@kernel.org> > Signed-off-by: Jianyue Wu <wujianyue000@gmail.com> Why are the patches still tagged RFC? Anyway, with one nit below: Acked-by: Yosry Ahmed <yosry@kernel.org> > --- > mm/zswap.c | 12 +++++------- > 1 file changed, 5 insertions(+), 7 deletions(-) > > diff --git a/mm/zswap.c b/mm/zswap.c > index 37f34e406c8e..0bb30e58950a 100644 > --- a/mm/zswap.c > +++ b/mm/zswap.c > @@ -155,7 +155,7 @@ struct zswap_pool { > struct crypto_acomp_ctx __percpu *acomp_ctx; > struct percpu_ref ref; > struct list_head list; > - struct work_struct release_work; > + struct rcu_work release_work; Seems like the convention is to use rwork in the name instead of work. > struct hlist_node node; > char tfm_name[CRYPTO_MAX_ALG_NAME]; > }; > @@ -379,10 +379,8 @@ static void zswap_pool_destroy(struct zswap_pool *pool) > > static void __zswap_pool_release(struct work_struct *work) > { > - struct zswap_pool *pool = container_of(work, typeof(*pool), > - release_work); > - > - synchronize_rcu(); > + struct zswap_pool *pool = container_of(to_rcu_work(work), > + typeof(*pool), release_work); > > /* nobody should have been able to get a ref... */ > WARN_ON(!percpu_ref_is_zero(&pool->ref)); > @@ -406,8 +404,8 @@ static void __zswap_pool_empty(struct percpu_ref *ref) > > list_del_rcu(&pool->list); > > - INIT_WORK(&pool->release_work, __zswap_pool_release); > - schedule_work(&pool->release_work); > + INIT_RCU_WORK(&pool->release_work, __zswap_pool_release); > + queue_rcu_work(system_percpu_wq, &pool->release_work); > > spin_unlock_bh(&zswap_pools_lock); > } > -- > 2.43.0 > ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC PATCH v4 1/3] mm/zswap: release retired pools via queue_rcu_work() instead of synchronize_rcu() 2026-08-31 15:20 ` Yosry Ahmed @ 2026-09-01 14:33 ` Jianyue Wu 0 siblings, 0 replies; 21+ messages in thread From: Jianyue Wu @ 2026-09-01 14:33 UTC (permalink / raw) To: Yosry Ahmed Cc: Johannes Weiner, Nhat Pham, Chengming Zhou, Andrew Morton, Chris Li, linux-mm, linux-kernel On Mon, Aug 31, 2026 at 11:20 PM Yosry Ahmed <yosry@kernel.org> wrote: > > On Sun, Aug 30, 2026 at 4:47 AM Jianyue Wu <wujianyue000@gmail.com> wrote: > > > > When a pool's last reference is dropped, __zswap_pool_empty() removes it > > from the pool list and schedules __zswap_pool_release(), which calls > > synchronize_rcu() to wait for readers before tearing the pool down. > > > > synchronize_rcu() is a synchronous, potentially long wait. Replace it > > with queue_rcu_work(): __zswap_pool_empty() hands the pool to > > queue_rcu_work(), which waits for a grace period asynchronously and then > > runs __zswap_pool_release() from a worker for the sleepable teardown > > (__zswap_pool_empty() can run in atomic context and must not block). > > The grace-period guarantee is unchanged; the retirement path just no > > longer blocks on it. > > > > Suggested-by: Yosry Ahmed <yosry@kernel.org> > > Signed-off-by: Jianyue Wu <wujianyue000@gmail.com> > > Why are the patches still tagged RFC? Exactly, should be removed, I dropped the RFC tag in the new version. > Anyway, with one nit below: > > Acked-by: Yosry Ahmed <yosry@kernel.org> > > > --- > > mm/zswap.c | 12 +++++------- > > 1 file changed, 5 insertions(+), 7 deletions(-) > > > > diff --git a/mm/zswap.c b/mm/zswap.c > > index 37f34e406c8e..0bb30e58950a 100644 > > --- a/mm/zswap.c > > +++ b/mm/zswap.c > > @@ -155,7 +155,7 @@ struct zswap_pool { > > struct crypto_acomp_ctx __percpu *acomp_ctx; > > struct percpu_ref ref; > > struct list_head list; > > - struct work_struct release_work; > > + struct rcu_work release_work; > > Seems like the convention is to use rwork in the name instead of work. Good point, I renamed it to release_rwork in the new version. Best regards, Jianyue ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC PATCH v4 1/3] mm/zswap: release retired pools via queue_rcu_work() instead of synchronize_rcu() 2026-08-30 11:47 ` [RFC PATCH v4 1/3] mm/zswap: release retired pools via queue_rcu_work() instead of synchronize_rcu() Jianyue Wu 2026-08-31 15:20 ` Yosry Ahmed @ 2026-09-01 15:38 ` Johannes Weiner 2026-09-02 0:53 ` Jianyue Wu 1 sibling, 1 reply; 21+ messages in thread From: Johannes Weiner @ 2026-09-01 15:38 UTC (permalink / raw) To: Jianyue Wu Cc: Yosry Ahmed, Nhat Pham, Chengming Zhou, Andrew Morton, Chris Li, linux-mm, linux-kernel On Sun, Aug 30, 2026 at 07:47:29PM +0800, Jianyue Wu wrote: > When a pool's last reference is dropped, __zswap_pool_empty() removes it > from the pool list and schedules __zswap_pool_release(), which calls > synchronize_rcu() to wait for readers before tearing the pool down. > > synchronize_rcu() is a synchronous, potentially long wait. Replace it > with queue_rcu_work(): __zswap_pool_empty() hands the pool to > queue_rcu_work(), which waits for a grace period asynchronously and then > runs __zswap_pool_release() from a worker for the sleepable teardown > (__zswap_pool_empty() can run in atomic context and must not block). > The grace-period guarantee is unchanged; the retirement path just no > longer blocks on it. > > Suggested-by: Yosry Ahmed <yosry@kernel.org> > Signed-off-by: Jianyue Wu <wujianyue000@gmail.com> With "release_rwork", Reviewed-by: Johannes Weiner <hannes@cmpxchg.org> ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC PATCH v4 1/3] mm/zswap: release retired pools via queue_rcu_work() instead of synchronize_rcu() 2026-09-01 15:38 ` Johannes Weiner @ 2026-09-02 0:53 ` Jianyue Wu 0 siblings, 0 replies; 21+ messages in thread From: Jianyue Wu @ 2026-09-02 0:53 UTC (permalink / raw) To: Johannes Weiner Cc: Yosry Ahmed, Nhat Pham, Chengming Zhou, Andrew Morton, Chris Li, linux-mm, linux-kernel On Tue, Sep 1, 2026 at 11:38 PM Johannes Weiner <hannes@cmpxchg.org> wrote: > > On Sun, Aug 30, 2026 at 07:47:29PM +0800, Jianyue Wu wrote: > > When a pool's last reference is dropped, __zswap_pool_empty() removes it > > from the pool list and schedules __zswap_pool_release(), which calls > > synchronize_rcu() to wait for readers before tearing the pool down. > > > > synchronize_rcu() is a synchronous, potentially long wait. Replace it > > with queue_rcu_work(): __zswap_pool_empty() hands the pool to > > queue_rcu_work(), which waits for a grace period asynchronously and then > > runs __zswap_pool_release() from a worker for the sleepable teardown > > (__zswap_pool_empty() can run in atomic context and must not block). > > The grace-period guarantee is unchanged; the retirement path just no > > longer blocks on it. > > > > Suggested-by: Yosry Ahmed <yosry@kernel.org> > > Signed-off-by: Jianyue Wu <wujianyue000@gmail.com> > > With "release_rwork", > > Reviewed-by: Johannes Weiner <hannes@cmpxchg.org> Thanks, I will use release_rwork instead. Best regards, Jianyue ^ permalink raw reply [flat|nested] 21+ messages in thread
* [RFC PATCH v4 2/3] mm/zswap: replace the zswap_pools list with a fixed pools array 2026-08-30 11:47 [RFC PATCH v4 0/3] mm/zswap: shrink zswap_entry via a fixed pool index Jianyue Wu 2026-08-30 11:47 ` [RFC PATCH v4 1/3] mm/zswap: release retired pools via queue_rcu_work() instead of synchronize_rcu() Jianyue Wu @ 2026-08-30 11:47 ` Jianyue Wu 2026-08-31 15:28 ` Yosry Ahmed 2026-09-01 16:13 ` Johannes Weiner 2026-08-30 11:47 ` [RFC PATCH v4 3/3] mm/zswap: reference the pool by index to shrink struct zswap_entry Jianyue Wu 2026-09-04 13:24 ` [PATCH v5 0/3] mm/zswap: shrink zswap_entry via a pool id Jianyue Wu 3 siblings, 2 replies; 21+ messages in thread From: Jianyue Wu @ 2026-08-30 11:47 UTC (permalink / raw) To: Johannes Weiner, Yosry Ahmed, Nhat Pham, Chengming Zhou, Andrew Morton Cc: Jianyue Wu, Chris Li, linux-mm, linux-kernel Originally zswap holds its pools on an RCU list whose head also serves as the "current pool". Only a handful of pools are ever live at once, since a new pool is only created when the compressor is (re)set and pools are reused across compressor switches. Hold the pools in a fixed ZSWAP_MAX_POOLS-element array so each pool has a stable slot number, and track the current pool with a separate rcu-protected pointer. Slot 0 is intentionally left unused (always NULL): a zeroed or incorrectly initialized pool index then resolves to NULL and trips a WARN rather than silently aliasing a live pool in another slot. The array keeps the same RCU publish/retire discipline the list had, so lookup and teardown stay equivalent. A fully-constructed pool is stored into its slot as the last step of zswap_pool_create(), so array walkers only ever observe a NULL slot or a ready pool. Pool creation is serialized by the module-wide kernel param mutex (all built-in params share one lock) and otherwise only happens during single-threaded init, so no two creators race for a slot. zswap_pools_lock still serializes the store against a retiring pool clearing its slot in __zswap_pool_empty(). Behavior change: the fixed array bounds the number of simultaneously live pools at ZSWAP_MAX_POOLS - 1 (15, since slot 0 is reserved), whereas the old list was unbounded. A pool is only live while it is the current pool or still has stored pages referencing it, and pools are reused across compressor switches, so 15 is far more than any real configuration needs. Once all slots are occupied, creating a pool for a 16th distinct compressor fails: zswap_pool_create() errors and returns NULL, and the compressor switch is rejected with -EINVAL rather than silently succeeding. The cap can be raised by increasing ZSWAP_MAX_POOLS (bounded by the u8 slot index, so up to 256). Suggested-by: Nhat Pham <nphamcs@gmail.com> Suggested-by: Yosry Ahmed <yosry@kernel.org> Signed-off-by: Jianyue Wu <wujianyue000@gmail.com> --- mm/zswap.c | 97 ++++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 72 insertions(+), 25 deletions(-) diff --git a/mm/zswap.c b/mm/zswap.c index 0bb30e58950a..b3b5e2887c00 100644 --- a/mm/zswap.c +++ b/mm/zswap.c @@ -13,6 +13,7 @@ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt +#include <linux/cleanup.h> #include <linux/module.h> #include <linux/cpu.h> #include <linux/highmem.h> @@ -154,12 +155,27 @@ struct zswap_pool { struct zs_pool *zs_pool; struct crypto_acomp_ctx __percpu *acomp_ctx; struct percpu_ref ref; - struct list_head list; struct rcu_work release_work; struct hlist_node node; + u8 idx; char tfm_name[CRYPTO_MAX_ALG_NAME]; }; +#define ZSWAP_MAX_POOLS 16 +/* + * Slot 0 is intentionally never used: it stays NULL so that a zeroed or + * incorrectly initialized pool->idx resolves to NULL (and trips a WARN) + * instead of silently aliasing a live pool in another slot. + */ +#define ZSWAP_FIRST_POOL_SLOT 1 +static struct zswap_pool __rcu *zswap_pools[ZSWAP_MAX_POOLS]; +static_assert(ZSWAP_MAX_POOLS - 1 <= U8_MAX); +/* + * The current pool (NULL if none): an alias of one zswap_pools[] slot. + * It always holds a ref, so a pool is never retired while it is current. + */ +static struct zswap_pool __rcu *zswap_current_pool; + /* Global LRU lists shared by all zswap pools. */ static struct list_lru zswap_list_lru; @@ -200,9 +216,6 @@ struct zswap_entry { static struct xarray *zswap_trees[MAX_SWAPFILES]; static unsigned int nr_zswap_trees[MAX_SWAPFILES]; -/* RCU-protected iteration */ -static LIST_HEAD(zswap_pools); -/* protects zswap_pools list modification */ static DEFINE_SPINLOCK(zswap_pools_lock); /* pool counter to provide unique names to zsmalloc */ static atomic_t zswap_pools_count = ATOMIC_INIT(0); @@ -270,6 +283,31 @@ static void acomp_ctx_free(struct crypto_acomp_ctx *acomp_ctx) acomp_ctx->buffer = NULL; } +/* + * Publish a fully-constructed pool into a free array slot. Pool creation is + * serialized by the module-wide kernel param mutex (all built-in params share + * one lock) and only otherwise happens during single-threaded init, so no two + * creators race for a slot. The pool is complete before it is stored, and + * zswap_pools_lock still serializes this store against a concurrent retiring + * pool clearing its slot in __zswap_pool_empty(), so array walkers only ever + * observe a NULL slot or a ready pool. + */ +static int zswap_pool_assign_slot(struct zswap_pool *pool) +{ + int i; + + guard(spinlock_bh)(&zswap_pools_lock); + for (i = ZSWAP_FIRST_POOL_SLOT; i < ZSWAP_MAX_POOLS; i++) { + if (!rcu_access_pointer(zswap_pools[i])) { + pool->idx = i; + rcu_assign_pointer(zswap_pools[i], pool); + return i; + } + } + + return -ENOSPC; +} + static struct zswap_pool *zswap_pool_create(char *compressor) { struct zswap_pool *pool; @@ -313,19 +351,29 @@ static struct zswap_pool *zswap_pool_create(char *compressor) if (ret) goto cpuhp_add_fail; - /* being the current pool takes 1 ref; this func expects the - * caller to always add the new pool as the current pool + /* + * The initial ref keeps the pool alive while it is current. Stored + * entries take additional refs so a retired pool remains alive while + * any entries still reference it. */ ret = percpu_ref_init(&pool->ref, __zswap_pool_empty, PERCPU_REF_ALLOW_REINIT, GFP_KERNEL); if (ret) goto ref_fail; - INIT_LIST_HEAD(&pool->list); + + ret = zswap_pool_assign_slot(pool); + if (ret < 0) { + pr_err("cannot create more than %d pools\n", + ZSWAP_MAX_POOLS - ZSWAP_FIRST_POOL_SLOT); + goto slot_fail; + } zswap_pool_debug("created", pool); return pool; +slot_fail: + percpu_ref_exit(&pool->ref); ref_fail: cpuhp_state_remove_instance(CPUHP_MM_ZSWP_POOL_PREPARE, &pool->node); @@ -386,7 +434,6 @@ static void __zswap_pool_release(struct work_struct *work) WARN_ON(!percpu_ref_is_zero(&pool->ref)); percpu_ref_exit(&pool->ref); - /* pool is now off zswap_pools list and has no references. */ zswap_pool_destroy(pool); } @@ -402,7 +449,7 @@ static void __zswap_pool_empty(struct percpu_ref *ref) WARN_ON(pool == zswap_pool_current()); - list_del_rcu(&pool->list); + rcu_assign_pointer(zswap_pools[pool->idx], NULL); INIT_RCU_WORK(&pool->release_work, __zswap_pool_release); queue_rcu_work(system_percpu_wq, &pool->release_work); @@ -433,7 +480,8 @@ static struct zswap_pool *__zswap_pool_current(void) { struct zswap_pool *pool; - pool = list_first_or_null_rcu(&zswap_pools, typeof(*pool), list); + pool = rcu_dereference_check(zswap_current_pool, + lockdep_is_held(&zswap_pools_lock)); WARN_ONCE(!pool && zswap_has_pool, "%s: no page storage pool!\n", __func__); @@ -466,11 +514,12 @@ static struct zswap_pool *zswap_pool_current_get(void) static struct zswap_pool *zswap_pool_find_get(char *compressor) { struct zswap_pool *pool; + int i; - assert_spin_locked(&zswap_pools_lock); - - list_for_each_entry_rcu(pool, &zswap_pools, list) { - if (strcmp(pool->tfm_name, compressor)) + for (i = ZSWAP_FIRST_POOL_SLOT; i < ZSWAP_MAX_POOLS; i++) { + pool = rcu_dereference_protected(zswap_pools[i], + lockdep_is_held(&zswap_pools_lock)); + if (!pool || strcmp(pool->tfm_name, compressor)) continue; /* if we can't get it, it's about to be destroyed */ if (!zswap_pool_tryget(pool)) @@ -495,10 +544,14 @@ unsigned long zswap_total_pages(void) { struct zswap_pool *pool; unsigned long total = 0; + int i; rcu_read_lock(); - list_for_each_entry_rcu(pool, &zswap_pools, list) - total += zs_get_total_pages(pool->zs_pool); + for (i = ZSWAP_FIRST_POOL_SLOT; i < ZSWAP_MAX_POOLS; i++) { + pool = rcu_dereference(zswap_pools[i]); + if (pool) + total += zs_get_total_pages(pool->zs_pool); + } rcu_read_unlock(); return total; @@ -560,7 +613,6 @@ static int zswap_compressor_param_set(const char *val, const struct kernel_param if (pool) { zswap_pool_debug("using existing", pool); WARN_ON(pool == zswap_pool_current()); - list_del_rcu(&pool->list); } spin_unlock_bh(&zswap_pools_lock); @@ -588,15 +640,9 @@ static int zswap_compressor_param_set(const char *val, const struct kernel_param if (!ret) { put_pool = zswap_pool_current(); - list_add_rcu(&pool->list, &zswap_pools); + rcu_assign_pointer(zswap_current_pool, pool); zswap_has_pool = true; } else if (pool) { - /* - * Add the possibly pre-existing pool to the end of the pools - * list; if it's new (and empty) then it'll be removed and - * destroyed by the put after we drop the lock - */ - list_add_tail_rcu(&pool->list, &zswap_pools); put_pool = pool; } @@ -1801,7 +1847,8 @@ static int zswap_setup(void) pool = __zswap_pool_create_fallback(); if (pool) { pr_info("loaded using pool %s\n", pool->tfm_name); - list_add(&pool->list, &zswap_pools); + /* zswap_pool_create() already stored the pool in its array slot. */ + rcu_assign_pointer(zswap_current_pool, pool); zswap_has_pool = true; static_branch_enable(&zswap_ever_enabled); } else { -- 2.43.0 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC PATCH v4 2/3] mm/zswap: replace the zswap_pools list with a fixed pools array 2026-08-30 11:47 ` [RFC PATCH v4 2/3] mm/zswap: replace the zswap_pools list with a fixed pools array Jianyue Wu @ 2026-08-31 15:28 ` Yosry Ahmed 2026-09-01 16:13 ` Johannes Weiner 1 sibling, 0 replies; 21+ messages in thread From: Yosry Ahmed @ 2026-08-31 15:28 UTC (permalink / raw) To: Jianyue Wu Cc: Johannes Weiner, Nhat Pham, Chengming Zhou, Andrew Morton, Chris Li, linux-mm, linux-kernel On Sun, Aug 30, 2026 at 4:47 AM Jianyue Wu <wujianyue000@gmail.com> wrote: > > Originally zswap holds its pools on an RCU list whose head also serves > as the "current pool". Only a handful of pools are ever live at once, > since a new pool is only created when the compressor is (re)set and > pools are reused across compressor switches. > > Hold the pools in a fixed ZSWAP_MAX_POOLS-element array so each pool > has a stable slot number, and track the current pool with a separate > rcu-protected pointer. > > Slot 0 is intentionally left unused (always NULL): a zeroed or > incorrectly initialized pool index then resolves to NULL and trips a > WARN rather than silently aliasing a live pool in another slot. > > The array keeps the same RCU publish/retire discipline the list had, > so lookup and teardown stay equivalent. A fully-constructed pool is > stored into its slot as the last step of zswap_pool_create(), so array > walkers only ever observe a NULL slot or a ready pool. Pool creation > is serialized by the module-wide kernel param mutex (all built-in > params share one lock) and otherwise only happens during > single-threaded init, so no two creators race for a slot. > zswap_pools_lock still serializes the store against a retiring pool > clearing its slot in __zswap_pool_empty(). > > Behavior change: the fixed array bounds the number of simultaneously > live pools at ZSWAP_MAX_POOLS - 1 (15, since slot 0 is reserved), > whereas the old list was unbounded. A pool is only live while it is > the current pool or still has stored pages referencing it, and pools > are reused across compressor switches, so 15 is far more than any real > configuration needs. Once all slots are occupied, creating a pool for > a 16th distinct compressor fails: zswap_pool_create() errors and > returns NULL, and the compressor switch is rejected with -EINVAL > rather than silently succeeding. The cap can be raised by increasing > ZSWAP_MAX_POOLS (bounded by the u8 slot index, so up to 256). > > Suggested-by: Nhat Pham <nphamcs@gmail.com> > Suggested-by: Yosry Ahmed <yosry@kernel.org> > Signed-off-by: Jianyue Wu <wujianyue000@gmail.com> Acked-by: Yosry Ahmed <yosry@kernel.org> ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC PATCH v4 2/3] mm/zswap: replace the zswap_pools list with a fixed pools array 2026-08-30 11:47 ` [RFC PATCH v4 2/3] mm/zswap: replace the zswap_pools list with a fixed pools array Jianyue Wu 2026-08-31 15:28 ` Yosry Ahmed @ 2026-09-01 16:13 ` Johannes Weiner 2026-09-02 0:50 ` Jianyue Wu 1 sibling, 1 reply; 21+ messages in thread From: Johannes Weiner @ 2026-09-01 16:13 UTC (permalink / raw) To: Jianyue Wu Cc: Yosry Ahmed, Nhat Pham, Chengming Zhou, Andrew Morton, Chris Li, linux-mm, linux-kernel On Sun, Aug 30, 2026 at 07:47:30PM +0800, Jianyue Wu wrote: > Originally zswap holds its pools on an RCU list whose head also serves > as the "current pool". Only a handful of pools are ever live at once, > since a new pool is only created when the compressor is (re)set and > pools are reused across compressor switches. > > Hold the pools in a fixed ZSWAP_MAX_POOLS-element array so each pool > has a stable slot number, and track the current pool with a separate > rcu-protected pointer. > > Slot 0 is intentionally left unused (always NULL): a zeroed or > incorrectly initialized pool index then resolves to NULL and trips a > WARN rather than silently aliasing a live pool in another slot. > > The array keeps the same RCU publish/retire discipline the list had, > so lookup and teardown stay equivalent. A fully-constructed pool is > stored into its slot as the last step of zswap_pool_create(), so array > walkers only ever observe a NULL slot or a ready pool. Pool creation > is serialized by the module-wide kernel param mutex (all built-in > params share one lock) and otherwise only happens during > single-threaded init, so no two creators race for a slot. > zswap_pools_lock still serializes the store against a retiring pool > clearing its slot in __zswap_pool_empty(). > > Behavior change: the fixed array bounds the number of simultaneously > live pools at ZSWAP_MAX_POOLS - 1 (15, since slot 0 is reserved), > whereas the old list was unbounded. A pool is only live while it is > the current pool or still has stored pages referencing it, and pools > are reused across compressor switches, so 15 is far more than any real > configuration needs. Once all slots are occupied, creating a pool for > a 16th distinct compressor fails: zswap_pool_create() errors and > returns NULL, and the compressor switch is rejected with -EINVAL > rather than silently succeeding. The cap can be raised by increasing > ZSWAP_MAX_POOLS (bounded by the u8 slot index, so up to 256). > > Suggested-by: Nhat Pham <nphamcs@gmail.com> > Suggested-by: Yosry Ahmed <yosry@kernel.org> > Signed-off-by: Jianyue Wu <wujianyue000@gmail.com> > --- > mm/zswap.c | 97 ++++++++++++++++++++++++++++++++++++++++-------------- > 1 file changed, 72 insertions(+), 25 deletions(-) > > diff --git a/mm/zswap.c b/mm/zswap.c > index 0bb30e58950a..b3b5e2887c00 100644 > --- a/mm/zswap.c > +++ b/mm/zswap.c > @@ -13,6 +13,7 @@ > > #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > > +#include <linux/cleanup.h> > #include <linux/module.h> > #include <linux/cpu.h> > #include <linux/highmem.h> > @@ -154,12 +155,27 @@ struct zswap_pool { > struct zs_pool *zs_pool; > struct crypto_acomp_ctx __percpu *acomp_ctx; > struct percpu_ref ref; > - struct list_head list; > struct rcu_work release_work; > struct hlist_node node; > + u8 idx; > char tfm_name[CRYPTO_MAX_ALG_NAME]; > }; > > +#define ZSWAP_MAX_POOLS 16 It's unlikely to happen, but this is a super annoying failure mode. User would have to kill something, delete shmem/tmpfs, or swapoff. And it's not obvious which entries are in which pool. Wouldn't an idr make more sense? > @@ -270,6 +283,31 @@ static void acomp_ctx_free(struct crypto_acomp_ctx *acomp_ctx) > acomp_ctx->buffer = NULL; > } > > +/* > + * Publish a fully-constructed pool into a free array slot. Pool creation is > + * serialized by the module-wide kernel param mutex (all built-in params share > + * one lock) and only otherwise happens during single-threaded init, so no two > + * creators race for a slot. The pool is complete before it is stored, and > + * zswap_pools_lock still serializes this store against a concurrent retiring > + * pool clearing its slot in __zswap_pool_empty(), so array walkers only ever > + * observe a NULL slot or a ready pool. > + */ > +static int zswap_pool_assign_slot(struct zswap_pool *pool) > +{ > + int i; > + > + guard(spinlock_bh)(&zswap_pools_lock); > + for (i = ZSWAP_FIRST_POOL_SLOT; i < ZSWAP_MAX_POOLS; i++) { > + if (!rcu_access_pointer(zswap_pools[i])) { > + pool->idx = i; > + rcu_assign_pointer(zswap_pools[i], pool); > + return i; > + } > + } > + > + return -ENOSPC; > +} It was kind of overdue, but with this now requiring a pool walk as well, it would be better to factor out a find_or_create function? Something like: static struct zswap_pool *zswap_pool_find_or_create(char *compressor) { struct zswap_pool *pool, *new_pool = NULL; u8 id, new_id = 0; insert_new: spin_lock_bh(&zswap_pools_lock); idr_for_each_entry(&zswap_pools, pool, id) { if (pool && !strcmp(pool->tfm_name, compressor) && zswap_pool_tryget(pool)) { if (new_pool) { pool_put(new_pool); idr_free(&zswap_pools, new_id); } spin_unlock_bh(&zswap_pools_lock); return pool; } } if (new_pool) { idr_replace(&zswap_pools, new_pool, new_id); spin_unlock_bh(&zswap_pools_lock); return new_pool; } spin_unlock_bh(&zswap_pools_lock); new_pool = pool_alloc(); if (!new_pool) ... new_id = idr_alloc(&zswap_pools, NULL, 1, 256, GFP_KERNEL); if (new_id < 0) ... goto insert_new; } ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC PATCH v4 2/3] mm/zswap: replace the zswap_pools list with a fixed pools array 2026-09-01 16:13 ` Johannes Weiner @ 2026-09-02 0:50 ` Jianyue Wu 2026-09-03 12:59 ` Jianyue Wu 0 siblings, 1 reply; 21+ messages in thread From: Jianyue Wu @ 2026-09-02 0:50 UTC (permalink / raw) To: Johannes Weiner Cc: Yosry Ahmed, Nhat Pham, Chengming Zhou, Andrew Morton, Chris Li, linux-mm, linux-kernel On Wed, Sep 2, 2026 at 12:13 AM Johannes Weiner <hannes@cmpxchg.org> wrote: > > On Sun, Aug 30, 2026 at 07:47:30PM +0800, Jianyue Wu wrote: > > Originally zswap holds its pools on an RCU list whose head also serves > > as the "current pool". Only a handful of pools are ever live at once, > > since a new pool is only created when the compressor is (re)set and > > pools are reused across compressor switches. > > > > Hold the pools in a fixed ZSWAP_MAX_POOLS-element array so each pool > > has a stable slot number, and track the current pool with a separate > > rcu-protected pointer. > > > > Slot 0 is intentionally left unused (always NULL): a zeroed or > > incorrectly initialized pool index then resolves to NULL and trips a > > WARN rather than silently aliasing a live pool in another slot. > > > > The array keeps the same RCU publish/retire discipline the list had, > > so lookup and teardown stay equivalent. A fully-constructed pool is > > stored into its slot as the last step of zswap_pool_create(), so array > > walkers only ever observe a NULL slot or a ready pool. Pool creation > > is serialized by the module-wide kernel param mutex (all built-in > > params share one lock) and otherwise only happens during > > single-threaded init, so no two creators race for a slot. > > zswap_pools_lock still serializes the store against a retiring pool > > clearing its slot in __zswap_pool_empty(). > > > > Behavior change: the fixed array bounds the number of simultaneously > > live pools at ZSWAP_MAX_POOLS - 1 (15, since slot 0 is reserved), > > whereas the old list was unbounded. A pool is only live while it is > > the current pool or still has stored pages referencing it, and pools > > are reused across compressor switches, so 15 is far more than any real > > configuration needs. Once all slots are occupied, creating a pool for > > a 16th distinct compressor fails: zswap_pool_create() errors and > > returns NULL, and the compressor switch is rejected with -EINVAL > > rather than silently succeeding. The cap can be raised by increasing > > ZSWAP_MAX_POOLS (bounded by the u8 slot index, so up to 256). > > > > Suggested-by: Nhat Pham <nphamcs@gmail.com> > > Suggested-by: Yosry Ahmed <yosry@kernel.org> > > Signed-off-by: Jianyue Wu <wujianyue000@gmail.com> > > --- > > mm/zswap.c | 97 ++++++++++++++++++++++++++++++++++++++++-------------- > > 1 file changed, 72 insertions(+), 25 deletions(-) > > > > diff --git a/mm/zswap.c b/mm/zswap.c > > index 0bb30e58950a..b3b5e2887c00 100644 > > --- a/mm/zswap.c > > +++ b/mm/zswap.c > > @@ -13,6 +13,7 @@ > > > > #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > > > > +#include <linux/cleanup.h> > > #include <linux/module.h> > > #include <linux/cpu.h> > > #include <linux/highmem.h> > > @@ -154,12 +155,27 @@ struct zswap_pool { > > struct zs_pool *zs_pool; > > struct crypto_acomp_ctx __percpu *acomp_ctx; > > struct percpu_ref ref; > > - struct list_head list; > > struct rcu_work release_work; > > struct hlist_node node; > > + u8 idx; > > char tfm_name[CRYPTO_MAX_ALG_NAME]; > > }; > > > > +#define ZSWAP_MAX_POOLS 16 > > It's unlikely to happen, but this is a super annoying failure > mode. User would have to kill something, delete shmem/tmpfs, or > swapoff. And it's not obvious which entries are in which pool. > > Wouldn't an idr make more sense? > > > @@ -270,6 +283,31 @@ static void acomp_ctx_free(struct crypto_acomp_ctx *acomp_ctx) > > acomp_ctx->buffer = NULL; > > } > > > > +/* > > + * Publish a fully-constructed pool into a free array slot. Pool creation is > > + * serialized by the module-wide kernel param mutex (all built-in params share > > + * one lock) and only otherwise happens during single-threaded init, so no two > > + * creators race for a slot. The pool is complete before it is stored, and > > + * zswap_pools_lock still serializes this store against a concurrent retiring > > + * pool clearing its slot in __zswap_pool_empty(), so array walkers only ever > > + * observe a NULL slot or a ready pool. > > + */ > > +static int zswap_pool_assign_slot(struct zswap_pool *pool) > > +{ > > + int i; > > + > > + guard(spinlock_bh)(&zswap_pools_lock); > > + for (i = ZSWAP_FIRST_POOL_SLOT; i < ZSWAP_MAX_POOLS; i++) { > > + if (!rcu_access_pointer(zswap_pools[i])) { > > + pool->idx = i; > > + rcu_assign_pointer(zswap_pools[i], pool); > > + return i; > > + } > > + } > > + > > + return -ENOSPC; > > +} > > It was kind of overdue, but with this now requiring a pool walk as > well, it would be better to factor out a find_or_create function? > > Something like: > > static struct zswap_pool *zswap_pool_find_or_create(char *compressor) > { > struct zswap_pool *pool, *new_pool = NULL; > u8 id, new_id = 0; > > insert_new: > spin_lock_bh(&zswap_pools_lock); > idr_for_each_entry(&zswap_pools, pool, id) { > if (pool && !strcmp(pool->tfm_name, compressor) && zswap_pool_tryget(pool)) { > if (new_pool) { > pool_put(new_pool); > idr_free(&zswap_pools, new_id); > } > spin_unlock_bh(&zswap_pools_lock); > return pool; > } > } > if (new_pool) { > idr_replace(&zswap_pools, new_pool, new_id); > spin_unlock_bh(&zswap_pools_lock); > return new_pool; > } > spin_unlock_bh(&zswap_pools_lock); > > new_pool = pool_alloc(); > if (!new_pool) > ... > new_id = idr_alloc(&zswap_pools, NULL, 1, 256, GFP_KERNEL); > if (new_id < 0) > ... > goto insert_new; > } Agreed. Hitting the 15-pool cap would be an annoying failure mode, even if it should be rare. I will switch this to an IDR with IDs in the 1..255 range, keeping 0 as the invalid entry value, and factor the lookup/allocation path into a zswap_pool_find_or_create() helper. Best regards, Jianyue ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC PATCH v4 2/3] mm/zswap: replace the zswap_pools list with a fixed pools array 2026-09-02 0:50 ` Jianyue Wu @ 2026-09-03 12:59 ` Jianyue Wu 0 siblings, 0 replies; 21+ messages in thread From: Jianyue Wu @ 2026-09-03 12:59 UTC (permalink / raw) To: Johannes Weiner Cc: Yosry Ahmed, Nhat Pham, Chengming Zhou, Andrew Morton, Chris Li, linux-mm, linux-kernel On Wed, Sep 2, 2026 at 8:50 AM Jianyue Wu <wujianyue000@gmail.com> wrote: > > On Wed, Sep 2, 2026 at 12:13 AM Johannes Weiner <hannes@cmpxchg.org> wrote: > > > > On Sun, Aug 30, 2026 at 07:47:30PM +0800, Jianyue Wu wrote: > > > Originally zswap holds its pools on an RCU list whose head also serves > > > as the "current pool". Only a handful of pools are ever live at once, > > > since a new pool is only created when the compressor is (re)set and > > > pools are reused across compressor switches. > > > > > > Hold the pools in a fixed ZSWAP_MAX_POOLS-element array so each pool > > > has a stable slot number, and track the current pool with a separate > > > rcu-protected pointer. > > > > > > Slot 0 is intentionally left unused (always NULL): a zeroed or > > > incorrectly initialized pool index then resolves to NULL and trips a > > > WARN rather than silently aliasing a live pool in another slot. > > > > > > The array keeps the same RCU publish/retire discipline the list had, > > > so lookup and teardown stay equivalent. A fully-constructed pool is > > > stored into its slot as the last step of zswap_pool_create(), so array > > > walkers only ever observe a NULL slot or a ready pool. Pool creation > > > is serialized by the module-wide kernel param mutex (all built-in > > > params share one lock) and otherwise only happens during > > > single-threaded init, so no two creators race for a slot. > > > zswap_pools_lock still serializes the store against a retiring pool > > > clearing its slot in __zswap_pool_empty(). > > > > > > Behavior change: the fixed array bounds the number of simultaneously > > > live pools at ZSWAP_MAX_POOLS - 1 (15, since slot 0 is reserved), > > > whereas the old list was unbounded. A pool is only live while it is > > > the current pool or still has stored pages referencing it, and pools > > > are reused across compressor switches, so 15 is far more than any real > > > configuration needs. Once all slots are occupied, creating a pool for > > > a 16th distinct compressor fails: zswap_pool_create() errors and > > > returns NULL, and the compressor switch is rejected with -EINVAL > > > rather than silently succeeding. The cap can be raised by increasing > > > ZSWAP_MAX_POOLS (bounded by the u8 slot index, so up to 256). > > > > > > Suggested-by: Nhat Pham <nphamcs@gmail.com> > > > Suggested-by: Yosry Ahmed <yosry@kernel.org> > > > Signed-off-by: Jianyue Wu <wujianyue000@gmail.com> > > > --- > > > mm/zswap.c | 97 ++++++++++++++++++++++++++++++++++++++++-------------- > > > 1 file changed, 72 insertions(+), 25 deletions(-) > > > > > > diff --git a/mm/zswap.c b/mm/zswap.c > > > index 0bb30e58950a..b3b5e2887c00 100644 > > > --- a/mm/zswap.c > > > +++ b/mm/zswap.c > > > @@ -13,6 +13,7 @@ > > > > > > #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > > > > > > +#include <linux/cleanup.h> > > > #include <linux/module.h> > > > #include <linux/cpu.h> > > > #include <linux/highmem.h> > > > @@ -154,12 +155,27 @@ struct zswap_pool { > > > struct zs_pool *zs_pool; > > > struct crypto_acomp_ctx __percpu *acomp_ctx; > > > struct percpu_ref ref; > > > - struct list_head list; > > > struct rcu_work release_work; > > > struct hlist_node node; > > > + u8 idx; > > > char tfm_name[CRYPTO_MAX_ALG_NAME]; > > > }; > > > > > > +#define ZSWAP_MAX_POOLS 16 > > > > It's unlikely to happen, but this is a super annoying failure > > mode. User would have to kill something, delete shmem/tmpfs, or > > swapoff. And it's not obvious which entries are in which pool. > > > > Wouldn't an idr make more sense? > > > > > @@ -270,6 +283,31 @@ static void acomp_ctx_free(struct crypto_acomp_ctx *acomp_ctx) > > > acomp_ctx->buffer = NULL; > > > } > > > > > > +/* > > > + * Publish a fully-constructed pool into a free array slot. Pool creation is > > > + * serialized by the module-wide kernel param mutex (all built-in params share > > > + * one lock) and only otherwise happens during single-threaded init, so no two > > > + * creators race for a slot. The pool is complete before it is stored, and > > > + * zswap_pools_lock still serializes this store against a concurrent retiring > > > + * pool clearing its slot in __zswap_pool_empty(), so array walkers only ever > > > + * observe a NULL slot or a ready pool. > > > + */ > > > +static int zswap_pool_assign_slot(struct zswap_pool *pool) > > > +{ > > > + int i; > > > + > > > + guard(spinlock_bh)(&zswap_pools_lock); > > > + for (i = ZSWAP_FIRST_POOL_SLOT; i < ZSWAP_MAX_POOLS; i++) { > > > + if (!rcu_access_pointer(zswap_pools[i])) { > > > + pool->idx = i; > > > + rcu_assign_pointer(zswap_pools[i], pool); > > > + return i; > > > + } > > > + } > > > + > > > + return -ENOSPC; > > > +} > > > > It was kind of overdue, but with this now requiring a pool walk as > > well, it would be better to factor out a find_or_create function? > > > > Something like: > > > > static struct zswap_pool *zswap_pool_find_or_create(char *compressor) > > { > > struct zswap_pool *pool, *new_pool = NULL; > > u8 id, new_id = 0; > > > > insert_new: > > spin_lock_bh(&zswap_pools_lock); > > idr_for_each_entry(&zswap_pools, pool, id) { > > if (pool && !strcmp(pool->tfm_name, compressor) && zswap_pool_tryget(pool)) { > > if (new_pool) { > > pool_put(new_pool); > > idr_free(&zswap_pools, new_id); > > } > > spin_unlock_bh(&zswap_pools_lock); > > return pool; > > } > > } > > if (new_pool) { > > idr_replace(&zswap_pools, new_pool, new_id); > > spin_unlock_bh(&zswap_pools_lock); > > return new_pool; > > } > > spin_unlock_bh(&zswap_pools_lock); > > > > new_pool = pool_alloc(); > > if (!new_pool) > > ... > > new_id = idr_alloc(&zswap_pools, NULL, 1, 256, GFP_KERNEL); > > if (new_id < 0) > > ... > > goto insert_new; > > } > Agreed. Hitting the 15-pool cap would be an annoying failure mode, even > if it should be rare. > I will switch this to an IDR with IDs in the 1..255 range, keeping 0 as > the invalid entry value, and factor the lookup/allocation path into a > zswap_pool_find_or_create() helper. > > Best regards, > Jianyue Hello Johannes, I have this working locally, but I am still testing and some parts still need clarification. 1. Could I use xarray here? Similar to idr, can naturally start from 1, and zswap already uses xarray for the entry trees. i.e. the pool table is an allocating xarray (XA_FLAGS_ALLOC1, ids 1..255, 0 reserved). 2. Compressor updates are already serialized by the param lock, and the only other create path is zswap_setup() during init (or when enabling zswap). That does not run alongside another creator, so I think we can keep the simpler lookup-then-create flow. It is almost the same as the original list code, just using the xarray API. Best regards, Jianyue ^ permalink raw reply [flat|nested] 21+ messages in thread
* [RFC PATCH v4 3/3] mm/zswap: reference the pool by index to shrink struct zswap_entry 2026-08-30 11:47 [RFC PATCH v4 0/3] mm/zswap: shrink zswap_entry via a fixed pool index Jianyue Wu 2026-08-30 11:47 ` [RFC PATCH v4 1/3] mm/zswap: release retired pools via queue_rcu_work() instead of synchronize_rcu() Jianyue Wu 2026-08-30 11:47 ` [RFC PATCH v4 2/3] mm/zswap: replace the zswap_pools list with a fixed pools array Jianyue Wu @ 2026-08-30 11:47 ` Jianyue Wu 2026-08-31 15:30 ` Yosry Ahmed 2026-09-04 13:24 ` [PATCH v5 0/3] mm/zswap: shrink zswap_entry via a pool id Jianyue Wu 3 siblings, 1 reply; 21+ messages in thread From: Jianyue Wu @ 2026-08-30 11:47 UTC (permalink / raw) To: Johannes Weiner, Yosry Ahmed, Nhat Pham, Chengming Zhou, Andrew Morton Cc: Jianyue Wu, Chris Li, linux-mm, linux-kernel struct zswap_entry is one allocation per stored page, so its size is pure overhead. It currently embeds an 8-byte pool pointer, even though the live pools now sit in a small fixed array indexed by a u8 slot number. Replace the per-entry pool pointer with that u8 slot index and resolve it through the fixed pool array. A live entry holds a reference to its pool, so the slot cannot be reused under it. The lookup therefore needs no RCU read-side section or zswap_pools_lock. The u8 fits in the padding after the bool referenced field, shrinking the entry from 56 to 48 bytes on x86_64. This raises objs_per_slab from 73 to 85 and saves about 2MiB of metadata per 1GiB of data held in zswap. Suggested-by: Chris Li <chrisl@kernel.org> Signed-off-by: Jianyue Wu <wujianyue000@gmail.com> --- mm/zswap.c | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/mm/zswap.c b/mm/zswap.c index b3b5e2887c00..521e0187bcd1 100644 --- a/mm/zswap.c +++ b/mm/zswap.c @@ -198,7 +198,7 @@ static struct shrinker *zswap_shrinker; * writeback logic. The entry is only reclaimed by the writeback * logic if referenced is unset. See comments in the shrinker * section for context. - * pool - the zswap_pool the entry's data is in + * pool_idx - slot of the zswap_pool that the entry's data is in. * handle - zsmalloc allocation handle that stores the compressed page data * objcg - the obj_cgroup that the compressed memory is charged to * lru - handle to the pool's lru used to evict pages. @@ -207,12 +207,22 @@ struct zswap_entry { swp_entry_t swpentry; unsigned int length; bool referenced; - struct zswap_pool *pool; + u8 pool_idx; unsigned long handle; struct obj_cgroup *objcg; struct list_head lru; }; +static struct zswap_pool *zswap_entry_pool(struct zswap_entry *entry) +{ + /* + * A live entry holds a pool reference, so the slot stays valid with no + * RCU read-side section. The != 0 check marks access protected by + * the reference. A live entry never uses the reserved slot 0. + */ + return rcu_dereference_check(zswap_pools[entry->pool_idx], entry->pool_idx != 0); +} + static struct xarray *zswap_trees[MAX_SWAPFILES]; static unsigned int nr_zswap_trees[MAX_SWAPFILES]; @@ -808,9 +818,13 @@ static void zswap_entry_cache_free(struct zswap_entry *entry) */ static void zswap_entry_free(struct zswap_entry *entry) { + struct zswap_pool *pool = zswap_entry_pool(entry); + zswap_lru_del(entry); - zs_free(entry->pool->zs_pool, entry->handle); - zswap_pool_put(entry->pool); + if (!WARN_ON_ONCE(!pool)) { + zs_free(pool->zs_pool, entry->handle); + zswap_pool_put(pool); + } if (entry->objcg) { obj_cgroup_uncharge_zswap(entry->objcg, entry->length); obj_cgroup_put(entry->objcg); @@ -967,12 +981,15 @@ static bool zswap_compress(struct page *page, struct zswap_entry *entry, static bool zswap_decompress(struct zswap_entry *entry, struct folio *folio) { - struct zswap_pool *pool = entry->pool; + struct zswap_pool *pool = zswap_entry_pool(entry); struct scatterlist input[2]; /* zsmalloc returns an SG list 1-2 entries */ struct scatterlist output; struct crypto_acomp_ctx *acomp_ctx; int ret = 0, dlen; + if (WARN_ON_ONCE(!pool)) + return false; + acomp_ctx = raw_cpu_ptr(pool->acomp_ctx); mutex_lock(&acomp_ctx->mutex); zs_obj_read_sg_begin(pool->zs_pool, entry->handle, input, entry->length); @@ -1008,7 +1025,7 @@ static bool zswap_decompress(struct zswap_entry *entry, struct folio *folio) pr_alert_ratelimited("Decompression error from zswap (%d:%lu %s %u->%d)\n", swp_type(entry->swpentry), swp_offset(entry->swpentry), - entry->pool->tfm_name, + pool->tfm_name, entry->length, dlen); return false; } @@ -1511,7 +1528,7 @@ static bool zswap_store_page(struct page *page, * The publishing order matters to prevent writeback from seeing * an incoherent entry. */ - entry->pool = pool; + entry->pool_idx = pool->idx; entry->swpentry = page_swpentry; entry->objcg = objcg; entry->referenced = true; -- 2.43.0 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC PATCH v4 3/3] mm/zswap: reference the pool by index to shrink struct zswap_entry 2026-08-30 11:47 ` [RFC PATCH v4 3/3] mm/zswap: reference the pool by index to shrink struct zswap_entry Jianyue Wu @ 2026-08-31 15:30 ` Yosry Ahmed 0 siblings, 0 replies; 21+ messages in thread From: Yosry Ahmed @ 2026-08-31 15:30 UTC (permalink / raw) To: Jianyue Wu Cc: Johannes Weiner, Nhat Pham, Chengming Zhou, Andrew Morton, Chris Li, linux-mm, linux-kernel On Sun, Aug 30, 2026 at 4:48 AM Jianyue Wu <wujianyue000@gmail.com> wrote: > > struct zswap_entry is one allocation per stored page, so its size is > pure overhead. It currently embeds an 8-byte pool pointer, even though > the live pools now sit in a small fixed array indexed by a u8 slot > number. > > Replace the per-entry pool pointer with that u8 slot index and resolve > it through the fixed pool array. A live entry holds a reference to its > pool, so the slot cannot be reused under it. The lookup therefore needs > no RCU read-side section or zswap_pools_lock. > > The u8 fits in the padding after the bool referenced field, shrinking > the entry from 56 to 48 bytes on x86_64. This raises objs_per_slab from > 73 to 85 and saves about 2MiB of metadata per 1GiB of data held in > zswap. > > Suggested-by: Chris Li <chrisl@kernel.org> > Signed-off-by: Jianyue Wu <wujianyue000@gmail.com> Acked-by: Yosry Ahmed <yosry@kernel.org> ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v5 0/3] mm/zswap: shrink zswap_entry via a pool id 2026-08-30 11:47 [RFC PATCH v4 0/3] mm/zswap: shrink zswap_entry via a fixed pool index Jianyue Wu ` (2 preceding siblings ...) 2026-08-30 11:47 ` [RFC PATCH v4 3/3] mm/zswap: reference the pool by index to shrink struct zswap_entry Jianyue Wu @ 2026-09-04 13:24 ` Jianyue Wu 2026-09-04 13:24 ` [PATCH v5 1/3] mm/zswap: release retired pools via queue_rcu_work() instead of synchronize_rcu() Jianyue Wu ` (2 more replies) 3 siblings, 3 replies; 21+ messages in thread From: Jianyue Wu @ 2026-09-04 13:24 UTC (permalink / raw) To: Johannes Weiner, Yosry Ahmed, Nhat Pham, Chengming Zhou, Andrew Morton Cc: Chris Li, linux-mm, linux-kernel Every stored page has a struct zswap_entry, so its size is pure per-page overhead. On 64-bit it is currently 56 bytes, of which 8 bytes are a pointer to the owning zswap_pool. Only a handful of pools are ever live: a new pool is created only when the compressor is (re)set, and pools are reused across compressor switches. That makes a per-entry pool pointer more expensive than it needs to be, and the RCU list that currently tracks pools is more machinery than this needs once each pool already has a stable id. This series: 1. Releases retired pools with queue_rcu_work() instead of a worker calling synchronize_rcu(), so the release worker no longer blocks on an RCU grace period. 2. Replaces the zswap_pools list with an allocating xarray (XA_FLAGS_ALLOC1 | XA_FLAGS_LOCK_BH) and a separate RCU-protected current-pool pointer, giving each pool a stable small id. Ids start at 1. The reserved id 0 is never allocated, so looking it up resolves to NULL. The table grows as needed up to 255 live pools (u8 pool_idx), not a fixed slot array. 3. Stores that u8 pool id in each zswap_entry instead of the pool pointer. The u8 fits in padding after the bool referenced field, so the entry shrinks from 56 to 48 bytes on 64-bit (~2MiB of metadata saved per 1GiB of data held in zswap). Runtime compressor switching is preserved. Pool ids are bounded to 1..255 because struct zswap_entry stores the id in a u8. The cap counts every id still in the xarray, including a killed pool that still has entries. Ids are reused when a pool is erased. Switching back to a compressor that still has a pool in the xarray resurrects it rather than allocating a new id. If all usable ids are full, creating a pool for another compressor fails and the compressor switch is rejected. On 64-bit, struct zswap_entry is 56 -> 48 bytes, which fits 73 -> 85 objects in a 4K slab. Benchmark (x86_64, compressor=lzo, MADV_PAGEOUT store + fault-in load): - e2e store+load median latency: no measurable regression vs baseline at matched stored_delta Each store, free, and decompress looks up the pool with xa_load() instead of following a pointer. With only a handful of live pools the xarray walk is short. Testing ======= - Boot with DEBUG_ATOMIC_SLEEP + lockdep/PROVE_RCU + KASAN: zswap store/load, shrinker writeback, and compressor switch (retire, then switch back to resurrect) pass This series is based on akpm/mm-unstable as of 2026-09-04 (20cab322c95c). To: Johannes Weiner <hannes@cmpxchg.org> To: Yosry Ahmed <yosry@kernel.org> To: Nhat Pham <nphamcs@gmail.com> To: Chengming Zhou <chengming.zhou@linux.dev> To: Andrew Morton <akpm@linux-foundation.org> Cc: Chris Li <chrisl@kernel.org> Cc: linux-mm@kvack.org Cc: linux-kernel@vger.kernel.org Signed-off-by: Jianyue Wu <wujianyue000@gmail.com> Changes since RFC v4: - Replace the RCU pool list with an allocating xarray (XA_FLAGS_ALLOC1 | XA_FLAGS_LOCK_BH) and a separate RCU-protected current-pool pointer. - Resolve entry->pool_idx with xa_load() under rcu_read_lock(). - Write pool_idx before the entry is stored in the swap tree, so a lookup cannot see a stale pool_idx left over from slab reuse. - Retire pools with queue_rcu_work() on system_percpu_wq. - Drop the RFC tag. Link: https://lore.kernel.org/all/20260830114731.8322-1-wujianyue000@gmail.com/ Link: https://lore.kernel.org/all/20260815-shrink_zswap_entry_0815_v2-v3-3-0171bd86a667@gmail.com/ Link: https://lore.kernel.org/all/20260731-shrink_zswap_entry_v2-0-0-v2-0-e72083aa8734@gmail.com/ Link: https://lore.kernel.org/all/20260726-shrink_zswap_entry_v1-0-0-v1-1-30957e4d0cb6@gmail.com/ --- Jianyue Wu (3): mm/zswap: release retired pools via queue_rcu_work() instead of synchronize_rcu() mm/zswap: replace the zswap_pools list with an allocating xarray mm/zswap: reference the pool by id to shrink struct zswap_entry mm/zswap.c | 144 ++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 97 insertions(+), 47 deletions(-) base-commit: 20cab322c95cea0327215bb81a05df69336032dd -- 2.43.0 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v5 1/3] mm/zswap: release retired pools via queue_rcu_work() instead of synchronize_rcu() 2026-09-04 13:24 ` [PATCH v5 0/3] mm/zswap: shrink zswap_entry via a pool id Jianyue Wu @ 2026-09-04 13:24 ` Jianyue Wu 2026-09-04 13:24 ` [PATCH v5 2/3] mm/zswap: replace the zswap_pools list with an allocating xarray Jianyue Wu 2026-09-04 13:24 ` [PATCH v5 3/3] mm/zswap: reference the pool by id to shrink struct zswap_entry Jianyue Wu 2 siblings, 0 replies; 21+ messages in thread From: Jianyue Wu @ 2026-09-04 13:24 UTC (permalink / raw) To: Johannes Weiner, Yosry Ahmed, Nhat Pham, Chengming Zhou, Andrew Morton Cc: Chris Li, linux-mm, linux-kernel When a pool's last reference is dropped, __zswap_pool_empty() removes it from the pool list and schedules __zswap_pool_release(), which calls synchronize_rcu() to wait for readers before tearing the pool down. synchronize_rcu() is a synchronous, potentially long wait. Replace it with queue_rcu_work(): __zswap_pool_empty() hands the pool to queue_rcu_work(), which waits for a grace period asynchronously and then runs __zswap_pool_release() from a worker for the sleepable teardown (__zswap_pool_empty() can run in atomic context and must not block). The grace-period guarantee is unchanged; the retirement path just no longer blocks on it. Suggested-by: Yosry Ahmed <yosry@kernel.org> Signed-off-by: Jianyue Wu <wujianyue000@gmail.com> Acked-by: Yosry Ahmed <yosry@kernel.org> --- mm/zswap.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/mm/zswap.c b/mm/zswap.c index f3ae3c81e48e..e456e5080531 100644 --- a/mm/zswap.c +++ b/mm/zswap.c @@ -155,7 +155,7 @@ struct zswap_pool { struct crypto_acomp_ctx __percpu *acomp_ctx; struct percpu_ref ref; struct list_head list; - struct work_struct release_work; + struct rcu_work release_rwork; struct hlist_node node; char tfm_name[CRYPTO_MAX_ALG_NAME]; }; @@ -379,10 +379,8 @@ static void zswap_pool_destroy(struct zswap_pool *pool) static void __zswap_pool_release(struct work_struct *work) { - struct zswap_pool *pool = container_of(work, typeof(*pool), - release_work); - - synchronize_rcu(); + struct zswap_pool *pool = container_of(to_rcu_work(work), + typeof(*pool), release_rwork); /* nobody should have been able to get a ref... */ WARN_ON(!percpu_ref_is_zero(&pool->ref)); @@ -406,8 +404,8 @@ static void __zswap_pool_empty(struct percpu_ref *ref) list_del_rcu(&pool->list); - INIT_WORK(&pool->release_work, __zswap_pool_release); - schedule_work(&pool->release_work); + INIT_RCU_WORK(&pool->release_rwork, __zswap_pool_release); + queue_rcu_work(system_percpu_wq, &pool->release_rwork); spin_unlock_bh(&zswap_pools_lock); } -- 2.43.0 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v5 2/3] mm/zswap: replace the zswap_pools list with an allocating xarray 2026-09-04 13:24 ` [PATCH v5 0/3] mm/zswap: shrink zswap_entry via a pool id Jianyue Wu 2026-09-04 13:24 ` [PATCH v5 1/3] mm/zswap: release retired pools via queue_rcu_work() instead of synchronize_rcu() Jianyue Wu @ 2026-09-04 13:24 ` Jianyue Wu 2026-09-04 16:04 ` Yosry Ahmed 2026-09-04 13:24 ` [PATCH v5 3/3] mm/zswap: reference the pool by id to shrink struct zswap_entry Jianyue Wu 2 siblings, 1 reply; 21+ messages in thread From: Jianyue Wu @ 2026-09-04 13:24 UTC (permalink / raw) To: Johannes Weiner, Yosry Ahmed, Nhat Pham, Chengming Zhou, Andrew Morton Cc: Chris Li, linux-mm, linux-kernel Originally zswap keeps its pools on an RCU list whose head also serves as the current pool. Convert the pool table to an allocating xarray keyed by a small integer id, and track the current pool with a separate RCU-protected pointer. The xarray gives each pool a stable id for a later zswap_entry shrink. XA_FLAGS_ALLOC1 starts ids at 1, so id 0 remains reserved. The id range is bounded by ZSWAP_MAX_POOL_ID because the later entry field is a u8. Keep compressor switching close to the previous flow: look up an existing pool under xa_lock, resurrect it outside the lock if reused, or create a new one. zswap_pool_create() allocates the pool's id and publishes it into the xarray as its final step, so the create call is itself atomic: it either fully builds the pool and publishes it, or unwinds completely on failure. Publishing makes the pool live, so a caller that later fails (e.g. param_set_charp()) must still kill the pool to erase it from the xarray. Runtime compressor parameter updates are serialized by the module parameter lock, so no speculative loser path is needed. A retiring pool is erased from the xarray in __zswap_pool_empty() and freed via queue_rcu_work(), preserving the old RCU teardown ordering. Suggested-by: Nhat Pham <nphamcs@gmail.com> Suggested-by: Yosry Ahmed <yosry@kernel.org> Suggested-by: Johannes Weiner <hannes@cmpxchg.org> Signed-off-by: Jianyue Wu <wujianyue000@gmail.com> --- mm/zswap.c | 87 +++++++++++++++++++++++++++++++++--------------------- 1 file changed, 54 insertions(+), 33 deletions(-) diff --git a/mm/zswap.c b/mm/zswap.c index e456e5080531..74876acfa9dc 100644 --- a/mm/zswap.c +++ b/mm/zswap.c @@ -34,6 +34,7 @@ #include <linux/writeback.h> #include <linux/pagemap.h> #include <linux/workqueue.h> +#include <linux/xarray.h> #include <linux/list_lru.h> #include <linux/zsmalloc.h> @@ -154,12 +155,24 @@ struct zswap_pool { struct zs_pool *zs_pool; struct crypto_acomp_ctx __percpu *acomp_ctx; struct percpu_ref ref; - struct list_head list; struct rcu_work release_rwork; struct hlist_node node; + u8 idx; char tfm_name[CRYPTO_MAX_ALG_NAME]; }; +/* + * Live pools keyed by id (1..ZSWAP_MAX_POOL_ID). XA_FLAGS_ALLOC1 keeps + * the reserved id 0 unallocated, so looking it up never aliases a live + * pool. XA_FLAGS_LOCK_BH makes the xa_lock softirq-safe: it is taken + * from __zswap_pool_empty(), which runs from a percpu_ref release + * callback in softirq context. + */ +#define ZSWAP_FIRST_POOL_ID 1 +#define ZSWAP_MAX_POOL_ID U8_MAX +static DEFINE_XARRAY_FLAGS(zswap_pools, XA_FLAGS_ALLOC1 | XA_FLAGS_LOCK_BH); +static struct zswap_pool __rcu *zswap_current_pool; + /* Global LRU lists shared by all zswap pools. */ static struct list_lru zswap_list_lru; @@ -200,10 +213,6 @@ struct zswap_entry { static struct xarray *zswap_trees[MAX_SWAPFILES]; static unsigned int nr_zswap_trees[MAX_SWAPFILES]; -/* RCU-protected iteration */ -static LIST_HEAD(zswap_pools); -/* protects zswap_pools list modification */ -static DEFINE_SPINLOCK(zswap_pools_lock); /* pool counter to provide unique names to zsmalloc */ static atomic_t zswap_pools_count = ATOMIC_INIT(0); @@ -275,6 +284,7 @@ static struct zswap_pool *zswap_pool_create(char *compressor) struct zswap_pool *pool; char name[38]; /* 'zswap' + 32 char (max) num + \0 */ int ret, cpu; + u32 id; if (!zswap_has_pool && !strcmp(compressor, ZSWAP_PARAM_UNSET)) return NULL; @@ -320,12 +330,24 @@ static struct zswap_pool *zswap_pool_create(char *compressor) PERCPU_REF_ALLOW_REINIT, GFP_KERNEL); if (ret) goto ref_fail; - INIT_LIST_HEAD(&pool->list); + + ret = xa_alloc_bh(&zswap_pools, &id, pool, + XA_LIMIT(ZSWAP_FIRST_POOL_ID, ZSWAP_MAX_POOL_ID), + GFP_KERNEL); + if (ret) { + if (ret == -EBUSY) + pr_err("cannot allocate pool id (max %d live pools)\n", + ZSWAP_MAX_POOL_ID - ZSWAP_FIRST_POOL_ID + 1); + goto xa_fail; + } + pool->idx = id; zswap_pool_debug("created", pool); return pool; +xa_fail: + percpu_ref_exit(&pool->ref); ref_fail: cpuhp_state_remove_instance(CPUHP_MM_ZSWP_POOL_PREPARE, &pool->node); @@ -386,7 +408,7 @@ static void __zswap_pool_release(struct work_struct *work) WARN_ON(!percpu_ref_is_zero(&pool->ref)); percpu_ref_exit(&pool->ref); - /* pool is now off zswap_pools list and has no references. */ + /* The pool is no longer in zswap_pools and has no references. */ zswap_pool_destroy(pool); } @@ -398,16 +420,16 @@ static void __zswap_pool_empty(struct percpu_ref *ref) pool = container_of(ref, typeof(*pool), ref); - spin_lock_bh(&zswap_pools_lock); + xa_lock_bh(&zswap_pools); WARN_ON(pool == zswap_pool_current()); - list_del_rcu(&pool->list); + __xa_erase(&zswap_pools, pool->idx); INIT_RCU_WORK(&pool->release_rwork, __zswap_pool_release); queue_rcu_work(system_percpu_wq, &pool->release_rwork); - spin_unlock_bh(&zswap_pools_lock); + xa_unlock_bh(&zswap_pools); } static int __must_check zswap_pool_tryget(struct zswap_pool *pool) @@ -433,7 +455,8 @@ static struct zswap_pool *__zswap_pool_current(void) { struct zswap_pool *pool; - pool = list_first_or_null_rcu(&zswap_pools, typeof(*pool), list); + pool = rcu_dereference_check(zswap_current_pool, + lockdep_is_held(&zswap_pools.xa_lock)); WARN_ONCE(!pool && zswap_has_pool, "%s: no page storage pool!\n", __func__); @@ -442,7 +465,7 @@ static struct zswap_pool *__zswap_pool_current(void) static struct zswap_pool *zswap_pool_current(void) { - assert_spin_locked(&zswap_pools_lock); + lockdep_assert_held(&zswap_pools.xa_lock); return __zswap_pool_current(); } @@ -462,14 +485,15 @@ static struct zswap_pool *zswap_pool_current_get(void) return pool; } -/* type and compressor must be null-terminated */ +/* compressor must be null-terminated */ static struct zswap_pool *zswap_pool_find_get(char *compressor) { struct zswap_pool *pool; + unsigned long id; - assert_spin_locked(&zswap_pools_lock); + lockdep_assert_held(&zswap_pools.xa_lock); - list_for_each_entry_rcu(pool, &zswap_pools, list) { + xa_for_each(&zswap_pools, id, pool) { if (strcmp(pool->tfm_name, compressor)) continue; /* if we can't get it, it's about to be destroyed */ @@ -495,9 +519,15 @@ unsigned long zswap_total_pages(void) { struct zswap_pool *pool; unsigned long total = 0; + unsigned long id; + /* + * rcu_read_lock() is required here, not just for xa_for_each(): it also + * keeps each pool alive while it is dereferenced, since a concurrently + * retired pool is freed via queue_rcu_work() after a grace period. + */ rcu_read_lock(); - list_for_each_entry_rcu(pool, &zswap_pools, list) + xa_for_each(&zswap_pools, id, pool) total += zs_get_total_pages(pool->zs_pool); rcu_read_unlock(); @@ -554,20 +584,17 @@ static int zswap_compressor_param_set(const char *val, const struct kernel_param return -ENOENT; } - spin_lock_bh(&zswap_pools_lock); - + xa_lock_bh(&zswap_pools); pool = zswap_pool_find_get(s); if (pool) { zswap_pool_debug("using existing", pool); WARN_ON(pool == zswap_pool_current()); - list_del_rcu(&pool->list); } + xa_unlock_bh(&zswap_pools); - spin_unlock_bh(&zswap_pools_lock); - - if (!pool) + if (!pool) { pool = zswap_pool_create(s); - else { + } else { /* * Restore the initial ref dropped by percpu_ref_kill() * when the pool was decommissioned and switch it again @@ -584,23 +611,17 @@ static int zswap_compressor_param_set(const char *val, const struct kernel_param else ret = -EINVAL; - spin_lock_bh(&zswap_pools_lock); + xa_lock_bh(&zswap_pools); if (!ret) { put_pool = zswap_pool_current(); - list_add_rcu(&pool->list, &zswap_pools); + rcu_assign_pointer(zswap_current_pool, pool); zswap_has_pool = true; } else if (pool) { - /* - * Add the possibly pre-existing pool to the end of the pools - * list; if it's new (and empty) then it'll be removed and - * destroyed by the put after we drop the lock - */ - list_add_tail_rcu(&pool->list, &zswap_pools); put_pool = pool; } - spin_unlock_bh(&zswap_pools_lock); + xa_unlock_bh(&zswap_pools); /* * Drop the ref from either the old current pool, @@ -1788,7 +1809,7 @@ static int zswap_setup(void) pool = __zswap_pool_create_fallback(); if (pool) { pr_info("loaded using pool %s\n", pool->tfm_name); - list_add(&pool->list, &zswap_pools); + rcu_assign_pointer(zswap_current_pool, pool); zswap_has_pool = true; static_branch_enable(&zswap_ever_enabled); } else { -- 2.43.0 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v5 2/3] mm/zswap: replace the zswap_pools list with an allocating xarray 2026-09-04 13:24 ` [PATCH v5 2/3] mm/zswap: replace the zswap_pools list with an allocating xarray Jianyue Wu @ 2026-09-04 16:04 ` Yosry Ahmed 2026-09-05 13:15 ` Jianyue Wu 0 siblings, 1 reply; 21+ messages in thread From: Yosry Ahmed @ 2026-09-04 16:04 UTC (permalink / raw) To: Jianyue Wu Cc: Johannes Weiner, Nhat Pham, Chengming Zhou, Andrew Morton, Chris Li, linux-mm, linux-kernel On Fri, Sep 4, 2026 at 6:25 AM Jianyue Wu <wujianyue000@gmail.com> wrote: > > Originally zswap keeps its pools on an RCU list whose head also serves > as the current pool. Convert the pool table to an allocating xarray > keyed by a small integer id, and track the current pool with a separate > RCU-protected pointer. > > The xarray gives each pool a stable id for a later zswap_entry shrink. > XA_FLAGS_ALLOC1 starts ids at 1, so id 0 remains reserved. The id range > is bounded by ZSWAP_MAX_POOL_ID because the later entry field is a u8. > > Keep compressor switching close to the previous flow: look up an > existing pool under xa_lock, resurrect it outside the lock if reused, or > create a new one. zswap_pool_create() allocates the pool's id and > publishes it into the xarray as its final step, so the create call is > itself atomic: it either fully builds the pool and publishes it, or > unwinds completely on failure. Publishing makes the pool live, so a > caller that later fails (e.g. param_set_charp()) must still kill the > pool to erase it from the xarray. Runtime compressor parameter updates > are serialized by the module parameter lock, so no speculative loser > path is needed. > > A retiring pool is erased from the xarray in __zswap_pool_empty() and > freed via queue_rcu_work(), preserving the old RCU teardown ordering. > > Suggested-by: Nhat Pham <nphamcs@gmail.com> > Suggested-by: Yosry Ahmed <yosry@kernel.org> > Suggested-by: Johannes Weiner <hannes@cmpxchg.org> > Signed-off-by: Jianyue Wu <wujianyue000@gmail.com> > --- > mm/zswap.c | 87 +++++++++++++++++++++++++++++++++--------------------- > 1 file changed, 54 insertions(+), 33 deletions(-) > > diff --git a/mm/zswap.c b/mm/zswap.c > index e456e5080531..74876acfa9dc 100644 > --- a/mm/zswap.c > +++ b/mm/zswap.c > @@ -34,6 +34,7 @@ > #include <linux/writeback.h> > #include <linux/pagemap.h> > #include <linux/workqueue.h> > +#include <linux/xarray.h> > #include <linux/list_lru.h> > #include <linux/zsmalloc.h> > > @@ -154,12 +155,24 @@ struct zswap_pool { > struct zs_pool *zs_pool; > struct crypto_acomp_ctx __percpu *acomp_ctx; > struct percpu_ref ref; > - struct list_head list; > struct rcu_work release_rwork; > struct hlist_node node; > + u8 idx; > char tfm_name[CRYPTO_MAX_ALG_NAME]; > }; > > +/* > + * Live pools keyed by id (1..ZSWAP_MAX_POOL_ID). XA_FLAGS_ALLOC1 keeps > + * the reserved id 0 unallocated, so looking it up never aliases a live > + * pool. XA_FLAGS_LOCK_BH makes the xa_lock softirq-safe: it is taken > + * from __zswap_pool_empty(), which runs from a percpu_ref release > + * callback in softirq context. > + */ > +#define ZSWAP_FIRST_POOL_ID 1 > +#define ZSWAP_MAX_POOL_ID U8_MAX > +static DEFINE_XARRAY_FLAGS(zswap_pools, XA_FLAGS_ALLOC1 | XA_FLAGS_LOCK_BH); > +static struct zswap_pool __rcu *zswap_current_pool; > + > /* Global LRU lists shared by all zswap pools. */ > static struct list_lru zswap_list_lru; > > @@ -200,10 +213,6 @@ struct zswap_entry { > static struct xarray *zswap_trees[MAX_SWAPFILES]; > static unsigned int nr_zswap_trees[MAX_SWAPFILES]; > > -/* RCU-protected iteration */ > -static LIST_HEAD(zswap_pools); > -/* protects zswap_pools list modification */ > -static DEFINE_SPINLOCK(zswap_pools_lock); > /* pool counter to provide unique names to zsmalloc */ > static atomic_t zswap_pools_count = ATOMIC_INIT(0); > > @@ -275,6 +284,7 @@ static struct zswap_pool *zswap_pool_create(char *compressor) > struct zswap_pool *pool; > char name[38]; /* 'zswap' + 32 char (max) num + \0 */ > int ret, cpu; > + u32 id; > > if (!zswap_has_pool && !strcmp(compressor, ZSWAP_PARAM_UNSET)) > return NULL; > @@ -320,12 +330,24 @@ static struct zswap_pool *zswap_pool_create(char *compressor) > PERCPU_REF_ALLOW_REINIT, GFP_KERNEL); > if (ret) > goto ref_fail; > - INIT_LIST_HEAD(&pool->list); > + > + ret = xa_alloc_bh(&zswap_pools, &id, pool, > + XA_LIMIT(ZSWAP_FIRST_POOL_ID, ZSWAP_MAX_POOL_ID), > + GFP_KERNEL); > + if (ret) { > + if (ret == -EBUSY) > + pr_err("cannot allocate pool id (max %d live pools)\n", > + ZSWAP_MAX_POOL_ID - ZSWAP_FIRST_POOL_ID + 1); > + goto xa_fail; > + } > + pool->idx = id; > > zswap_pool_debug("created", pool); > > return pool; > > +xa_fail: > + percpu_ref_exit(&pool->ref); > ref_fail: > cpuhp_state_remove_instance(CPUHP_MM_ZSWP_POOL_PREPARE, &pool->node); > > @@ -386,7 +408,7 @@ static void __zswap_pool_release(struct work_struct *work) > WARN_ON(!percpu_ref_is_zero(&pool->ref)); > percpu_ref_exit(&pool->ref); > > - /* pool is now off zswap_pools list and has no references. */ > + /* The pool is no longer in zswap_pools and has no references. */ > zswap_pool_destroy(pool); > } > > @@ -398,16 +420,16 @@ static void __zswap_pool_empty(struct percpu_ref *ref) > > pool = container_of(ref, typeof(*pool), ref); > > - spin_lock_bh(&zswap_pools_lock); > + xa_lock_bh(&zswap_pools); > > WARN_ON(pool == zswap_pool_current()); > > - list_del_rcu(&pool->list); > + __xa_erase(&zswap_pools, pool->idx); > > INIT_RCU_WORK(&pool->release_rwork, __zswap_pool_release); > queue_rcu_work(system_percpu_wq, &pool->release_rwork); > > - spin_unlock_bh(&zswap_pools_lock); > + xa_unlock_bh(&zswap_pools); Do we need to call queue_rcu_work() under the lock? I assume not. Can we just call xa_erase_bh()? > } > > static int __must_check zswap_pool_tryget(struct zswap_pool *pool) > @@ -433,7 +455,8 @@ static struct zswap_pool *__zswap_pool_current(void) > { > struct zswap_pool *pool; > > - pool = list_first_or_null_rcu(&zswap_pools, typeof(*pool), list); > + pool = rcu_dereference_check(zswap_current_pool, > + lockdep_is_held(&zswap_pools.xa_lock)); > WARN_ONCE(!pool && zswap_has_pool, > "%s: no page storage pool!\n", __func__); > > @@ -442,7 +465,7 @@ static struct zswap_pool *__zswap_pool_current(void) > > static struct zswap_pool *zswap_pool_current(void) > { > - assert_spin_locked(&zswap_pools_lock); > + lockdep_assert_held(&zswap_pools.xa_lock); > > return __zswap_pool_current(); > } > @@ -462,14 +485,15 @@ static struct zswap_pool *zswap_pool_current_get(void) > return pool; > } > > -/* type and compressor must be null-terminated */ > +/* compressor must be null-terminated */ > static struct zswap_pool *zswap_pool_find_get(char *compressor) > { > struct zswap_pool *pool; > + unsigned long id; > > - assert_spin_locked(&zswap_pools_lock); > + lockdep_assert_held(&zswap_pools.xa_lock); > > - list_for_each_entry_rcu(pool, &zswap_pools, list) { > + xa_for_each(&zswap_pools, id, pool) { > if (strcmp(pool->tfm_name, compressor)) > continue; > /* if we can't get it, it's about to be destroyed */ > @@ -495,9 +519,15 @@ unsigned long zswap_total_pages(void) > { > struct zswap_pool *pool; > unsigned long total = 0; > + unsigned long id; > > + /* > + * rcu_read_lock() is required here, not just for xa_for_each(): it also > + * keeps each pool alive while it is dereferenced, since a concurrently > + * retired pool is freed via queue_rcu_work() after a grace period. > + */ Doesn't xa_for_each() already handle RCU locking? > rcu_read_lock(); > - list_for_each_entry_rcu(pool, &zswap_pools, list) > + xa_for_each(&zswap_pools, id, pool) > total += zs_get_total_pages(pool->zs_pool); > rcu_read_unlock(); > > @@ -554,20 +584,17 @@ static int zswap_compressor_param_set(const char *val, const struct kernel_param > return -ENOENT; > } > > - spin_lock_bh(&zswap_pools_lock); > - > + xa_lock_bh(&zswap_pools); > pool = zswap_pool_find_get(s); This is the only caller of zswap_pool_find_get(), and since we remove list_del_rcu() below we have no reason for holding the lock here other than zswap_pool_find_get(). So let's move the locking inside? I am also not really sure if we actually need to hold the lock here anymore to begin with. > if (pool) { > zswap_pool_debug("using existing", pool); > WARN_ON(pool == zswap_pool_current()); > - list_del_rcu(&pool->list); > } > + xa_unlock_bh(&zswap_pools); > > - spin_unlock_bh(&zswap_pools_lock); > - > - if (!pool) > + if (!pool) { > pool = zswap_pool_create(s); > - else { > + } else { > /* > * Restore the initial ref dropped by percpu_ref_kill() > * when the pool was decommissioned and switch it again > @@ -584,23 +611,17 @@ static int zswap_compressor_param_set(const char *val, const struct kernel_param > else > ret = -EINVAL; > > - spin_lock_bh(&zswap_pools_lock); > + xa_lock_bh(&zswap_pools); Do we still need to hold the lock here? > > if (!ret) { > put_pool = zswap_pool_current(); > - list_add_rcu(&pool->list, &zswap_pools); > + rcu_assign_pointer(zswap_current_pool, pool); > zswap_has_pool = true; > } else if (pool) { > - /* > - * Add the possibly pre-existing pool to the end of the pools > - * list; if it's new (and empty) then it'll be removed and > - * destroyed by the put after we drop the lock > - */ > - list_add_tail_rcu(&pool->list, &zswap_pools); > put_pool = pool; > } > > - spin_unlock_bh(&zswap_pools_lock); > + xa_unlock_bh(&zswap_pools); > > /* > * Drop the ref from either the old current pool, > @@ -1788,7 +1809,7 @@ static int zswap_setup(void) > pool = __zswap_pool_create_fallback(); > if (pool) { > pr_info("loaded using pool %s\n", pool->tfm_name); > - list_add(&pool->list, &zswap_pools); > + rcu_assign_pointer(zswap_current_pool, pool); > zswap_has_pool = true; > static_branch_enable(&zswap_ever_enabled); > } else { > -- > 2.43.0 > ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v5 2/3] mm/zswap: replace the zswap_pools list with an allocating xarray 2026-09-04 16:04 ` Yosry Ahmed @ 2026-09-05 13:15 ` Jianyue Wu 0 siblings, 0 replies; 21+ messages in thread From: Jianyue Wu @ 2026-09-05 13:15 UTC (permalink / raw) To: Yosry Ahmed Cc: Johannes Weiner, Nhat Pham, Chengming Zhou, Andrew Morton, Chris Li, linux-mm, linux-kernel On Sat, Sep 5, 2026 at 12:04 AM Yosry Ahmed <yosry@kernel.org> wrote: > > > - list_del_rcu(&pool->list); > > + __xa_erase(&zswap_pools, pool->idx); > > > > INIT_RCU_WORK(&pool->release_rwork, __zswap_pool_release); > > queue_rcu_work(system_percpu_wq, &pool->release_rwork); > > > > - spin_unlock_bh(&zswap_pools_lock); > > + xa_unlock_bh(&zswap_pools); > > Do we need to call queue_rcu_work() under the lock? I assume not. Can > we just call xa_erase_bh()? Right, no need. queue_rcu_work() only touches the retiring pool's own release_rwork. xa_erase_bh(&zswap_pools, pool->idx) is enough. > > Doesn't xa_for_each() already handle RCU locking? > > > rcu_read_lock(); > > - list_for_each_entry_rcu(pool, &zswap_pools, list) > > + xa_for_each(&zswap_pools, id, pool) > > total += zs_get_total_pages(pool->zs_pool); > > rcu_read_unlock(); xa_for_each() only takes RCU around each internal lookup, not across the loop body, so the explicit rcu_read_lock() is still needed to keep a concurrently retired pool alive while we dereference pool->zs_pool. > > @@ -554,20 +584,17 @@ static int zswap_compressor_param_set(const char *val, const struct kernel_param > > return -ENOENT; > > } > > > > - spin_lock_bh(&zswap_pools_lock); > > - > > + xa_lock_bh(&zswap_pools); > > pool = zswap_pool_find_get(s); > > This is the only caller of zswap_pool_find_get(), and since we remove > list_del_rcu() below we have no reason for holding the lock here other > than zswap_pool_find_get(). So let's move the locking inside? I am > also not really sure if we actually need to hold the lock here anymore > to begin with. Agreed, and it turns out we don't need xa_lock here at all. > > @@ -584,23 +611,17 @@ static int zswap_compressor_param_set(const char *val, const struct kernel_param > > else > > ret = -EINVAL; > > > > - spin_lock_bh(&zswap_pools_lock); > > + xa_lock_bh(&zswap_pools); > > Do we still need to hold the lock here? > Agreed, no need lock here. Compressor switches are serialized by the module parameter lock, so this is the only writer of zswap_current_pool. Thanks for the review! Best regards, Jianyue ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v5 3/3] mm/zswap: reference the pool by id to shrink struct zswap_entry 2026-09-04 13:24 ` [PATCH v5 0/3] mm/zswap: shrink zswap_entry via a pool id Jianyue Wu 2026-09-04 13:24 ` [PATCH v5 1/3] mm/zswap: release retired pools via queue_rcu_work() instead of synchronize_rcu() Jianyue Wu 2026-09-04 13:24 ` [PATCH v5 2/3] mm/zswap: replace the zswap_pools list with an allocating xarray Jianyue Wu @ 2026-09-04 13:24 ` Jianyue Wu 2026-09-04 15:38 ` Yosry Ahmed 2 siblings, 1 reply; 21+ messages in thread From: Jianyue Wu @ 2026-09-04 13:24 UTC (permalink / raw) To: Johannes Weiner, Yosry Ahmed, Nhat Pham, Chengming Zhou, Andrew Morton Cc: Chris Li, linux-mm, linux-kernel struct zswap_entry is one allocation per stored page, so its size is pure overhead. It currently embeds an 8-byte pool pointer, even though the live pools now sit in an allocating xarray keyed by a small integer id that fits in a u8. Replace the per-entry pool pointer with that u8 id and resolve it through the xarray with xa_load(). A live entry holds a reference to its pool, so the id cannot be reused under it; xa_load() is lockless and only needs an rcu_read_lock() section, no zswap_pools_lock. A live entry never uses the reserved id 0, so looking up that id resolves to NULL and trips a WARN rather than aliasing a live pool. The u8 fits in the padding after the bool referenced field, shrinking the entry from 56 to 48 bytes on 64-bit. This raises objs_per_slab from 73 to 85 and saves about 2MiB of metadata per 1GiB of data held in zswap. Suggested-by: Chris Li <chrisl@kernel.org> Signed-off-by: Jianyue Wu <wujianyue000@gmail.com> --- mm/zswap.c | 45 ++++++++++++++++++++++++++++++++++++++------- 1 file changed, 38 insertions(+), 7 deletions(-) diff --git a/mm/zswap.c b/mm/zswap.c index 74876acfa9dc..31cf0ef43d23 100644 --- a/mm/zswap.c +++ b/mm/zswap.c @@ -195,7 +195,7 @@ static struct shrinker *zswap_shrinker; * writeback logic. The entry is only reclaimed by the writeback * logic if referenced is unset. See comments in the shrinker * section for context. - * pool - the zswap_pool the entry's data is in + * pool_idx - id of the zswap_pool that the entry's data is in. * handle - zsmalloc allocation handle that stores the compressed page data * objcg - the obj_cgroup that the compressed memory is charged to * lru - handle to the pool's lru used to evict pages. @@ -204,12 +204,27 @@ struct zswap_entry { swp_entry_t swpentry; unsigned int length; bool referenced; - struct zswap_pool *pool; + u8 pool_idx; unsigned long handle; struct obj_cgroup *objcg; struct list_head lru; }; +/* + * The pool stays alive after this returns because a stored entry holds a + * reference to its pool (taken in zswap_store_page()). + */ +static struct zswap_pool *zswap_entry_pool(struct zswap_entry *entry) +{ + struct zswap_pool *pool; + + rcu_read_lock(); + pool = xa_load(&zswap_pools, entry->pool_idx); + rcu_read_unlock(); + + return pool; +} + static struct xarray *zswap_trees[MAX_SWAPFILES]; static unsigned int nr_zswap_trees[MAX_SWAPFILES]; @@ -770,9 +785,13 @@ static void zswap_entry_cache_free(struct zswap_entry *entry) */ static void zswap_entry_free(struct zswap_entry *entry) { + struct zswap_pool *pool = zswap_entry_pool(entry); + zswap_lru_del(entry); - zs_free(entry->pool->zs_pool, entry->handle); - zswap_pool_put(entry->pool); + if (!WARN_ON_ONCE(!pool)) { + zs_free(pool->zs_pool, entry->handle); + zswap_pool_put(pool); + } if (entry->objcg) { obj_cgroup_uncharge_zswap(entry->objcg, entry->length); obj_cgroup_put(entry->objcg); @@ -929,12 +948,15 @@ static bool zswap_compress(struct page *page, struct zswap_entry *entry, static bool zswap_decompress(struct zswap_entry *entry, struct folio *folio) { - struct zswap_pool *pool = entry->pool; + struct zswap_pool *pool = zswap_entry_pool(entry); struct scatterlist input[2]; /* zsmalloc returns an SG list 1-2 entries */ struct scatterlist output; struct crypto_acomp_ctx *acomp_ctx; int ret = 0, dlen; + if (WARN_ON_ONCE(!pool)) + return false; + acomp_ctx = raw_cpu_ptr(pool->acomp_ctx); mutex_lock(&acomp_ctx->mutex); zs_obj_read_sg_begin(pool->zs_pool, entry->handle, input, entry->length); @@ -970,7 +992,7 @@ static bool zswap_decompress(struct zswap_entry *entry, struct folio *folio) pr_alert_ratelimited("Decompression error from zswap (%d:%lu %s %u->%d)\n", swp_type(entry->swpentry), swp_offset(entry->swpentry), - entry->pool->tfm_name, + pool->tfm_name, entry->length, dlen); return false; } @@ -1428,6 +1450,16 @@ static bool zswap_store_page(struct page *page, if (!zswap_compress(page, entry, pool)) goto compress_failed; + /* + * Set pool_idx before publishing the entry: compression has + * succeeded and the pool is already pinned by this store, so the id is + * final. Doing it here (rather than after xa_store()) means the entry + * is never briefly visible with a stale pool_idx left over from slab + * reuse, which zswap_entry_pool() would otherwise resolve to an + * unrelated live pool. + */ + entry->pool_idx = pool->idx; + old = xa_store(swap_zswap_tree(page_swpentry), swp_offset(page_swpentry), entry, GFP_KERNEL); @@ -1473,7 +1505,6 @@ static bool zswap_store_page(struct page *page, * The publishing order matters to prevent writeback from seeing * an incoherent entry. */ - entry->pool = pool; entry->swpentry = page_swpentry; entry->objcg = objcg; entry->referenced = true; -- 2.43.0 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v5 3/3] mm/zswap: reference the pool by id to shrink struct zswap_entry 2026-09-04 13:24 ` [PATCH v5 3/3] mm/zswap: reference the pool by id to shrink struct zswap_entry Jianyue Wu @ 2026-09-04 15:38 ` Yosry Ahmed 2026-09-05 13:20 ` Jianyue Wu 0 siblings, 1 reply; 21+ messages in thread From: Yosry Ahmed @ 2026-09-04 15:38 UTC (permalink / raw) To: Jianyue Wu Cc: Johannes Weiner, Nhat Pham, Chengming Zhou, Andrew Morton, Chris Li, linux-mm, linux-kernel On Fri, Sep 4, 2026 at 6:25 AM Jianyue Wu <wujianyue000@gmail.com> wrote: > > struct zswap_entry is one allocation per stored page, so its size is > pure overhead. It currently embeds an 8-byte pool pointer, even though > the live pools now sit in an allocating xarray keyed by a small integer > id that fits in a u8. > > Replace the per-entry pool pointer with that u8 id and resolve it > through the xarray with xa_load(). A live entry holds a reference to its > pool, so the id cannot be reused under it; xa_load() is lockless and > only needs an rcu_read_lock() section, no zswap_pools_lock. A live entry > never uses the reserved id 0, so looking up that id resolves to NULL > and trips a WARN rather than aliasing a live pool. > > The u8 fits in the padding after the bool referenced field, shrinking > the entry from 56 to 48 bytes on 64-bit. This raises objs_per_slab from > 73 to 85 and saves about 2MiB of metadata per 1GiB of data held in > zswap. > > Suggested-by: Chris Li <chrisl@kernel.org> > Signed-off-by: Jianyue Wu <wujianyue000@gmail.com> > --- > mm/zswap.c | 45 ++++++++++++++++++++++++++++++++++++++------- > 1 file changed, 38 insertions(+), 7 deletions(-) > > diff --git a/mm/zswap.c b/mm/zswap.c > index 74876acfa9dc..31cf0ef43d23 100644 > --- a/mm/zswap.c > +++ b/mm/zswap.c > @@ -195,7 +195,7 @@ static struct shrinker *zswap_shrinker; > * writeback logic. The entry is only reclaimed by the writeback > * logic if referenced is unset. See comments in the shrinker > * section for context. > - * pool - the zswap_pool the entry's data is in > + * pool_idx - id of the zswap_pool that the entry's data is in. > * handle - zsmalloc allocation handle that stores the compressed page data > * objcg - the obj_cgroup that the compressed memory is charged to > * lru - handle to the pool's lru used to evict pages. > @@ -204,12 +204,27 @@ struct zswap_entry { > swp_entry_t swpentry; > unsigned int length; > bool referenced; > - struct zswap_pool *pool; > + u8 pool_idx; > unsigned long handle; > struct obj_cgroup *objcg; > struct list_head lru; > }; > > +/* > + * The pool stays alive after this returns because a stored entry holds a > + * reference to its pool (taken in zswap_store_page()). > + */ > +static struct zswap_pool *zswap_entry_pool(struct zswap_entry *entry) > +{ > + struct zswap_pool *pool; > + > + rcu_read_lock(); > + pool = xa_load(&zswap_pools, entry->pool_idx); > + rcu_read_unlock(); Doesn't xa_load() already call rcu_read_[un]lock()? > + > + return pool; > +} > + > static struct xarray *zswap_trees[MAX_SWAPFILES]; > static unsigned int nr_zswap_trees[MAX_SWAPFILES]; > > @@ -770,9 +785,13 @@ static void zswap_entry_cache_free(struct zswap_entry *entry) > */ > static void zswap_entry_free(struct zswap_entry *entry) > { > + struct zswap_pool *pool = zswap_entry_pool(entry); > + > zswap_lru_del(entry); > - zs_free(entry->pool->zs_pool, entry->handle); > - zswap_pool_put(entry->pool); > + if (!WARN_ON_ONCE(!pool)) { > + zs_free(pool->zs_pool, entry->handle); > + zswap_pool_put(pool); > + } > if (entry->objcg) { > obj_cgroup_uncharge_zswap(entry->objcg, entry->length); > obj_cgroup_put(entry->objcg); > @@ -929,12 +948,15 @@ static bool zswap_compress(struct page *page, struct zswap_entry *entry, > > static bool zswap_decompress(struct zswap_entry *entry, struct folio *folio) > { > - struct zswap_pool *pool = entry->pool; > + struct zswap_pool *pool = zswap_entry_pool(entry); > struct scatterlist input[2]; /* zsmalloc returns an SG list 1-2 entries */ > struct scatterlist output; > struct crypto_acomp_ctx *acomp_ctx; > int ret = 0, dlen; > > + if (WARN_ON_ONCE(!pool)) > + return false; > + > acomp_ctx = raw_cpu_ptr(pool->acomp_ctx); > mutex_lock(&acomp_ctx->mutex); > zs_obj_read_sg_begin(pool->zs_pool, entry->handle, input, entry->length); > @@ -970,7 +992,7 @@ static bool zswap_decompress(struct zswap_entry *entry, struct folio *folio) > pr_alert_ratelimited("Decompression error from zswap (%d:%lu %s %u->%d)\n", > swp_type(entry->swpentry), > swp_offset(entry->swpentry), > - entry->pool->tfm_name, > + pool->tfm_name, > entry->length, dlen); > return false; > } > @@ -1428,6 +1450,16 @@ static bool zswap_store_page(struct page *page, > if (!zswap_compress(page, entry, pool)) > goto compress_failed; > > + /* > + * Set pool_idx before publishing the entry: compression has > + * succeeded and the pool is already pinned by this store, so the id is > + * final. Doing it here (rather than after xa_store()) means the entry > + * is never briefly visible with a stale pool_idx left over from slab > + * reuse, which zswap_entry_pool() would otherwise resolve to an > + * unrelated live pool. > + */ > + entry->pool_idx = pool->idx; > + > old = xa_store(swap_zswap_tree(page_swpentry), > swp_offset(page_swpentry), > entry, GFP_KERNEL); > @@ -1473,7 +1505,6 @@ static bool zswap_store_page(struct page *page, > * The publishing order matters to prevent writeback from seeing > * an incoherent entry. > */ > - entry->pool = pool; > entry->swpentry = page_swpentry; > entry->objcg = objcg; > entry->referenced = true; > -- > 2.43.0 > ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v5 3/3] mm/zswap: reference the pool by id to shrink struct zswap_entry 2026-09-04 15:38 ` Yosry Ahmed @ 2026-09-05 13:20 ` Jianyue Wu 0 siblings, 0 replies; 21+ messages in thread From: Jianyue Wu @ 2026-09-05 13:20 UTC (permalink / raw) To: Yosry Ahmed Cc: Johannes Weiner, Nhat Pham, Chengming Zhou, Andrew Morton, Chris Li, linux-mm, linux-kernel On Fri, Sep 4, 2026 at 11:38 PM Yosry Ahmed <yosry@kernel.org> wrote: > > +/* > > + * The pool stays alive after this returns because a stored entry holds a > > + * reference to its pool (taken in zswap_store_page()). > > + */ > > +static struct zswap_pool *zswap_entry_pool(struct zswap_entry *entry) > > +{ > > + struct zswap_pool *pool; > > + > > + rcu_read_lock(); > > + pool = xa_load(&zswap_pools, entry->pool_idx); > > + rcu_read_unlock(); > > Doesn't xa_load() already call rcu_read_[un]lock()? > Yes, xa_load() does the lookup under RCU internally, so the wrapping rcu_read_lock()/unlock() is redundant, and will remove it. Best regards, Jianyue ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2026-09-05 13:20 UTC | newest] Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-08-30 11:47 [RFC PATCH v4 0/3] mm/zswap: shrink zswap_entry via a fixed pool index Jianyue Wu 2026-08-30 11:47 ` [RFC PATCH v4 1/3] mm/zswap: release retired pools via queue_rcu_work() instead of synchronize_rcu() Jianyue Wu 2026-08-31 15:20 ` Yosry Ahmed 2026-09-01 14:33 ` Jianyue Wu 2026-09-01 15:38 ` Johannes Weiner 2026-09-02 0:53 ` Jianyue Wu 2026-08-30 11:47 ` [RFC PATCH v4 2/3] mm/zswap: replace the zswap_pools list with a fixed pools array Jianyue Wu 2026-08-31 15:28 ` Yosry Ahmed 2026-09-01 16:13 ` Johannes Weiner 2026-09-02 0:50 ` Jianyue Wu 2026-09-03 12:59 ` Jianyue Wu 2026-08-30 11:47 ` [RFC PATCH v4 3/3] mm/zswap: reference the pool by index to shrink struct zswap_entry Jianyue Wu 2026-08-31 15:30 ` Yosry Ahmed 2026-09-04 13:24 ` [PATCH v5 0/3] mm/zswap: shrink zswap_entry via a pool id Jianyue Wu 2026-09-04 13:24 ` [PATCH v5 1/3] mm/zswap: release retired pools via queue_rcu_work() instead of synchronize_rcu() Jianyue Wu 2026-09-04 13:24 ` [PATCH v5 2/3] mm/zswap: replace the zswap_pools list with an allocating xarray Jianyue Wu 2026-09-04 16:04 ` Yosry Ahmed 2026-09-05 13:15 ` Jianyue Wu 2026-09-04 13:24 ` [PATCH v5 3/3] mm/zswap: reference the pool by id to shrink struct zswap_entry Jianyue Wu 2026-09-04 15:38 ` Yosry Ahmed 2026-09-05 13:20 ` Jianyue Wu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®