linux-kernel - Re: [PATCH v5 02/12] crypto: acomp - Define new interfaces for compress/decompress batching.

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] [day] [month] [year] [list]

Message-ID: <Z7zpZlWmSV15EUVV@google.com>
Date: Mon, 24 Feb 2025 21:49:26 +0000
From: Yosry Ahmed <yosry.ahmed@...ux.dev>
To: Barry Song <21cnbao@...il.com>
Cc: Herbert Xu <herbert@...dor.apana.org.au>,
	Minchan Kim <minchan@...nel.org>,
	Sergey Senozhatsky <senozhatsky@...omium.org>,
	"Sridhar, Kanchana P" <kanchana.p.sridhar@...el.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>,
	"nphamcs@...il.com" <nphamcs@...il.com>,
	"chengming.zhou@...ux.dev" <chengming.zhou@...ux.dev>,
	"usamaarif642@...il.com" <usamaarif642@...il.com>,
	"ryan.roberts@....com" <ryan.roberts@....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>
Subject: Re: [PATCH v5 02/12] crypto: acomp - Define new interfaces for
 compress/decompress batching.

On Sat, Feb 22, 2025 at 08:13:13PM +1300, Barry Song wrote:
> On Sat, Feb 22, 2025 at 7:52 PM Herbert Xu <herbert@...dor.apana.org.au> wrote:
> >
> > On Sat, Feb 22, 2025 at 07:41:54PM +1300, Barry Song wrote:
> > >
> > > probably no, as an incompressible page might become compressible
> > > after changing an algorithm. This is possible, users may swith an
> > > algorithm to compress an incompressible page in the background.
> >
> > I don't understand the difference.  If something is wrong with
> > the system causing the compression algorithm to fail, shouldn't
> > zswap just hobble along as if the page was incompressible?
> >
> > In fact it would be quite reasonble to try to recompress it if
> > the admin did change the algorithm later because the error may
> > have been specific to the previous algorithm implementation.
> >
> 
> Somehow, I find your comment reasonable. Another point I want
> to mention is the semantic difference. For example, in a system
> with only one algorithm, a dst_buf overflow still means a successful
> swap-out. However, other errors actually indicate an I/O failure.
> In such cases, vmscan.c will log the relevant error in pageout() to
> notify the user.
> 
> Anyway, I'm not an authority on this, so I’d like to see comments
> from Minchan, Sergey, and Yosry.

>From a zswap perspective, things are a bit simpler. Currently zswap
handles compression errors and pages compressing to above PAGE_SIZE in
the same way (because zs_pool_malloc() will fail for sizes larger than
PAGE_SIZE). In both cases, zswap_store() will err out, and the page will
either go to the underlying swap disk or reclaim of that page will fail
if writeback is disabled for this cgroup.

Zswap currently does not do anything special about incompressible pages,
it just passes them along to disk. So if the Crypto API can guarantee
that compression nevers writes past PAGE_SIZE, the main benefit for
zswap would be reducing the buffer size from PAGE_SIZE*2 to PAGE_SIZE.

If/when zswap develops handling of incompressible memory (to avoid LRU
inversion), I imagine we would handle compression errors and
incompressible pages similarly. In both cases we'd store the page as-is
and move th LRU along to write more pages to disk. There is no point to
fail the reclaim operation in this case, because unlike zram we do have
a choice :)