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 Suggested-by: Yosry Ahmed Suggested-by: Johannes Weiner Signed-off-by: Jianyue Wu --- 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 #include #include +#include #include #include @@ -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