[PATCH 0/2] mm/zswap: distinguish incompressible pages from compressor failures

[PATCH 0/2] mm/zswap: distinguish incompressible pages from compressor failures

[Suseendran Bhaskaran]

Commit dca4437a5861 ("mm/zswap: store <PAGE_SIZE compression failed page
as-is") and commit 0b1bf60c324a ("mm/zswap: reduce the size of the
compression buffer to a single page") updated zswap to pass a
single-page (PAGE_SIZE) destination buffer to crypto_acomp_compress()
(so incompressible pages typically overflow the PAGE_SIZE buffer due to
compressor metadata overhead). Because the single-buffer software
compressors return -EINVAL on destination buffer overflow and
zswap_compress() checks 'if (comp_ret || !dlen || dlen >= PAGE_SIZE)',
destination buffer overflow (incompressible page) is conflated with
unexpected compressor failures, causing two user-visible bugs:

1. Broken debugfs telemetry when writeback is disabled
   (memory.zswap.writeback = 0):
   Single-buffer software compressors (zstd_compress_one(), lz4, lz4hc,
   lzo, and lzo-rle) return -EINVAL on destination buffer overflow
   (whereas the multi-segment streaming path in zstd_compress(),
   deflate, and 842 return -ENOSPC). Furthermore, in zswap_compress(),
   'comp_ret = comp_ret ? comp_ret : -EINVAL' forces comp_ret to -EINVAL
   even when comp_ret == 0 and dlen == PAGE_SIZE. As a result,
   incompressible pages are misaccounted in
   /sys/kernel/debug/zswap/reject_compress_fail instead of
   reject_compress_poor, leaving reject_compress_poor at 0 and causing
   false-positive compressor failure alerts in monitoring.

2. Silent masking of real compressor failures when writeback is enabled
   (memory.zswap.writeback = 1):
   Because zswap_compress() checks 'if (comp_ret || !dlen || dlen >=
   PAGE_SIZE)' and unconditionally clears comp_ret = 0 to store the raw
   page in zsmalloc, genuine compressor errors (such as -ENOMEM,
   -EINVAL from an invalid context, or hardware accelerator errors) are
   silently masked as incompressible pages. Users see 0
   reject_compress_fail events, and pages that encounter an unexpected
   compressor error are miscounted under stored_incompressible_pages.

Patch 1 updates zstd_compress_one(), lz4, lz4hc, lzo, and lzo-rle to
return -ENOSPC on destination buffer overflow, matching the streaming
path in zstd_compress(), deflate, and 842.

Patch 2 updates zswap_compress() to treat only comp_ret == -ENOSPC or
(!comp_ret && dlen >= PAGE_SIZE) as an incompressible page (storing it
uncompressed in zswap when writeback is enabled to preserve LRU order,
or rejecting it with comp_ret = -ENOSPC when writeback is disabled so
zswap_reject_compress_poor is incremented), while jumping to unlock on
unexpected compressor errors (comp_ret || !dlen) so
zswap_reject_compress_fail is properly incremented.

Suseendran Bhaskaran (2):
  crypto: return -ENOSPC on compression destination overflow
  mm/zswap: classify compression dst overflow as poor compression

 crypto/lz4.c     | 2 +-
 crypto/lz4hc.c   | 2 +-
 crypto/lzo-rle.c | 2 +-
 crypto/lzo.c     | 2 +-
 crypto/zstd.c    | 5 ++++-
 mm/zswap.c       | 7 +++++--
 6 files changed, 13 insertions(+), 7 deletions(-)

-- 
2.56.0.218.g1ec904b173-goog

[PATCH 1/2] crypto: return -ENOSPC on compression destination overflow

[Suseendran Bhaskaran]

In the acomp/scomp compression interface, callers pass a bounded
destination buffer and rely on the return code to distinguish a
destination buffer overflow (incompressible input) from genuine
compressor failures (such as invalid parameters, uninitialized state, or
hardware errors).

While the multi-segment streaming path in zstd_compress(),
crypto/deflate.c, and lib/842/842_compress.c returns -ENOSPC when the
destination buffer runs out of space, the single-buffer path
zstd_compress_one() (taken when input and output are contiguous single
pages, such as in zswap) as well as lz4, lz4hc, lzo, and lzo-rle return
-EINVAL instead of -ENOSPC on destination buffer overflow.

