Re: [PATCH 2/2] mm/zswap: classify compression dst overflow as poor compression

[Suseendran Bhaskaran]

On Wed, Oct 21, 2026 at 02:47:46PM -0700, Yosry Ahmed wrote:
> On Wed, Oct 21, 2026 at 1:57 PM Suseendran Bhaskaran
> <suseendranb@xxxxxxxxxx> wrote:
> >
> > 1. When zswap writeback is disabled (e.g. memory.zswap.writeback = 0),
> >    zswap_compress() sets 'comp_ret = comp_ret ? comp_ret : -EINVAL' and
> >    jumps to unlock. Whether the compressor fails on destination overflow
> >    with -EINVAL or succeeds with dlen == PAGE_SIZE, comp_ret becomes
> >    -EINVAL. Consequently, incompressible pages are misaccounted in
> >    debugfs under reject_compress_fail instead of reject_compress_poor
> >    (leaving reject_compress_poor stuck at 0 and triggering false
> >    compressor failure alerts on normal incompressible workloads).
>
> Hmm IIUC this is completely fixed by patch 1, so it doesn't belong here, right?

You are right, thank you for pointing this out. For the destination
buffer overflow case, Patch 1 alone fixes it since comp_ret is already
-ENOSPC and 'comp_ret ? comp_ret : -EINVAL' preserves it. The only case
not covered by Patch 1 when writeback is disabled is when compression
succeeds (!comp_ret) with dlen >= PAGE_SIZE, where 'comp_ret ? comp_ret :
-EINVAL' sets comp_ret to -EINVAL instead of -ENOSPC.

> > 2. When zswap writeback is enabled, any non-zero comp_ret (such as
> >    -EINVAL from an invalid compressor context, -ENOMEM, or a hardware
> >    accelerator error) is treated identically to an incompressible page:
> >    comp_ret is cleared to 0 and the raw page is stored uncompressed in
> >    zsmalloc. This silently masks genuine compressor failures
> >    (reject_compress_fail is never incremented) and miscounts pages that
> >    encountered a compressor error under stored_incompressible_pages.
>
> Storing pages in zswap when compression fails to avoid LRU inversion
> was intentional in commit dca4437a5861 ("mm/zswap: store <PAGE_SIZE
> compression failed page as-is"). Unless we think this needs to be
> changed, I think all we really need to do is make sure we account
> debugfs correctly for -ENOSPC vs other error codes? Or am I missing
> something?

Thank you for the context. Keeping the writeback-enabled behavior from
commit dca4437a5861 ("mm/zswap: store <PAGE_SIZE compression failed page
as-is") as-is makes sense.

With that unchanged, existing zswap_compress() already accounts -ENOSPC
vs other error codes when writeback is disabled, except for the
!comp_ret && dlen >= PAGE_SIZE case mentioned above:

-			comp_ret = comp_ret ? comp_ret : -EINVAL;
+			comp_ret = comp_ret ? comp_ret :
+				   (dlen >= PAGE_SIZE ? -ENOSPC : -EINVAL);

Would you prefer that I narrow Patch 2 to just that change in v2, or drop
Patch 2 and only send Patch 1?