Message-ID: <SJ0PR11MB56780FE68BA4F639C1261B13C9372@SJ0PR11MB5678.namprd11.prod.outlook.com>
Date: Wed, 4 Dec 2024 23:12:45 +0000
From: "Sridhar, Kanchana P" <kanchana.p.sridhar@...el.com>
To: Yosry Ahmed <yosryahmed@...gle.com>
CC: Herbert Xu <herbert@...dor.apana.org.au>, Nhat Pham <nphamcs@...il.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-mm@...ck.org" <linux-mm@...ck.org>, "hannes@...xchg.org"
	<hannes@...xchg.org>, "chengming.zhou@...ux.dev" <chengming.zhou@...ux.dev>,
	"usamaarif642@...il.com" <usamaarif642@...il.com>, "ryan.roberts@....com"
	<ryan.roberts@....com>, "ying.huang@...el.com" <ying.huang@...el.com>,
	"21cnbao@...il.com" <21cnbao@...il.com>, "akpm@...ux-foundation.org"
	<akpm@...ux-foundation.org>, "linux-crypto@...r.kernel.org"
	<linux-crypto@...r.kernel.org>, "davem@...emloft.net" <davem@...emloft.net>,
	"clabbe@...libre.com" <clabbe@...libre.com>, "ardb@...nel.org"
	<ardb@...nel.org>, "ebiggers@...gle.com" <ebiggers@...gle.com>,
	"surenb@...gle.com" <surenb@...gle.com>, "Accardi, Kristen C"
	<kristen.c.accardi@...el.com>, "Feghali, Wajdi K"
	<wajdi.k.feghali@...el.com>, "Gopal, Vinodh" <vinodh.gopal@...el.com>,
	"Sridhar, Kanchana P" <kanchana.p.sridhar@...el.com>
Subject: RE: [PATCH v4 09/10] mm: zswap: Allocate pool batching resources if
 the crypto_alg supports batching.


> -----Original Message-----
> From: Yosry Ahmed <yosryahmed@...gle.com>
> Sent: Wednesday, December 4, 2024 2:55 PM
> To: Sridhar, Kanchana P <kanchana.p.sridhar@...el.com>
> Cc: Herbert Xu <herbert@...dor.apana.org.au>; Nhat Pham
> <nphamcs@...il.com>; linux-kernel@...r.kernel.org; linux-mm@...ck.org;
> hannes@...xchg.org; chengming.zhou@...ux.dev;
> usamaarif642@...il.com; ryan.roberts@....com; ying.huang@...el.com;
> 21cnbao@...il.com; akpm@...ux-foundation.org; linux-
> crypto@...r.kernel.org; davem@...emloft.net; clabbe@...libre.com;
> ardb@...nel.org; ebiggers@...gle.com; surenb@...gle.com; Accardi,
> Kristen C <kristen.c.accardi@...el.com>; Feghali, Wajdi K
> <wajdi.k.feghali@...el.com>; Gopal, Vinodh <vinodh.gopal@...el.com>
> 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@...el.com> wrote:
> >
> >
> > > -----Original Message-----
> > > From: Yosry Ahmed <yosryahmed@...gle.com>
> > > Sent: Wednesday, December 4, 2024 2:36 PM
> > > To: Herbert Xu <herbert@...dor.apana.org.au>
> > > Cc: Sridhar, Kanchana P <kanchana.p.sridhar@...el.com>; Nhat Pham
> > > <nphamcs@...il.com>; linux-kernel@...r.kernel.org; linux-
> mm@...ck.org;
> > > hannes@...xchg.org; chengming.zhou@...ux.dev;
> > > usamaarif642@...il.com; ryan.roberts@....com;
> ying.huang@...el.com;
> > > 21cnbao@...il.com; akpm@...ux-foundation.org; linux-
> > > crypto@...r.kernel.org; davem@...emloft.net; clabbe@...libre.com;
> > > ardb@...nel.org; ebiggers@...gle.com; surenb@...gle.com; Accardi,
> > > Kristen C <kristen.c.accardi@...el.com>; Feghali, Wajdi K
> > > <wajdi.k.feghali@...el.com>; Gopal, Vinodh <vinodh.gopal@...el.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@...dor.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.
> 
> 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

Powered by blists - more mailing lists