For users such as zswap, which reduced its per-CPU compression buffer to
a single page in commit 0b1bf60c324a ("mm/zswap: reduce the size of the
compression buffer to a single page"), incompressible pages typically
overflow the PAGE_SIZE destination buffer due to compressor header/token
overhead. Because these compressors return -EINVAL rather than -ENOSPC,
zswap cannot distinguish a destination buffer overflow from a compressor
failure, causing broken debugfs error accounting (reject_compress_fail
vs. reject_compress_poor) or masking real compressor errors.

Update zstd_compress_one(), lz4, lz4hc, lzo, and lzo-rle to return
-ENOSPC when the compressed output overflows the destination buffer,
matching the streaming path in zstd_compress(), deflate, and 842.

Fixes: f5ad93ffb541 ("crypto: zstd - convert to acomp")
Signed-off-by: Suseendran Bhaskaran <suseendranb@google.com>
---
 crypto/lz4.c     | 2 +-
 crypto/lz4hc.c   | 2 +-
 crypto/lzo-rle.c | 2 +-
 crypto/lzo.c     | 2 +-
 crypto/zstd.c    | 5 ++++-
 5 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/crypto/lz4.c b/crypto/lz4.c
index 57b713516aef..c37e29afc831 100644
--- a/crypto/lz4.c
+++ b/crypto/lz4.c
@@ -35,7 +35,7 @@ static int __lz4_compress_crypto(const u8 *src, unsigned int slen,
 		slen, *dlen, ctx);
 
 	if (!out_len)
-		return -EINVAL;
+		return -ENOSPC;
 
 	*dlen = out_len;
 	return 0;
diff --git a/crypto/lz4hc.c b/crypto/lz4hc.c
index bb84f8a68cb5..3098466eb72a 100644
--- a/crypto/lz4hc.c
+++ b/crypto/lz4hc.c
@@ -33,7 +33,7 @@ static int __lz4hc_compress_crypto(const u8 *src, unsigned int slen,
 		*dlen, LZ4HC_DEFAULT_CLEVEL, ctx);
 
 	if (!out_len)
-		return -EINVAL;
+		return -ENOSPC;
 
 	*dlen = out_len;
 	return 0;
diff --git a/crypto/lzo-rle.c b/crypto/lzo-rle.c
index 794e7ec49536..483327873efd 100644
--- a/crypto/lzo-rle.c
+++ b/crypto/lzo-rle.c
@@ -34,7 +34,7 @@ static int __lzorle_compress(const u8 *src, unsigned int slen,
 	err = lzorle1x_1_compress_safe(src, slen, dst, &tmp_len, ctx);
 
 	if (err != LZO_E_OK)
-		return -EINVAL;
+		return err == LZO_E_OUTPUT_OVERRUN ? -ENOSPC : -EINVAL;
 
 	*dlen = tmp_len;
 	return 0;
diff --git a/crypto/lzo.c b/crypto/lzo.c
index d43242b24b4e..378b491b0cbd 100644
--- a/crypto/lzo.c
+++ b/crypto/lzo.c
@@ -34,7 +34,7 @@ static int __lzo_compress(const u8 *src, unsigned int slen,
 	err = lzo1x_1_compress_safe(src, slen, dst, &tmp_len, ctx);
 
 	if (err != LZO_E_OK)
-		return -EINVAL;
+		return err == LZO_E_OUTPUT_OVERRUN ? -ENOSPC : -EINVAL;
 
 	*dlen = tmp_len;
 	return 0;
diff --git a/crypto/zstd.c b/crypto/zstd.c
index 556f5d2bdd5f..7fafbdc19fd9 100644
--- a/crypto/zstd.c
+++ b/crypto/zstd.c
@@ -86,8 +86,11 @@ static int zstd_compress_one(struct acomp_req *req, struct zstd_ctx *ctx,
 
 	out_len = zstd_compress_cctx(ctx->cctx, dst, req->dlen, src, req->slen,
 				     &ctx->params);
-	if (zstd_is_error(out_len))
+	if (zstd_is_error(out_len)) {
+		if (zstd_get_error_code(out_len) == ZSTD_error_dstSize_tooSmall)
+			return -ENOSPC;
 		return -EINVAL;
+	}
 
 	*dlen = out_len;
 
-- 
2.56.0.218.g1ec904b173-goog

Re: [PATCH 1/2] crypto: return -ENOSPC on compression destination overflow

[Herbert Xu]

On Wed, Oct 21, 2026 at 08:56:25PM +0000, Suseendran Bhaskaran wrote:
> In the acomp/scomp compression interface, callers pass a bounded
> destination buffer and rely on the return code to distinguish a
> destination buffer overflow (incompressible input) from genuine
> compressor failures (such as invalid parameters, uninitialized state, or
> hardware errors).
> 
> While the multi-segment streaming path in zstd_compress(),
> crypto/deflate.c, and lib/842/842_compress.c returns -ENOSPC when the
> destination buffer runs out of space, the single-buffer path
> zstd_compress_one() (taken when input and output are contiguous single
> pages, such as in zswap) as well as lz4, lz4hc, lzo, and lzo-rle return
> -EINVAL instead of -ENOSPC on destination buffer overflow.
> 
> For users such as zswap, which reduced its per-CPU compression buffer to
> a single page in commit 0b1bf60c324a ("mm/zswap: reduce the size of the
> compression buffer to a single page"), incompressible pages typically
> overflow the PAGE_SIZE destination buffer due to compressor header/token
> overhead. Because these compressors return -EINVAL rather than -ENOSPC,
> zswap cannot distinguish a destination buffer overflow from a compressor
> failure, causing broken debugfs error accounting (reject_compress_fail
> vs. reject_compress_poor) or masking real compressor errors.
> 
> Update zstd_compress_one(), lz4, lz4hc, lzo, and lzo-rle to return
> -ENOSPC when the compressed output overflows the destination buffer,
> matching the streaming path in zstd_compress(), deflate, and 842.
> 
> Fixes: f5ad93ffb541 ("crypto: zstd - convert to acomp")
> Signed-off-by: Suseendran Bhaskaran <suseendranb@google.com>
> ---
>  crypto/lz4.c     | 2 +-
>  crypto/lz4hc.c   | 2 +-
>  crypto/lzo-rle.c | 2 +-
>  crypto/lzo.c     | 2 +-
>  crypto/zstd.c    | 5 ++++-
>  5 files changed, 8 insertions(+), 5 deletions(-)

I can take this patch through the cryptodev tree.  Is that OK with
you Yosry?

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH 1/2] crypto: return -ENOSPC on compression destination overflow

[Yosry Ahmed]

On Thu, Oct 22, 2026 at 1:14 AM Herbert Xu <herbert@gondor.apana.org.au> wrote:
>
> On Wed, Oct 21, 2026 at 08:56:25PM +0000, Suseendran Bhaskaran wrote:
> > In the acomp/scomp compression interface, callers pass a bounded
> > destination buffer and rely on the return code to distinguish a
> > destination buffer overflow (incompressible input) from genuine
> > compressor failures (such as invalid parameters, uninitialized state, or
> > hardware errors).
> >
> > While the multi-segment streaming path in zstd_compress(),
> > crypto/deflate.c, and lib/842/842_compress.c returns -ENOSPC when the
> > destination buffer runs out of space, the single-buffer path
> > zstd_compress_one() (taken when input and output are contiguous single
> > pages, such as in zswap) as well as lz4, lz4hc, lzo, and lzo-rle return
> > -EINVAL instead of -ENOSPC on destination buffer overflow.
> >
> > For users such as zswap, which reduced its per-CPU compression buffer to
> > a single page in commit 0b1bf60c324a ("mm/zswap: reduce the size of the
> > compression buffer to a single page"), incompressible pages typically
> > overflow the PAGE_SIZE destination buffer due to compressor header/token
> > overhead. Because these compressors return -EINVAL rather than -ENOSPC,
> > zswap cannot distinguish a destination buffer overflow from a compressor
> > failure, causing broken debugfs error accounting (reject_compress_fail
> > vs. reject_compress_poor) or masking real compressor errors.
> >
> > Update zstd_compress_one(), lz4, lz4hc, lzo, and lzo-rle to return
> > -ENOSPC when the compressed output overflows the destination buffer,
> > matching the streaming path in zstd_compress(), deflate, and 842.
> >
> > Fixes: f5ad93ffb541 ("crypto: zstd - convert to acomp")
> > Signed-off-by: Suseendran Bhaskaran <suseendranb@google.com>
> > ---
> >  crypto/lz4.c     | 2 +-
> >  crypto/lz4hc.c   | 2 +-
> >  crypto/lzo-rle.c | 2 +-
> >  crypto/lzo.c     | 2 +-
> >  crypto/zstd.c    | 5 ++++-
> >  5 files changed, 8 insertions(+), 5 deletions(-)
>
> I can take this patch through the cryptodev tree.  Is that OK with
> you Yosry?

Thanks Herbert. I think we should wait to see how patch #2 turns out.
If I understand correctly, as of now patch #2 keys off -ENOSPC to make
a behavioral change in zswap, so the zswap patch would need to land
*after* this patch. Unless you want to fast track this to Linus right
away?

Re: [PATCH 1/2] crypto: return -ENOSPC on compression destination overflow

[Yosry Ahmed]

On Thu, Oct 22, 2026 at 8:34 AM Yosry Ahmed <yosry@kernel.org> wrote:
>
> On Thu, Oct 22, 2026 at 1:14 AM Herbert Xu <herbert@gondor.apana.org.au> wrote:
> >
> > On Wed, Oct 21, 2026 at 08:56:25PM +0000, Suseendran Bhaskaran wrote:
> > > In the acomp/scomp compression interface, callers pass a bounded
> > > destination buffer and rely on the return code to distinguish a
> > > destination buffer overflow (incompressible input) from genuine
> > > compressor failures (such as invalid parameters, uninitialized state, or
> > > hardware errors).
> > >
> > > While the multi-segment streaming path in zstd_compress(),
> > > crypto/deflate.c, and lib/842/842_compress.c returns -ENOSPC when the
> > > destination buffer runs out of space, the single-buffer path
> > > zstd_compress_one() (taken when input and output are contiguous single
> > > pages, such as in zswap) as well as lz4, lz4hc, lzo, and lzo-rle return
> > > -EINVAL instead of -ENOSPC on destination buffer overflow.
> > >
> > > For users such as zswap, which reduced its per-CPU compression buffer to
> > > a single page in commit 0b1bf60c324a ("mm/zswap: reduce the size of the
> > > compression buffer to a single page"), incompressible pages typically
> > > overflow the PAGE_SIZE destination buffer due to compressor header/token
> > > overhead. Because these compressors return -EINVAL rather than -ENOSPC,
> > > zswap cannot distinguish a destination buffer overflow from a compressor
> > > failure, causing broken debugfs error accounting (reject_compress_fail
> > > vs. reject_compress_poor) or masking real compressor errors.
> > >
> > > Update zstd_compress_one(), lz4, lz4hc, lzo, and lzo-rle to return
> > > -ENOSPC when the compressed output overflows the destination buffer,
> > > matching the streaming path in zstd_compress(), deflate, and 842.
> > >
> > > Fixes: f5ad93ffb541 ("crypto: zstd - convert to acomp")
> > > Signed-off-by: Suseendran Bhaskaran <suseendranb@google.com>
> > > ---
> > >  crypto/lz4.c     | 2 +-
> > >  crypto/lz4hc.c   | 2 +-
> > >  crypto/lzo-rle.c | 2 +-
> > >  crypto/lzo.c     | 2 +-
> > >  crypto/zstd.c    | 5 ++++-
> > >  5 files changed, 8 insertions(+), 5 deletions(-)
> >
> > I can take this patch through the cryptodev tree.  Is that OK with
> > you Yosry?
>
> Thanks Herbert. I think we should wait to see how patch #2 turns out.
> If I understand correctly, as of now patch #2 keys off -ENOSPC to make
> a behavioral change in zswap, so the zswap patch would need to land
> *after* this patch. Unless you want to fast track this to Linus right
> away?

Patch 2 will be re-sent separately, so please feel free to take this
patch as-is. Thanks!

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

[Suseendran Bhaskaran]

Commit dca4437a5861 ("mm/zswap: store <PAGE_SIZE compression failed page
as-is") and commit 0b1bf60c324a ("mm/zswap: reduce the size of the
compression buffer to a single page") updated zswap_compress() to pass a
PAGE_SIZE destination buffer to crypto_acomp_compress() (so
incompressible pages typically overflow the PAGE_SIZE buffer due to
compressor metadata overhead) and check:

    if (comp_ret || !dlen || dlen >= PAGE_SIZE)

Because crypto_acomp_compress() previously returned -EINVAL on
destination buffer overflow, this check conflated destination buffer
overflow (incompressible page) with unexpected compressor failures and
did not set comp_ret = -ENOSPC when rejecting incompressible pages,
resulting in two user-facing issues:

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).

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.

Update zswap_compress() to treat only comp_ret == -ENOSPC or
(!comp_ret && dlen >= PAGE_SIZE) as an incompressible page (storing it
uncompressed in zswap when writeback is enabled to preserve LRU order,
or rejecting it with comp_ret = -ENOSPC when writeback is disabled so
zswap_reject_compress_poor is incremented), while jumping to unlock on
unexpected compressor errors (comp_ret || !dlen) so
zswap_reject_compress_fail is properly incremented.

Fixes: dca4437a5861 ("mm/zswap: store <PAGE_SIZE compression failed page as-is")
Signed-off-by: Suseendran Bhaskaran <suseendranb@google.com>
---
 mm/zswap.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/mm/zswap.c b/mm/zswap.c
index 37f34e406c8e..5f2a941fcaf1 100644
--- a/mm/zswap.c
+++ b/mm/zswap.c
@@ -881,12 +881,12 @@ static bool zswap_compress(struct page *page, struct zswap_entry *entry,
 	 * only adds metadata overhead.  swap_writeout() will put the page back
 	 * to the active LRU list in the case.
 	 */
-	if (comp_ret || !dlen || dlen >= PAGE_SIZE) {
+	if (comp_ret == -ENOSPC || (!comp_ret && dlen >= PAGE_SIZE)) {
 		rcu_read_lock();
 		if (!mem_cgroup_zswap_writeback_enabled(
 					folio_memcg(page_folio(page)))) {
 			rcu_read_unlock();
-			comp_ret = comp_ret ? comp_ret : -EINVAL;
+			comp_ret = -ENOSPC;
 			goto unlock;
 		}
 		rcu_read_unlock();
@@ -894,6 +894,9 @@ static bool zswap_compress(struct page *page, struct zswap_entry *entry,
 		dlen = PAGE_SIZE;
 		dst = kmap_local_page(page);
 		mapped = true;
+	} else if (comp_ret || !dlen) {
+		comp_ret = comp_ret ? comp_ret : -EINVAL;
+		goto unlock;
 	}
 
 	gfp = GFP_NOWAIT | __GFP_NORETRY | __GFP_HIGHMEM | __GFP_MOVABLE;
-- 
2.56.0.218.g1ec904b173-goog

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

[Yosry Ahmed]

On Wed, Oct 21, 2026 at 1:57 PM Suseendran Bhaskaran
<suseendranb@google.com> wrote:
>
> Commit dca4437a5861 ("mm/zswap: store <PAGE_SIZE compression failed page
> as-is") and commit 0b1bf60c324a ("mm/zswap: reduce the size of the
> compression buffer to a single page") updated zswap_compress() to pass a
> PAGE_SIZE destination buffer to crypto_acomp_compress() (so
> incompressible pages typically overflow the PAGE_SIZE buffer due to
> compressor metadata overhead) and check:
>
>     if (comp_ret || !dlen || dlen >= PAGE_SIZE)
>
> Because crypto_acomp_compress() previously returned -EINVAL on
> destination buffer overflow, this check conflated destination buffer
> overflow (incompressible page) with unexpected compressor failures and
> did not set comp_ret = -ENOSPC when rejecting incompressible pages,
> resulting in two user-facing issues:
>
> 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?

>
> 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?

>
> Update zswap_compress() to treat only comp_ret == -ENOSPC or
> (!comp_ret && dlen >= PAGE_SIZE) as an incompressible page (storing it
> uncompressed in zswap when writeback is enabled to preserve LRU order,
> or rejecting it with comp_ret = -ENOSPC when writeback is disabled so
> zswap_reject_compress_poor is incremented), while jumping to unlock on
> unexpected compressor errors (comp_ret || !dlen) so
> zswap_reject_compress_fail is properly incremented.
>
> Fixes: dca4437a5861 ("mm/zswap: store <PAGE_SIZE compression failed page as-is")
> Signed-off-by: Suseendran Bhaskaran <suseendranb@google.com>
> ---
>  mm/zswap.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/mm/zswap.c b/mm/zswap.c
> index 37f34e406c8e..5f2a941fcaf1 100644
> --- a/mm/zswap.c
> +++ b/mm/zswap.c
> @@ -881,12 +881,12 @@ static bool zswap_compress(struct page *page, struct zswap_entry *entry,
>          * only adds metadata overhead.  swap_writeout() will put the page back
>          * to the active LRU list in the case.
>          */
> -       if (comp_ret || !dlen || dlen >= PAGE_SIZE) {
> +       if (comp_ret == -ENOSPC || (!comp_ret && dlen >= PAGE_SIZE)) {
>                 rcu_read_lock();
>                 if (!mem_cgroup_zswap_writeback_enabled(
>                                         folio_memcg(page_folio(page)))) {
>                         rcu_read_unlock();
> -                       comp_ret = comp_ret ? comp_ret : -EINVAL;
> +                       comp_ret = -ENOSPC;
>                         goto unlock;
>                 }
>                 rcu_read_unlock();
> @@ -894,6 +894,9 @@ static bool zswap_compress(struct page *page, struct zswap_entry *entry,
>                 dlen = PAGE_SIZE;
>                 dst = kmap_local_page(page);
>                 mapped = true;
> +       } else if (comp_ret || !dlen) {
> +               comp_ret = comp_ret ? comp_ret : -EINVAL;
> +               goto unlock;
>         }
>
>         gfp = GFP_NOWAIT | __GFP_NORETRY | __GFP_HIGHMEM | __GFP_MOVABLE;
> --
> 2.56.0.218.g1ec904b173-goog
>

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@google.com> 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?

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

[Yosry Ahmed]

On Thu, Oct 22, 2026 at 8:51 AM Suseendran Bhaskaran
<suseendranb@google.com> wrote:
>
> 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@google.com> 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:

Is this actually possible? When we changed the buffer size to
PAGE_SIZE, I thought we made sure all compressors wouldn't write
beyond the buffer size. dlen > PAGE_SIZE means buffer overflow, which
is a bigger problem than debugfs stats. Or maybe compressors can set
dlen > PAGE_SIZE but won't actually write to the buffer? I didn't
check.

As for the (!comp_ret && dlen == PAGE_SIZE) case specifically, yeah we
should probably use -ENOSPC to count it as poor compression.

The compt_ret and dlen checks are not super intuitive tbh, I wonder
how they should relate. What is the expected value of dlen if comp_ret
is an error? Is it always 0, or can the compressor set it as a
non-zero value if compression fails?

>
> -                       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?

Let's figure out what needs to be done for patch 2 first, if anything.

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

[Suseendran Bhaskaran]

On Thu, Oct 22, 2026 at 09:01:02AM -0700, Yosry Ahmed wrote:
> On Thu, Oct 22, 2026 at 8:51 AM Suseendran Bhaskaran
> <suseendranb@google.com> wrote:
[...]
> > 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:
>
> Is this actually possible? When we changed the buffer size to
> PAGE_SIZE, I thought we made sure all compressors wouldn't write
> beyond the buffer size. dlen > PAGE_SIZE means buffer overflow, which
> is a bigger problem than debugfs stats. Or maybe compressors can set
> dlen > PAGE_SIZE but won't actually write to the buffer? I didn't
> check.

dlen > PAGE_SIZE isn't possible, since all compressors bound their
output by the input req->dlen (PAGE_SIZE) as you mentioned, and only
set req->dlen <= PAGE_SIZE. The 'dlen >= PAGE_SIZE' check in commit
dca4437a5861 ("mm/zswap: store <PAGE_SIZE compression failed page
as-is") was written when the buffer was still 2 * PAGE_SIZE prior to
commit 0b1bf60c324a ("mm/zswap: reduce the size of the compression
buffer to a single page").

> As for the (!comp_ret && dlen == PAGE_SIZE) case specifically, yeah we
> should probably use -ENOSPC to count it as poor compression.

Ack.

> The compt_ret and dlen checks are not super intuitive tbh, I wonder
> how they should relate. What is the expected value of dlen if comp_ret
> is an error? Is it always 0, or can the compressor set it as a
> non-zero value if compression fails?

On error (comp_ret != 0), dlen is not guaranteed to be 0. The value
left in dlen depends on the compressor used. For example:
- zstd, 842, and qat set dlen = 0 on error.
- lz4, lz4hc, lzo, lzo-rle (via scomp_acomp_comp_decomp()), deflate,
  and iaa return the error code without modifying dlen, so dlen
  remains at its input value (PAGE_SIZE).
- hisi_zip sets dlen to the hardware-reported produced bytes even
  on error.

So dlen is only meaningful when !comp_ret. The !dlen check was
added in commit dca4437a5861 ("mm/zswap: store <PAGE_SIZE
compression failed page as-is") as a defensive check for the case when
comp_ret is 0 with 0 dlen which shouldn't happen unless the compressor
is faulty.

> >
> > -                       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?
>
> Let's figure out what needs to be done for patch 2 first, if anything.

Since dlen is only valid when !comp_ret, we could normalize comp_ret
first when !comp_ret and then just check 'if (comp_ret)':

diff --git a/mm/zswap.c b/mm/zswap.c
--- a/mm/zswap.c
+++ b/mm/zswap.c
@@ -874,19 +874,24 @@ static bool zswap_compress(struct page *page, struct zswap_entry *entry,
 	 */
 	comp_ret = crypto_wait_req(crypto_acomp_compress(acomp_ctx->req), &acomp_ctx->wait);
 	dlen = acomp_ctx->req->dlen;
+	if (!comp_ret) {
+		if (dlen == PAGE_SIZE)
+			comp_ret = -ENOSPC;
+		else if (WARN_ON_ONCE(!dlen || dlen > PAGE_SIZE))
+			comp_ret = -EINVAL;
+	}
 
 	/*
 	 * If a page cannot be compressed into a size smaller than PAGE_SIZE,
 	 * save the content as is without a compression, to keep the LRU order
 	 * of writebacks.  If writeback is disabled, reject the page since it
 	 * only adds metadata overhead.  swap_writeout() will put the page back
 	 * to the active LRU list in the case.
 	 */
-	if (comp_ret || !dlen || dlen >= PAGE_SIZE) {
+	if (comp_ret) {
 		rcu_read_lock();
 		if (!mem_cgroup_zswap_writeback_enabled(
 					folio_memcg(page_folio(page)))) {
 			rcu_read_unlock();
-			comp_ret = comp_ret ? comp_ret : -EINVAL;
 			goto unlock;
 		}

Does this look good to you?

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

[Yosry Ahmed]

On Thu, Oct 22, 2026 at 11:33 AM Suseendran Bhaskaran
<suseendranb@google.com> wrote:
>
> On Thu, Oct 22, 2026 at 09:01:02AM -0700, Yosry Ahmed wrote:
> > On Thu, Oct 22, 2026 at 8:51 AM Suseendran Bhaskaran
> > <suseendranb@google.com> wrote:
> [...]
> > > 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:
> >
> > Is this actually possible? When we changed the buffer size to
> > PAGE_SIZE, I thought we made sure all compressors wouldn't write
> > beyond the buffer size. dlen > PAGE_SIZE means buffer overflow, which
> > is a bigger problem than debugfs stats. Or maybe compressors can set
> > dlen > PAGE_SIZE but won't actually write to the buffer? I didn't
> > check.
>
> dlen > PAGE_SIZE isn't possible, since all compressors bound their
> output by the input req->dlen (PAGE_SIZE) as you mentioned, and only
> set req->dlen <= PAGE_SIZE. The 'dlen >= PAGE_SIZE' check in commit
> dca4437a5861 ("mm/zswap: store <PAGE_SIZE compression failed page
> as-is") was written when the buffer was still 2 * PAGE_SIZE prior to
> commit 0b1bf60c324a ("mm/zswap: reduce the size of the compression
> buffer to a single page").
>
> > As for the (!comp_ret && dlen == PAGE_SIZE) case specifically, yeah we
> > should probably use -ENOSPC to count it as poor compression.
>
> Ack.
>
> > The compt_ret and dlen checks are not super intuitive tbh, I wonder
> > how they should relate. What is the expected value of dlen if comp_ret
> > is an error? Is it always 0, or can the compressor set it as a
> > non-zero value if compression fails?
>
> On error (comp_ret != 0), dlen is not guaranteed to be 0. The value
> left in dlen depends on the compressor used. For example:
> - zstd, 842, and qat set dlen = 0 on error.
> - lz4, lz4hc, lzo, lzo-rle (via scomp_acomp_comp_decomp()), deflate,
>   and iaa return the error code without modifying dlen, so dlen
>   remains at its input value (PAGE_SIZE).
> - hisi_zip sets dlen to the hardware-reported produced bytes even
>   on error.
>
> So dlen is only meaningful when !comp_ret. The !dlen check was
> added in commit dca4437a5861 ("mm/zswap: store <PAGE_SIZE
> compression failed page as-is") as a defensive check for the case when
> comp_ret is 0 with 0 dlen which shouldn't happen unless the compressor
> is faulty.
>
> > >
> > > -                       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?
> >
> > Let's figure out what needs to be done for patch 2 first, if anything.
>
> Since dlen is only valid when !comp_ret, we could normalize comp_ret
> first when !comp_ret and then just check 'if (comp_ret)':
>
> diff --git a/mm/zswap.c b/mm/zswap.c
> --- a/mm/zswap.c
> +++ b/mm/zswap.c
> @@ -874,19 +874,24 @@ static bool zswap_compress(struct page *page, struct zswap_entry *entry,
>          */
>         comp_ret = crypto_wait_req(crypto_acomp_compress(acomp_ctx->req), &acomp_ctx->wait);
>         dlen = acomp_ctx->req->dlen;
> +       if (!comp_ret) {
> +               if (dlen == PAGE_SIZE)
> +                       comp_ret = -ENOSPC;
> +               else if (WARN_ON_ONCE(!dlen || dlen > PAGE_SIZE))
> +                       comp_ret = -EINVAL;
> +       }
>
>         /*
>          * If a page cannot be compressed into a size smaller than PAGE_SIZE,
>          * save the content as is without a compression, to keep the LRU order
>          * of writebacks.  If writeback is disabled, reject the page since it
>          * only adds metadata overhead.  swap_writeout() will put the page back
>          * to the active LRU list in the case.
>          */
> -       if (comp_ret || !dlen || dlen >= PAGE_SIZE) {
> +       if (comp_ret) {
>                 rcu_read_lock();
>                 if (!mem_cgroup_zswap_writeback_enabled(
>                                         folio_memcg(page_folio(page)))) {
>                         rcu_read_unlock();
> -                       comp_ret = comp_ret ? comp_ret : -EINVAL;
>                         goto unlock;
>                 }
>
> Does this look good to you?

Yes. I assume this is completely independent from patch 1, so we can
tell Herbert to pick up patch 1 and send this separately?

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

[Suseendran Bhaskaran]

On Thu, Oct 22, 2026 at 12:15:51PM -0700, Yosry Ahmed wrote:
> On Thu, Oct 22, 2026 at 11:33 AM Suseendran Bhaskaran
> <suseendranb@google.com> wrote:
[...]
> > Since dlen is only valid when !comp_ret, we could normalize comp_ret
> > first when !comp_ret and then just check 'if (comp_ret)':
> >
> > diff --git a/mm/zswap.c b/mm/zswap.c
> > --- a/mm/zswap.c
> > +++ b/mm/zswap.c
> > @@ -874,19 +874,24 @@ static bool zswap_compress(struct page *page, struct zswap_entry *entry,
> >          */
> >         comp_ret = crypto_wait_req(crypto_acomp_compress(acomp_ctx->req), &acomp_ctx->wait);
> >         dlen = acomp_ctx->req->dlen;
> > +       if (!comp_ret) {
> > +               if (dlen == PAGE_SIZE)
> > +                       comp_ret = -ENOSPC;
> > +               else if (WARN_ON_ONCE(!dlen || dlen > PAGE_SIZE))
> > +                       comp_ret = -EINVAL;
> > +       }
> >
> >         /*
> >          * If a page cannot be compressed into a size smaller than PAGE_SIZE,
> >          * save the content as is without a compression, to keep the LRU order
> >          * of writebacks.  If writeback is disabled, reject the page since it
> >          * only adds metadata overhead.  swap_writeout() will put the page back
> >          * to the active LRU list in the case.
> >          */
> > -       if (comp_ret || !dlen || dlen >= PAGE_SIZE) {
> > +       if (comp_ret) {
> >                 rcu_read_lock();
> >                 if (!mem_cgroup_zswap_writeback_enabled(
> >                                         folio_memcg(page_folio(page)))) {
> >                         rcu_read_unlock();
> > -                       comp_ret = comp_ret ? comp_ret : -EINVAL;
> >                         goto unlock;
> >                 }
> >
> > Does this look good to you?
>
> Yes. I assume this is completely independent from patch 1, so we can
> tell Herbert to pick up patch 1 and send this separately?

Yes, this change is completely independent from patch 1. Herbert can
pick up patch 1 as-is.
I will address the latest Sashiko finding on patch 2 and send the updated
patch as a standalone change separately.