RE: [PATCH v7 01/15] crypto: acomp - Add synchronous/asynchronous acomp request chaining.

["Sridhar, Kanchana P" <kanchana.p.sridhar@intel.com>]

> -----Original Message-----
> From: Herbert Xu <herbert@gondor.apana.org.au>
> Sent: Monday, March 3, 2025 9:20 PM
> To: Sridhar, Kanchana P <kanchana.p.sridhar@intel.com>
> Cc: linux-kernel@vger.kernel.org; linux-mm@kvack.org;
> hannes@cmpxchg.org; yosry.ahmed@linux.dev; nphamcs@gmail.com;
> chengming.zhou@linux.dev; usamaarif642@gmail.com;
> ryan.roberts@arm.com; 21cnbao@gmail.com;
> ying.huang@linux.alibaba.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>; Feghali, Wajdi K
> <wajdi.k.feghali@intel.com>; Gopal, Vinodh <vinodh.gopal@intel.com>
> Subject: Re: [PATCH v7 01/15] crypto: acomp - Add
> synchronous/asynchronous acomp request chaining.
> 
> On Fri, Feb 28, 2025 at 02:00:10AM -0800, Kanchana P Sridhar wrote:
> >
> >  Step 2: Process the request chain using the specified compress/decompress
> >          "op":
> >
> >   2.a) Synchronous: the chain of requests is processed in series:
> >
> >        int acomp_do_req_chain(struct acomp_req *req,
> >                               int (*op)(struct acomp_req *req));
> >
> >   2.b) Asynchronous: the chain of requests is processed in parallel using a
> >        submit-poll paradigm:
> >
> >        int acomp_do_async_req_chain(struct acomp_req *req,
> >                                     int (*op_submit)(struct acomp_req *req),
> >                                     int (*op_poll)(struct acomp_req *req));
> >
> > Request chaining will be used in subsequent patches to implement
> > compress/decompress batching in the iaa_crypto driver for the two
> supported
> > IAA driver sync_modes:
> >
> >   sync_mode = 'sync' will use (2.a),
> >   sync_mode = 'async' will use (2.b).
> 
> There shouldn't be any sync/async toggle.  The whole zswap code is
> synchronous only and it makes zero sense to expose this to the user.
> Just do whatever is the fastest from the driver's point of view.

These sync/async modes are mentioned here to distinguish between how 
request chaining will be supported from the iaa_crypto driver's perspective.
As you are aware, the iaa_crypto driver supports a fully synchronous mode
(sync_mode = 'sync') and a fully asynchronous, non-irq mode (sync_mode = 'async').

As mentioned in the cover letter, from zswap's perspective, the calls to crypto
are exactly the same whether or not batching (with request chaining) or sequential
compressions are used:

crypto_wait_req(crypto_acomp_compress(acomp_ctx->reqs[0]), &acomp_ctx->wait);


> 
> I've actually implemented acomp chaining in my tree and I will be
> reposting soon.

Thanks for the update. I took at look at your v2 [1], and will provide some
code review comments in [1]. My main open question is, how is this supposed
to work for iaa_crypto's submit-poll paradigm which is crucial to derive the 
benefits of hardware parallelism? IIUC, your v2 acomp request chaining only
works in fully synchronous mode, correct me if I am wrong.

In fact, at the start of "acomp_do_req_chain()", if the algorithm has opted in to
request chaining, it returns after processing the first request, without processing
the chained requests. These are some of the issues I had to resolve when using
the ahash reference implementation you provided, to develop my version of
acomp_do_request_chain() and acomp_do_async_request_chain() in patch 1
of my series.

I see that in your v2, you have introduced support for virtual addresses. One
suggestion I have is, can you please incorporate my implementation of acomp
request chaining (for both the above APIs) in a v3 of your patch-series that
enables both, acomp_do_async_request_chain() and acomp_do_request_chain(),
fixes some of the issues I pointed out above, and adds in the virtual address
support. Please let me know if this would be a good way for us to proceed in
getting zswap to realize the benefits of IAA batch compressions.

[1] https://patchwork.kernel.org/project/linux-mm/patch/a11883ded326c4f4f80dcf0307ad05fd8e31abc7.1741080140.git.herbert@gondor.apana.org.au/

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