diff mbox series

[RFC,v1,01/13] crypto: acomp - Add a poll() operation to acomp_alg and acomp_req

Message ID 20241018064101.336232-2-kanchana.p.sridhar@intel.com (mailing list archive)
State RFC
Delegated to: Herbert Xu
Headers show
Series zswap IAA compress batching | expand

Commit Message

Sridhar, Kanchana P Oct. 18, 2024, 6:40 a.m. UTC 
For async compress/decompress, provide a way for the caller to poll
for compress/decompress completion, rather than wait for an interrupt
to signal completion.

Callers can submit a compress/decompress using crypto_acomp_compress
and decompress and rather than wait on a completion, call
crypto_acomp_poll() to check for completion.

This is useful for hardware accelerators where the overhead of
interrupts and waiting for completions is too expensive.  Typically
the compress/decompress hw operations complete very quickly and in the
vast majority of cases, adding the overhead of interrupt handling and
waiting for completions simply adds unnecessary delays and cancels the
gains of using the hw acceleration.

Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
Signed-off-by: Kanchana P Sridhar <kanchana.p.sridhar@intel.com>
---
 crypto/acompress.c                  |  1 +
 include/crypto/acompress.h          | 18 ++++++++++++++++++
 include/crypto/internal/acompress.h |  1 +
 3 files changed, 20 insertions(+)

Comments

Herbert Xu Oct. 18, 2024, 7:55 a.m. UTC | #1 
On Thu, Oct 17, 2024 at 11:40:49PM -0700, Kanchana P Sridhar wrote:
> For async compress/decompress, provide a way for the caller to poll
> for compress/decompress completion, rather than wait for an interrupt
> to signal completion.
> 
> Callers can submit a compress/decompress using crypto_acomp_compress
> and decompress and rather than wait on a completion, call
> crypto_acomp_poll() to check for completion.
> 
> This is useful for hardware accelerators where the overhead of
> interrupts and waiting for completions is too expensive.  Typically
> the compress/decompress hw operations complete very quickly and in the
> vast majority of cases, adding the overhead of interrupt handling and
> waiting for completions simply adds unnecessary delays and cancels the
> gains of using the hw acceleration.
> 
> Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
> Signed-off-by: Kanchana P Sridhar <kanchana.p.sridhar@intel.com>
> ---
>  crypto/acompress.c                  |  1 +
>  include/crypto/acompress.h          | 18 ++++++++++++++++++
>  include/crypto/internal/acompress.h |  1 +
>  3 files changed, 20 insertions(+)

How about just adding a request flag that tells the driver to
make the request synchronous if possible?

Something like

#define CRYPTO_ACOMP_REQ_POLL	0x00000001

Cheers,
Sridhar, Kanchana P Oct. 18, 2024, 11:01 p.m. UTC | #2 
Hi Herbert,

