> -----Original Message----- > From: Vitaly Wool [mailto:[email protected]] > Sent: Sunday, June 21, 2020 11:16 PM > To: Song Bao Hua (Barry Song) <[email protected]> > Cc: Andrew Morton <[email protected]>; > [email protected]; [email protected]; > [email protected]; Linux-MM <[email protected]>; LKML > <[email protected]>; Linuxarm <[email protected]>; Luis > Claudio R . Goncalves <[email protected]>; Sebastian Andrzej Siewior > <[email protected]>; Mahipal Challa <[email protected]>; > Seth Jennings <[email protected]>; Dan Streetman <[email protected]>; > Wangzhou (B) <[email protected]> > Subject: Re: [PATCH v2] mm/zswap: move to use crypto_acomp API for > hardware acceleration > > On Sun, Jun 21, 2020 at 1:52 AM Barry Song <[email protected]> > wrote: > > > > right now, all new ZIP drivers are using crypto_acomp APIs rather than > > legacy crypto_comp APIs. But zswap.c is still using the old APIs. That > > means zswap won't be able to use any new zip drivers in kernel. > > > > This patch moves to use cryto_acomp APIs to fix the problem. On the > > other hand, tradiontal compressors like lz4,lzo etc have been wrapped > > into acomp via scomp backend. So platforms without async compressors > > can fallback to use acomp via scomp backend. > > > > Cc: Luis Claudio R. Goncalves <[email protected]> > > Cc: Sebastian Andrzej Siewior <[email protected]> > > Cc: Andrew Morton <[email protected]> > > Cc: Herbert Xu <[email protected]> > > Cc: David S. Miller <[email protected]> > > Cc: Mahipal Challa <[email protected]> > > Cc: Seth Jennings <[email protected]> > > Cc: Dan Streetman <[email protected]> > > Cc: Vitaly Wool <[email protected]> > > Cc: Zhou Wang <[email protected]> > > Signed-off-by: Barry Song <[email protected]> > > --- > > -v2: > > rebase to 5.8-rc1; > > cleanup commit log; > > cleanup to improve the readability according to Sebastian's comment > > > > mm/zswap.c | 153 > > ++++++++++++++++++++++++++++++++++++++--------------- > > 1 file changed, 110 insertions(+), 43 deletions(-) > > > > diff --git a/mm/zswap.c b/mm/zswap.c > > index fbb782924ccc..0d914ba6b4a0 100644 > > --- a/mm/zswap.c > > +++ b/mm/zswap.c > > @@ -24,8 +24,10 @@ > > #include <linux/rbtree.h> > > #include <linux/swap.h> > > #include <linux/crypto.h> > > +#include <linux/scatterlist.h> > > #include <linux/mempool.h> > > #include <linux/zpool.h> > > +#include <crypto/acompress.h> > > > > #include <linux/mm_types.h> > > #include <linux/page-flags.h> > > @@ -127,9 +129,17 @@ > module_param_named(same_filled_pages_enabled, > > zswap_same_filled_pages_enabled, > > * data structures > > **********************************/ > > > > +struct crypto_acomp_ctx { > > + struct crypto_acomp *acomp; > > + struct acomp_req *req; > > + struct crypto_wait wait; > > + u8 *dstmem; > > + struct mutex mutex; > > +}; > > + > > struct zswap_pool { > > struct zpool *zpool; > > - struct crypto_comp * __percpu *tfm; > > + struct crypto_acomp_ctx * __percpu *acomp_ctx; > > struct kref kref; > > struct list_head list; > > struct work_struct release_work; @@ -415,30 +425,60 @@ static > > int zswap_dstmem_dead(unsigned int cpu) 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_comp *tfm; > > + struct crypto_acomp *acomp; > > + struct acomp_req *req; > > + struct crypto_acomp_ctx *acomp_ctx; > > > > - if (WARN_ON(*per_cpu_ptr(pool->tfm, cpu))) > > + if (WARN_ON(*per_cpu_ptr(pool->acomp_ctx, cpu))) > > return 0; > > > > - tfm = crypto_alloc_comp(pool->tfm_name, 0, 0); > > - if (IS_ERR_OR_NULL(tfm)) { > > - pr_err("could not alloc crypto comp %s : %ld\n", > > - pool->tfm_name, PTR_ERR(tfm)); > > + acomp_ctx = kzalloc(sizeof(*acomp_ctx), GFP_KERNEL); > > + if (IS_ERR_OR_NULL(acomp_ctx)) { > > + pr_err("Could not initialize acomp_ctx\n"); > > + return -ENOMEM; > > + } > > + acomp = crypto_alloc_acomp(pool->tfm_name, 0, 0); > > + if (IS_ERR_OR_NULL(acomp)) { > > + pr_err("could not alloc crypto acomp %s : %ld\n", > > + pool->tfm_name, PTR_ERR(acomp)); > > return -ENOMEM; > > } > > I bet you actually want to free acomp_ctx here. Overall, could you please > provide more careful error path implementation or explain why it isn't > necessary?
Oops. I could hardly believe my eyes. it is definitely necessary to free the allocated data struct here, will send an incremental patch to fix this. Thanks for your comment. Best Regards, Barry > > Best regards, > Vitaly
