RE: [PATCH v4 09/10] mm: zswap: Allocate pool batching resources if the crypto_alg supports batching.

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

> -----Original Message-----
> From: Yosry Ahmed <yosryahmed@google.com>
> Sent: Wednesday, December 4, 2024 2:36 PM
> To: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: Sridhar, Kanchana P <kanchana.p.sridhar@intel.com>; Nhat Pham
> <nphamcs@gmail.com>; linux-kernel@vger.kernel.org; linux-mm@kvack.org;
> hannes@cmpxchg.org; chengming.zhou@linux.dev;
> usamaarif642@gmail.com; ryan.roberts@arm.com; 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>; Feghali, Wajdi K
> <wajdi.k.feghali@intel.com>; Gopal, Vinodh <vinodh.gopal@intel.com>
> Subject: Re: [PATCH v4 09/10] mm: zswap: Allocate pool batching resources if
> the crypto_alg supports batching.
> 
> On Tue, Dec 3, 2024 at 5:42 PM Herbert Xu <herbert@gondor.apana.org.au>
> wrote:
> >
> > On Tue, Dec 03, 2024 at 01:44:00PM -0800, Yosry Ahmed wrote:
> > >
> > > Does this mean that instead of zswap breaking down the folio into
> > > SWAP_CRYPTO_BATCH_SIZE -sized batches, we pass all the pages to the
> > > crypto layer and let it do the batching as it pleases?
> >
> > You provide as much (or little) as you're comfortable with.  Just
> > treat the acomp API as one that can take as much as you want to
> > give it.
> 
> In this case, it seems like the batch size is completely up to zswap,
> and not necessarily dependent on the compressor. That being said,
> Intel IAA will naturally prefer a batch size that maximizes the
> parallelization.
> 
> How about this, we can define a fixed max batch size in zswap, to
> provide a hard limit on the number of buffers we preallocate (e.g.
> MAX_BATCH_SIZE). The compressors can provide zswap a hint with their
> desired batch size (e.g. 8 for Intel IAA). Then zswap can allocate
> min(MAX_BATCH_SIZE, compressor_batch_size).
> 
> Assuming software compressors provide 1 for the batch size, if
> MAX_BATCH_SIZE is >= 8, Intel IAA gets the batching rate it wants, and
> software compressors get the same behavior as today. This abstracts
> the batch size needed by the compressor while making sure zswap does
> not preallocate a ridiculous amount of memory.
> 
> Does this make sense to everyone or am I missing something?

Thanks Yosry, this makes perfect sense. I can declare a default
CRYPTO_ACOMP_BATCH_SIZE=1, and a crypto API that zswap can
query, acomp_get_batch_size(struct crypto_acomp *tfm) that
can call a crypto algorithm interface if it is registered, for e.g.
crypto_get_batch_size() that IAA can register to return the max
batch size for IAA. If a compressor does not provide an
implementation for crypto_get_batch_size(), we would return
CRYPTO_ACOMP_BATCH_SIZE. This way, nothing specific will
need to be done for the software compressors for now. Unless
they define a specific batch_size via say, another interface,
crypto_set_batch_size(), the acomp_get_batch_size() will return 1.

Thanks,
Kanchana