On Tue, Feb 25, 2025 at 10:49 AM Yosry Ahmed <yosry.ahmed@xxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxx/ 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
