diff mbox series

[1/3] crypto: introduce acomp_is_async to expose if a acomp has a scomp backend

Message ID 20240103095006.608744-2-21cnbao@gmail.com (mailing list archive)
State Changes Requested
Delegated to: Herbert Xu
Headers show
Series mm/zswap & crypto/acompress: remove a couple of memcpy | expand

Commit Message

Barry Song Jan. 3, 2024, 9:50 a.m. UTC
From: Barry Song <v-songbaohua@oppo.com>

Almost all CPU-based compressors/decompressors are actually synchronous
though they support acomp APIs. While some hardware has hardware-based
accelerators to offload CPU's work such as hisilicon and intel/qat/,
their drivers are working in async mode.
Letting acomp's users know exactly if the acomp is really async will
help users know if the compression and decompression procedure can
sleep.

Signed-off-by: Barry Song <v-songbaohua@oppo.com>
Tested-by: Chengming Zhou <zhouchengming@bytedance.com>
---
 crypto/acompress.c         | 8 ++++++++
 include/crypto/acompress.h | 9 +++++++++
 2 files changed, 17 insertions(+)

Comments

Yosry Ahmed Jan. 8, 2024, 10:35 p.m. UTC | #1
On Wed, Jan 3, 2024 at 1:50 AM Barry Song <21cnbao@gmail.com> wrote:
>
> From: Barry Song <v-songbaohua@oppo.com>
>
> Almost all CPU-based compressors/decompressors are actually synchronous
> though they support acomp APIs. While some hardware has hardware-based
> accelerators to offload CPU's work such as hisilicon and intel/qat/,
> their drivers are working in async mode.
> Letting acomp's users know exactly if the acomp is really async will
> help users know if the compression and decompression procedure can
> sleep.
>
> Signed-off-by: Barry Song <v-songbaohua@oppo.com>
> Tested-by: Chengming Zhou <zhouchengming@bytedance.com>
> ---
>  crypto/acompress.c         | 8 ++++++++
>  include/crypto/acompress.h | 9 +++++++++
>  2 files changed, 17 insertions(+)
>
> diff --git a/crypto/acompress.c b/crypto/acompress.c
> index 1c682810a484..99118e879a4a 100644
> --- a/crypto/acompress.c
> +++ b/crypto/acompress.c
> @@ -152,6 +152,14 @@ struct crypto_acomp *crypto_alloc_acomp_node(const char *alg_name, u32 type,
>  }
>  EXPORT_SYMBOL_GPL(crypto_alloc_acomp_node);
>
> +bool acomp_is_async(struct crypto_acomp *acomp)

Is synchronous semantically the same as sleepable? IIUC synchronous
code may still sleep, at least generally. The purpose of this change
is to know whether we will sleep or not in the zswap code, so I
suggest the code should be explicit about sleep-ability instead (e.g.
acomp_is_sleepable or acomp_may_sleep).
Barry Song Jan. 9, 2024, 3:38 a.m. UTC | #2
On Tue, Jan 9, 2024 at 6:36 AM Yosry Ahmed <yosryahmed@google.com> wrote:
>
> On Wed, Jan 3, 2024 at 1:50 AM Barry Song <21cnbao@gmail.com> wrote:
> >
> > From: Barry Song <v-songbaohua@oppo.com>
> >
> > Almost all CPU-based compressors/decompressors are actually synchronous
> > though they support acomp APIs. While some hardware has hardware-based
> > accelerators to offload CPU's work such as hisilicon and intel/qat/,
> > their drivers are working in async mode.
> > Letting acomp's users know exactly if the acomp is really async will
> > help users know if the compression and decompression procedure can
> > sleep.
> >
> > Signed-off-by: Barry Song <v-songbaohua@oppo.com>
> > Tested-by: Chengming Zhou <zhouchengming@bytedance.com>
> > ---
> >  crypto/acompress.c         | 8 ++++++++
> >  include/crypto/acompress.h | 9 +++++++++
> >  2 files changed, 17 insertions(+)
> >
> > diff --git a/crypto/acompress.c b/crypto/acompress.c
> > index 1c682810a484..99118e879a4a 100644
> > --- a/crypto/acompress.c
> > +++ b/crypto/acompress.c
> > @@ -152,6 +152,14 @@ struct crypto_acomp *crypto_alloc_acomp_node(const char *alg_name, u32 type,
> >  }
> >  EXPORT_SYMBOL_GPL(crypto_alloc_acomp_node);
> >
> > +bool acomp_is_async(struct crypto_acomp *acomp)
>
> Is synchronous semantically the same as sleepable? IIUC synchronous
> code may still sleep, at least generally. The purpose of this change
> is to know whether we will sleep or not in the zswap code, so I
> suggest the code should be explicit about sleep-ability instead (e.g.
> acomp_is_sleepable or acomp_may_sleep).

Thanks, Tosry. sounds reasonable.

I'd like to ask for Herbert's comment, do we have a better way to know
if an acomp can sleep other than checking the below?

return tfm->__crt_alg->cra_type == &crypto_acomp_type;

Thanks
Barry
diff mbox series

Patch

diff --git a/crypto/acompress.c b/crypto/acompress.c
index 1c682810a484..99118e879a4a 100644
--- a/crypto/acompress.c
+++ b/crypto/acompress.c
@@ -152,6 +152,14 @@  struct crypto_acomp *crypto_alloc_acomp_node(const char *alg_name, u32 type,
 }
 EXPORT_SYMBOL_GPL(crypto_alloc_acomp_node);
 
+bool acomp_is_async(struct crypto_acomp *acomp)
+{
+	struct crypto_tfm *tfm = crypto_acomp_tfm(acomp);
+
+	return tfm->__crt_alg->cra_type == &crypto_acomp_type;
+}
+EXPORT_SYMBOL_GPL(acomp_is_async);
+
 struct acomp_req *acomp_request_alloc(struct crypto_acomp *acomp)
 {
 	struct crypto_tfm *tfm = crypto_acomp_tfm(acomp);
diff --git a/include/crypto/acompress.h b/include/crypto/acompress.h
index 574cffc90730..d91830c2d442 100644
--- a/include/crypto/acompress.h
+++ b/include/crypto/acompress.h
@@ -204,6 +204,15 @@  struct acomp_req *acomp_request_alloc(struct crypto_acomp *tfm);
  */
 void acomp_request_free(struct acomp_req *req);
 
+/**
+ * acomp_is_async() -- check if an acomp is asynchronous(can sleep)
+ *
+ * @tfm:	ACOMPRESS tfm handle allocated with crypto_alloc_acomp()
+ *
+ * Return:	true if the acomp is asynchronous, otherwise, false
+ */
+bool acomp_is_async(struct crypto_acomp *tfm);
+
 /**
  * acomp_request_set_callback() -- Sets an asynchronous callback
  *