On Wed, Dec 4, 2024 at 2:49 PM Sridhar, Kanchana P <kanchana.p.sridhar@xxxxxxxxx> wrote: > > > > -----Original Message----- > > From: Yosry Ahmed <yosryahmed@xxxxxxxxxx> > > Sent: Wednesday, December 4, 2024 2:36 PM > > To: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx> > > Cc: Sridhar, Kanchana P <kanchana.p.sridhar@xxxxxxxxx>; Nhat Pham > > <nphamcs@xxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx; linux-mm@xxxxxxxxx; > > hannes@xxxxxxxxxxx; chengming.zhou@xxxxxxxxx; > > usamaarif642@xxxxxxxxx; ryan.roberts@xxxxxxx; ying.huang@xxxxxxxxx; > > 21cnbao@xxxxxxxxx; akpm@xxxxxxxxxxxxxxxxxxxx; linux- > > crypto@xxxxxxxxxxxxxxx; davem@xxxxxxxxxxxxx; clabbe@xxxxxxxxxxxx; > > ardb@xxxxxxxxxx; ebiggers@xxxxxxxxxx; surenb@xxxxxxxxxx; Accardi, > > Kristen C <kristen.c.accardi@xxxxxxxxx>; Feghali, Wajdi K > > <wajdi.k.feghali@xxxxxxxxx>; Gopal, Vinodh <vinodh.gopal@xxxxxxxxx> > > 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@xxxxxxxxxxxxxxxxxxx> > > 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. I still think zswap should define its own maximum to avoid having the compressors have complete control over the amount of memory that zswap preallocates. For the acomp stuff I will let Herbert decide what he thinks is best.
