mm/zswap: distinguish incompressible pages from compressor failures

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 <[email protected]>

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

Quoted 26 lines · 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.

Quoted 9 lines

Fixes f5ad93ffb541 ("crypto: zstd - convert to acomp")

Signed-off-by Suseendran Bhaskaran <[email protected]>

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

Quoted 28 lines · Herbert Xu

Quoted 26 lines · 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.

Quoted 12 lines

Quoted 9 lines

Fixes f5ad93ffb541 ("crypto: zstd - convert to acomp")

Signed-off-by Suseendran Bhaskaran <[email protected]>

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

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 <[email protected]>

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

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 <[email protected]>

---
 mm/zswap.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

mm/zswap.c +5 -3

index 37f34e406c8e..5f2a941fcaf1 100644
@@ -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

Quoted 9 lines · Suseendran Bhaskaran

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?

Quoted 7 lines

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?

Quoted 12 lines · Yosry Ahmed

Quoted 9 lines · Suseendran Bhaskaran

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

Quoted 1 line

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.

Quoted 8 lines

Quoted 7 lines

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.

Quoted 6 lines

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:

(quoted diff) +2 -1

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

On Thu, Oct 22, 2026 at 8:51 AM Suseendran Bhaskaran <[email protected]> wrote:

Quoted 3 lines

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?

Quoted 7 lines

(quoted diff) +2 -1

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

Quoted 2 lines · Yosry Ahmed

On Thu, Oct 22, 2026 at 8:51 AM Suseendran Bhaskaran <[email protected]> wrote:

[...]

Quoted 10 lines

Quoted 3 lines

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

Quoted 2 lines

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

Ack.

Quoted 4 lines

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.

Quoted 9 lines

Quoted 7 lines

(quoted diff) +2 -1

-                       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)':

mm/zswap.c +7 -2

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

On Thu, Oct 22, 2026 at 11:33 AM Suseendran Bhaskaran <[email protected]> wrote:

Quoted 34 lines

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

mm/zswap.c +7 -2

@@ -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;
                }

Quoted 1 line

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?