> -----Original Message-----
> From: Herbert Xu <herbert@gondor.apana.org.au>
> Sent: Friday, October 18, 2024 12:55 AM
> To: Sridhar, Kanchana P <kanchana.p.sridhar@intel.com>
> Cc: linux-kernel@vger.kernel.org; linux-mm@kvack.org;
> hannes@cmpxchg.org; yosryahmed@google.com; nphamcs@gmail.com;
> chengming.zhou@linux.dev; usamaarif642@gmail.com;
> ryan.roberts@arm.com; Huang, Ying <ying.huang@intel.com>;
> 21cnbao@gmail.com; akpm@linux-foundation.org; linux-
> crypto@vger.kernel.org; davem@davemloft.net; clabbe@baylibre.com;
> ardb@kernel.org; ebiggers@google.com; surenb@google.com; Accardi,
> Kristen C <kristen.c.accardi@intel.com>; zanussi@kernel.org;
> viro@zeniv.linux.org.uk; brauner@kernel.org; jack@suse.cz;
> mcgrof@kernel.org; kees@kernel.org; joel.granados@kernel.org;
> bfoster@redhat.com; willy@infradead.org; linux-fsdevel@vger.kernel.org;
> Feghali, Wajdi K <wajdi.k.feghali@intel.com>; Gopal, Vinodh
> <vinodh.gopal@intel.com>
> Subject: Re: [RFC PATCH v1 01/13] crypto: acomp - Add a poll() operation to
> acomp_alg and acomp_req
> 
> On Thu, Oct 17, 2024 at 11:40:49PM -0700, Kanchana P Sridhar wrote:
> > For async compress/decompress, provide a way for the caller to poll
> > for compress/decompress completion, rather than wait for an interrupt
> > to signal completion.
> >
> > Callers can submit a compress/decompress using crypto_acomp_compress
> > and decompress and rather than wait on a completion, call
> > crypto_acomp_poll() to check for completion.
> >
> > This is useful for hardware accelerators where the overhead of
> > interrupts and waiting for completions is too expensive.  Typically
> > the compress/decompress hw operations complete very quickly and in the
> > vast majority of cases, adding the overhead of interrupt handling and
> > waiting for completions simply adds unnecessary delays and cancels the
> > gains of using the hw acceleration.
> >
> > Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
> > Signed-off-by: Kanchana P Sridhar <kanchana.p.sridhar@intel.com>
> > ---
> >  crypto/acompress.c                  |  1 +
> >  include/crypto/acompress.h          | 18 ++++++++++++++++++
> >  include/crypto/internal/acompress.h |  1 +
> >  3 files changed, 20 insertions(+)
> 
> How about just adding a request flag that tells the driver to
> make the request synchronous if possible?
> 
> Something like
> 
> #define CRYPTO_ACOMP_REQ_POLL	0x00000001

Thanks for your code review comments. Are you referring to how the
async/poll interface is enabled at the level of say zswap (by setting a
flag in the acomp_req), followed by the iaa_crypto driver testing for
the flag and submitting the request and returning -EINPROGRESS.
Wouldn't we still need a separate API to do the polling?

I am not the expert on this, and would like to request Kristen's inputs
on whether this is feasible.

Thanks,
Kanchana


> 
> Cheers,
> --
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Herbert Xu Oct. 19, 2024, 12:19 a.m. UTC | #3 
On Fri, Oct 18, 2024 at 11:01:10PM +0000, Sridhar, Kanchana P wrote:
>
> Thanks for your code review comments. Are you referring to how the
> async/poll interface is enabled at the level of say zswap (by setting a
> flag in the acomp_req), followed by the iaa_crypto driver testing for
> the flag and submitting the request and returning -EINPROGRESS.
> Wouldn't we still need a separate API to do the polling?

Correct me if I'm wrong, but I think what you want to do is this:

	crypto_acomp_compress(req)
	crypto_acomp_poll(req)

So instead of adding this interface, where the poll essentially
turns the request synchronous, just move this logic into the driver,
based on a flag bit in req.

