[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAGsJ_4wr3aT7zxn1MTjUjvQV9jeCGn=xi6VZz0qPm=xEBM6Qdg@mail.gmail.com>
Date: Thu, 27 Feb 2025 16:05:14 +1300
From: Barry Song <21cnbao@...il.com>
To: Yosry Ahmed <yosry.ahmed@...ux.dev>
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 Tue, Feb 25, 2025 at 10:49 AM Yosry Ahmed <yosry.ahmed@...ux.dev> wrote:
>
> 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 :)
Yes. For zswap, I suppose we just need to wait until all driver issues are
resolved, such as:
crypto: lzo - Fix compression buffer overrun
https://lore.kernel.org/lkml/Z7_JOAgi-Ej3CCic@gondor.apana.org.au/
for zswap, we just need to address point 1, which is not the case yet.
"
> 1. All drivers must be capable of handling dst_buf overflow.
-Not the case.
"
Thanks
Barry
Powered by blists - more mailing lists