linux-kernel - RE: [PATCH v4 09/10] mm: zswap: Allocate pool batching resources if the crypto

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <SJ0PR11MB5678F2E6E78A2B74D2F6AB8AC9372@SJ0PR11MB5678.namprd11.prod.outlook.com>
Date: Wed, 4 Dec 2024 22:49:16 +0000
From: "Sridhar, Kanchana P" <kanchana.p.sridhar@...el.com>
To: Yosry Ahmed <yosryahmed@...gle.com>, Herbert Xu
	<herbert@...dor.apana.org.au>
CC: 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: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.

Thanks,
Kanchana