Cheers,
Sridhar, Kanchana P Oct. 19, 2024, 7:10 p.m. UTC | #4 
> -----Original Message-----
> From: Herbert Xu <herbert@gondor.apana.org.au>
> Sent: Friday, October 18, 2024 5:20 PM
> To: Sridhar, Kanchana P <kanchana.p.sridhar@intel.com>
> Cc: linux-kernel@vger.kernel.org; linux-mm@kvack.org;
> hannes@cmpxchg.org; yosryahmed@google.com; nphamcs@gmail.com;
> chengming.zhou@linux.dev; usamaarif642@gmail.com;
> ryan.roberts@arm.com; Huang, Ying <ying.huang@intel.com>;
> 21cnbao@gmail.com; akpm@linux-foundation.org; linux-
> crypto@vger.kernel.org; davem@davemloft.net; clabbe@baylibre.com;
> ardb@kernel.org; ebiggers@google.com; surenb@google.com; Accardi,
> Kristen C <kristen.c.accardi@intel.com>; zanussi@kernel.org;
> viro@zeniv.linux.org.uk; brauner@kernel.org; jack@suse.cz;
> mcgrof@kernel.org; kees@kernel.org; joel.granados@kernel.org;
> bfoster@redhat.com; willy@infradead.org; linux-fsdevel@vger.kernel.org;
> Feghali, Wajdi K <wajdi.k.feghali@intel.com>; Gopal, Vinodh
> <vinodh.gopal@intel.com>
> Subject: Re: [RFC PATCH v1 01/13] crypto: acomp - Add a poll() operation to
> acomp_alg and acomp_req
> 
> On Fri, Oct 18, 2024 at 11:01:10PM +0000, Sridhar, Kanchana P wrote:
> >
> > Thanks for your code review comments. Are you referring to how the
> > async/poll interface is enabled at the level of say zswap (by setting a
> > flag in the acomp_req), followed by the iaa_crypto driver testing for
> > the flag and submitting the request and returning -EINPROGRESS.
> > Wouldn't we still need a separate API to do the polling?
> 
> Correct me if I'm wrong, but I think what you want to do is this:
> 
> 	crypto_acomp_compress(req)
> 	crypto_acomp_poll(req)
> 
> So instead of adding this interface, where the poll essentially
> turns the request synchronous, just move this logic into the driver,
> based on a flag bit in req.

Thanks Herbert, for this suggestion. I understand this better now,
and will work with Kristen for addressing this in v2.

Thanks,
Kanchana

> 
> Cheers,
> --
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
diff mbox series

Patch

diff --git a/crypto/acompress.c b/crypto/acompress.c
index 6fdf0ff9f3c0..00ec7faa2714 100644
--- a/crypto/acompress.c
+++ b/crypto/acompress.c
@@ -71,6 +71,7 @@  static int crypto_acomp_init_tfm(struct crypto_tfm *tfm)
 
 	acomp->compress = alg->compress;
 	acomp->decompress = alg->decompress;
+	acomp->poll = alg->poll;
 	acomp->dst_free = alg->dst_free;
 	acomp->reqsize = alg->reqsize;
 
diff --git a/include/crypto/acompress.h b/include/crypto/acompress.h
index 54937b615239..65b5de30c8b1 100644
--- a/include/crypto/acompress.h
+++ b/include/crypto/acompress.h
@@ -51,6 +51,7 @@  struct acomp_req {
 struct crypto_acomp {
 	int (*compress)(struct acomp_req *req);
 	int (*decompress)(struct acomp_req *req);
+	int (*poll)(struct acomp_req *req);
 	void (*dst_free)(struct scatterlist *dst);
 	unsigned int reqsize;
 	struct crypto_tfm base;
@@ -265,4 +266,21 @@  static inline int crypto_acomp_decompress(struct acomp_req *req)
 	return crypto_acomp_reqtfm(req)->decompress(req);
 }
 
+/**
+ * crypto_acomp_poll() -- Invoke asynchronous poll operation
+ *
+ * Function invokes the asynchronous poll operation
+ *
+ * @req:	asynchronous request
+ *
+ * Return: zero on poll completion, -EAGAIN if not complete, or
+ *         error code in case of error
+ */
+static inline int crypto_acomp_poll(struct acomp_req *req)
+{
+	struct crypto_acomp *tfm = crypto_acomp_reqtfm(req);
+
+	return tfm->poll(req);
+}
+
 #endif
diff --git a/include/crypto/internal/acompress.h b/include/crypto/internal/acompress.h
index 8831edaafc05..fbf5f6c6eeb6 100644
--- a/include/crypto/internal/acompress.h
+++ b/include/crypto/internal/acompress.h
@@ -37,6 +37,7 @@ 
 struct acomp_alg {
 	int (*compress)(struct acomp_req *req);
 	int (*decompress)(struct acomp_req *req);
+	int (*poll)(struct acomp_req *req);
 	void (*dst_free)(struct scatterlist *dst);
 	int (*init)(struct crypto_acomp *tfm);
 	void (*exit)(struct crypto_acomp *tfm);