> -----Original Message----- > From: Yosry Ahmed <yosryahmed@xxxxxxxxxx> > Sent: Wednesday, December 4, 2024 2:55 PM > To: Sridhar, Kanchana P <kanchana.p.sridhar@xxxxxxxxx> > Cc: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>; 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 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 sure, zswap should set the MAX_BATCH_SIZE for this purpose. > > For the acomp stuff I will let Herbert decide what he thinks is best. > From the zswap side, I just want: > - A hard limit on the amount of memory we preallocate. > - No change for the software compressors. Sounds good! Thanks, Kanchana > > > > > Thanks, > > Kanchana
