On Thu, Jul 3, 2025 at 9:23 PM Kanchana P Sridhar <kanchana.p.sridhar@xxxxxxxxx> wrote: > > This patch merely moves zswap_cpu_comp_prepare() and > zswap_cpu_comp_dead() to be in the "pool functions" section because > these functions are invoked upon pool creation/deletion. Hmm idk, "compressed storage" section seems fitting for zswap_cpu_comp_prepare() and zswap_cpu_comp_dead(). Is this patch necessary? > > Signed-off-by: Kanchana P Sridhar <kanchana.p.sridhar@xxxxxxxxx> > --- > mm/zswap.c | 188 ++++++++++++++++++++++++++--------------------------- > 1 file changed, 94 insertions(+), 94 deletions(-) > > diff --git a/mm/zswap.c b/mm/zswap.c > index 3c0fd8a137182..3538ecaed5e16 100644 > --- a/mm/zswap.c > +++ b/mm/zswap.c > @@ -248,6 +248,100 @@ static inline struct xarray *swap_zswap_tree(swp_entry_t swp) > **********************************/ > static void __zswap_pool_empty(struct percpu_ref *ref); > > +static int zswap_cpu_comp_prepare(unsigned int cpu, struct hlist_node *node) > +{ > + struct zswap_pool *pool = hlist_entry(node, struct zswap_pool, node); > + struct crypto_acomp_ctx *acomp_ctx = per_cpu_ptr(pool->acomp_ctx, cpu); > + struct crypto_acomp *acomp = NULL; > + struct acomp_req *req = NULL; > + u8 *buffer = NULL; > + int ret; > + > + buffer = kmalloc_node(PAGE_SIZE * 2, GFP_KERNEL, cpu_to_node(cpu)); > + if (!buffer) { > + ret = -ENOMEM; > + goto fail; > + } > + > + acomp = crypto_alloc_acomp_node(pool->tfm_name, 0, 0, cpu_to_node(cpu)); > + if (IS_ERR(acomp)) { > + pr_err("could not alloc crypto acomp %s : %ld\n", > + pool->tfm_name, PTR_ERR(acomp)); > + ret = PTR_ERR(acomp); > + goto fail; > + } > + > + req = acomp_request_alloc(acomp); > + if (!req) { > + pr_err("could not alloc crypto acomp_request %s\n", > + pool->tfm_name); > + ret = -ENOMEM; > + goto fail; > + } > + > + /* > + * Only hold the mutex after completing allocations, otherwise we may > + * recurse into zswap through reclaim and attempt to hold the mutex > + * again resulting in a deadlock. > + */ > + mutex_lock(&acomp_ctx->mutex); > + crypto_init_wait(&acomp_ctx->wait); > + > + /* > + * if the backend of acomp is async zip, crypto_req_done() will wakeup > + * crypto_wait_req(); if the backend of acomp is scomp, the callback > + * won't be called, crypto_wait_req() will return without blocking. > + */ > + acomp_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG, > + crypto_req_done, &acomp_ctx->wait); > + > + acomp_ctx->buffer = buffer; > + acomp_ctx->acomp = acomp; > + acomp_ctx->is_sleepable = acomp_is_async(acomp); > + acomp_ctx->req = req; > + mutex_unlock(&acomp_ctx->mutex); > + return 0; > + > +fail: > + if (acomp) > + crypto_free_acomp(acomp); > + kfree(buffer); > + return ret; > +} > + > +static int zswap_cpu_comp_dead(unsigned int cpu, struct hlist_node *node) > +{ > + struct zswap_pool *pool = hlist_entry(node, struct zswap_pool, node); > + struct crypto_acomp_ctx *acomp_ctx = per_cpu_ptr(pool->acomp_ctx, cpu); > + struct acomp_req *req; > + struct crypto_acomp *acomp; > + u8 *buffer; > + > + if (IS_ERR_OR_NULL(acomp_ctx)) > + return 0; > + > + mutex_lock(&acomp_ctx->mutex); > + req = acomp_ctx->req; > + acomp = acomp_ctx->acomp; > + buffer = acomp_ctx->buffer; > + acomp_ctx->req = NULL; > + acomp_ctx->acomp = NULL; > + acomp_ctx->buffer = NULL; > + mutex_unlock(&acomp_ctx->mutex); > + > + /* > + * Do the actual freeing after releasing the mutex to avoid subtle > + * locking dependencies causing deadlocks. > + */ > + if (!IS_ERR_OR_NULL(req)) > + acomp_request_free(req); > + if (!IS_ERR_OR_NULL(acomp)) > + crypto_free_acomp(acomp); > + kfree(buffer); > + > + return 0; > +} > + > static struct zswap_pool *zswap_pool_create(char *type, char *compressor) > { > struct zswap_pool *pool; > @@ -818,100 +912,6 @@ static void zswap_entry_free(struct zswap_entry *entry) > /********************************* > * compressed storage functions > **********************************/ > -static int zswap_cpu_comp_prepare(unsigned int cpu, struct hlist_node *node) > -{ > - struct zswap_pool *pool = hlist_entry(node, struct zswap_pool, node); > - struct crypto_acomp_ctx *acomp_ctx = per_cpu_ptr(pool->acomp_ctx, cpu); > - struct crypto_acomp *acomp = NULL; > - struct acomp_req *req = NULL; > - u8 *buffer = NULL; > - int ret; > - > - buffer = kmalloc_node(PAGE_SIZE * 2, GFP_KERNEL, cpu_to_node(cpu)); > - if (!buffer) { > - ret = -ENOMEM; > - goto fail; > - } > - > - acomp = crypto_alloc_acomp_node(pool->tfm_name, 0, 0, cpu_to_node(cpu)); > - if (IS_ERR(acomp)) { > - pr_err("could not alloc crypto acomp %s : %ld\n", > - pool->tfm_name, PTR_ERR(acomp)); > - ret = PTR_ERR(acomp); > - goto fail; > - } > - > - req = acomp_request_alloc(acomp); > - if (!req) { > - pr_err("could not alloc crypto acomp_request %s\n", > - pool->tfm_name); > - ret = -ENOMEM; > - goto fail; > - } > - > - /* > - * Only hold the mutex after completing allocations, otherwise we may > - * recurse into zswap through reclaim and attempt to hold the mutex > - * again resulting in a deadlock. > - */ > - mutex_lock(&acomp_ctx->mutex); > - crypto_init_wait(&acomp_ctx->wait); > - > - /* > - * if the backend of acomp is async zip, crypto_req_done() will wakeup > - * crypto_wait_req(); if the backend of acomp is scomp, the callback > - * won't be called, crypto_wait_req() will return without blocking. > - */ > - acomp_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG, > - crypto_req_done, &acomp_ctx->wait); > - > - acomp_ctx->buffer = buffer; > - acomp_ctx->acomp = acomp; > - acomp_ctx->is_sleepable = acomp_is_async(acomp); > - acomp_ctx->req = req; > - mutex_unlock(&acomp_ctx->mutex); > - return 0; > - > -fail: > - if (acomp) > - crypto_free_acomp(acomp); > - kfree(buffer); > - return ret; > -} > - > -static int zswap_cpu_comp_dead(unsigned int cpu, struct hlist_node *node) > -{ > - struct zswap_pool *pool = hlist_entry(node, struct zswap_pool, node); > - struct crypto_acomp_ctx *acomp_ctx = per_cpu_ptr(pool->acomp_ctx, cpu); > - struct acomp_req *req; > - struct crypto_acomp *acomp; > - u8 *buffer; > - > - if (IS_ERR_OR_NULL(acomp_ctx)) > - return 0; > - > - mutex_lock(&acomp_ctx->mutex); > - req = acomp_ctx->req; > - acomp = acomp_ctx->acomp; > - buffer = acomp_ctx->buffer; > - acomp_ctx->req = NULL; > - acomp_ctx->acomp = NULL; > - acomp_ctx->buffer = NULL; > - mutex_unlock(&acomp_ctx->mutex); > - > - /* > - * Do the actual freeing after releasing the mutex to avoid subtle > - * locking dependencies causing deadlocks. > - */ > - if (!IS_ERR_OR_NULL(req)) > - acomp_request_free(req); > - if (!IS_ERR_OR_NULL(acomp)) > - crypto_free_acomp(acomp); > - kfree(buffer); > - > - return 0; > -} > - > static struct crypto_acomp_ctx *acomp_ctx_get_cpu_lock(struct zswap_pool *pool) > { > struct crypto_acomp_ctx *acomp_ctx; > -- > 2.27.0 >