mm/zswap: distinguish incompressible pages from compressor failures
Subsystems (4)
cover mm/zswap: distinguish incompressible pages from compressor failures
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(-)
Signature · 1 line
2.56.0.218.g1ec904b173-goog
PATCH 1/2 crypto: return -ENOSPC on compression destination overflow
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.
--- 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(-)
crypto/lz4.c +1 -1
@@ -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 ;
crypto/lz4hc.c +1 -1
@@ -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 ;
crypto/lzo-rle.c +1 -1
@@ -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 ;
crypto/lzo.c +1 -1
@@ -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 ;
crypto/zstd.c +4 -1
@@ -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;
Signature · 1 line
2.56.0.218.g1ec904b173-goog
PATCH 2/2 mm/zswap: classify compression dst overflow as poor compression
In reply to Suseendran BhaskaranCommit 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.
--- mm/zswap.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
mm/zswap.c +5 -2
@@ -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;
Signature · 1 line
2.56.0.218.g1ec904b173-goog
Yosry Ahmed
maintainerRe: [PATCH 2/2] mm/zswap: classify compression dst overflow as poor compression
Quoted 16 lines · Suseendran Bhaskaran
Commit
dca4437a5861("mm/zswap: store <PAGE_SIZEcompression failed page as-is") and commit0b1bf60c324a("mm/zswap: reduce the size of the compression buffer to a single page") updatedzswap_compress()to pass aPAGE_SIZEdestination buffer tocrypto_acomp_compress()(so incompressible pages typically overflow thePAGE_SIZEbuffer due to compressor metadata overhead) and check:if (comp_ret || !dlen || dlen >= PAGE_SIZE)Because
crypto_acomp_compress()previously returned-EINVALon destination buffer overflow, this check conflated destination buffer overflow (incompressible page) with unexpected compressor failures and did not setcomp_ret=-ENOSPCwhen rejecting incompressible pages, resulting in two user-facing issues:
Quoted 8 lines
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-EINVALor succeeds with dlen ==PAGE_SIZE,comp_retbecomes-EINVAL. Consequently, incompressible pages are misaccounted in debugfs underreject_compress_failinstead ofreject_compress_poor(leavingreject_compress_poorstuck 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 8 lines
2. When zswap writeback is enabled, any non-zero
comp_ret(such as-EINVALfrom an invalid compressor context,-ENOMEM, or a hardware accelerator error) is treated identically to an incompressible page:
comp_retis cleared to 0 and the raw page is stored uncompressed in zsmalloc. This silently masks genuine compressor failures (reject_compress_failis never incremented) and miscounts pages that encountered a compressor error understored_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 47 lines
Update
zswap_compress()to treat onlycomp_ret==-ENOSPCor (!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 withcomp_ret=-ENOSPCwhen writeback is disabled sozswap_reject_compress_pooris incremented), while jumping to unlock on unexpected compressor errors (comp_ret|| !dlen) sozswap_reject_compress_failis 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@@ -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
Herbert Xu
maintainerRe: [PATCH 1/2] crypto: return -ENOSPC on compression destination overflow
In reply to Suseendran BhaskaranQuoted 28 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, andlib/842/842_compress.creturns-ENOSPCwhen the destination buffer runs out of space, the single-buffer pathzstd_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-EINVALinstead of-ENOSPCon 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 thePAGE_SIZEdestination buffer due to compressor header/token overhead. Because these compressors return-EINVALrather than-ENOSPC, zswap cannot distinguish a destination buffer overflow from a compressor failure, causing broken debugfs error accounting (reject_compress_failvs.reject_compress_poor) or masking real compressor errors.Update
zstd_compress_one(), lz4, lz4hc, lzo, and lzo-rle to return-ENOSPCwhen the compressed output overflows the destination buffer, matching the streaming path inzstd_compress(), deflate, and 842.Fixes f5ad93ffb541 ("crypto: zstd - convert to acomp")Signed-off-by Suseendran Bhaskaran <[email protected]>
Quoted 7 lines
--- 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,
Signature · 3 lines
Email: Herbert Xu <[email protected]> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Yosry Ahmed
maintainerRe: [PATCH 1/2] crypto: return -ENOSPC on compression destination overflow
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, andlib/842/842_compress.creturns-ENOSPCwhen the destination buffer runs out of space, the single-buffer pathzstd_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-EINVALinstead of-ENOSPCon 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 thePAGE_SIZEdestination buffer due to compressor header/token overhead. Because these compressors return-EINVALrather than-ENOSPC, zswap cannot distinguish a destination buffer overflow from a compressor failure, causing broken debugfs error accounting (reject_compress_failvs.reject_compress_poor) or masking real compressor errors.Update
zstd_compress_one(), lz4, lz4hc, lzo, and lzo-rle to return-ENOSPCwhen the compressed output overflows the destination buffer, matching the streaming path inzstd_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?
Re: [PATCH 2/2] mm/zswap: classify compression dst overflow as poor compression
In reply to Yosry AhmedQuoted 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-EINVALor succeeds with dlen ==PAGE_SIZE,comp_retbecomes-EINVAL. Consequently, incompressible pages are misaccounted in debugfs underreject_compress_failinstead ofreject_compress_poor(leavingreject_compress_poorstuck 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-EINVALfrom an invalid compressor context,-ENOMEM, or a hardware accelerator error) is treated identically to an incompressible page:
comp_retis cleared to 0 and the raw page is stored uncompressed in zsmalloc. This silently masks genuine compressor failures (reject_compress_failis never incremented) and miscounts pages that encountered a compressor error understored_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_SIZEcompression 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-ENOSPCvs 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?
Yosry Ahmed
maintainerRe: [PATCH 2/2] mm/zswap: classify compression dst overflow as poor compression
Quoted 38 lines · Suseendran Bhaskaran
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-EINVALor succeeds with dlen ==PAGE_SIZE,comp_retbecomes-EINVAL. Consequently, incompressible pages are misaccounted in debugfs underreject_compress_failinstead ofreject_compress_poor(leavingreject_compress_poorstuck 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_retis already-ENOSPCand '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' setscomp_retto-EINVALinstead of-ENOSPC.Quoted 8 lines
Quoted 7 lines
2. When zswap writeback is enabled, any non-zero
comp_ret(such as-EINVALfrom an invalid compressor context,-ENOMEM, or a hardware accelerator error) is treated identically to an incompressible page:
comp_retis cleared to 0 and the raw page is stored uncompressed in zsmalloc. This silently masks genuine compressor failures (reject_compress_failis never incremented) and miscounts pages that encountered a compressor error understored_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_SIZEcompression 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-ENOSPCvs other error codes? Or am I missing something?
Quoted 7 lines
Thank you for the context. Keeping the writeback-enabled behavior from commit
dca4437a5861("mm/zswap: store <PAGE_SIZEcompression failed page as-is") as-is makes sense.With that unchanged, existing
zswap_compress()already accounts-ENOSPCvs other error codes when writeback is disabled, except for the !comp_ret && dlen >=PAGE_SIZEcase 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.
Re: [PATCH 2/2] mm/zswap: classify compression dst overflow as poor compression
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-ENOSPCvs other error codes when writeback is disabled, except for the !comp_ret && dlen >=PAGE_SIZEcase 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_SIZEmeans buffer overflow, which is a bigger problem than debugfs stats. Or maybe compressors can set dlen >PAGE_SIZEbut 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-ENOSPCto count it as poor compression.
Ack.
Quoted 4 lines
The
compt_retand dlen checks are not super intuitive tbh, I wonder how they should relate. What is the expected value of dlen ifcomp_retis 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?
Yosry Ahmed
maintainerRe: [PATCH 2/2] mm/zswap: classify compression dst overflow as poor compression
Quoted 93 lines · Suseendran Bhaskaran
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-ENOSPCvs other error codes when writeback is disabled, except for the !comp_ret && dlen >=PAGE_SIZEcase 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_SIZEmeans buffer overflow, which is a bigger problem than debugfs stats. Or maybe compressors can set dlen >PAGE_SIZEbut won't actually write to the buffer? I didn't check.dlen >
PAGE_SIZEisn't possible, since all compressors bound their output by the inputreq->dlen(PAGE_SIZE) as you mentioned, and only setreq->dlen<=PAGE_SIZE. The 'dlen >=PAGE_SIZE' check in commitdca4437a5861("mm/zswap: store <PAGE_SIZEcompression failed page as-is") was written when the buffer was still 2 *PAGE_SIZEprior to commit0b1bf60c324a("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-ENOSPCto count it as poor compression.Ack.
Quoted 4 lines
The
compt_retand dlen checks are not super intuitive tbh, I wonder how they should relate. What is the expected value of dlen ifcomp_retis 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 (viascomp_acomp_comp_decomp()), deflate, and iaa return the error code without modifying dlen, so dlen remains at its input value (PAGE_SIZE).
-hisi_zipsets 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_SIZEcompression failed page as-is") as a defensive check for the case whencomp_retis 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_retfirst 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?
[email protected]
AI reviewRe: [PATCH 2/2] mm/zswap: classify compression dst overflow as poor compression
In reply to Suseendran BhaskaranThank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] mm/zswap: zswap_compress() mishandles -E2BIG overflow from hw compressors --
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] mm/zswap: zswap_compress() mishandles -E2BIG overflow from hw compressors --
commit 6e958455f7e6f806b4bd7b7afc39fe7c9fb8cd1f
Author: Suseendran Bhaskaran <[email protected]>
mm/zswap: classify compression dst overflow as poor compression
This commit updates zswap_compress() to treat only -ENOSPC or an output length of at least PAGE_SIZE as an incompressible page, while routing other non-zero return values from crypto_acomp_compress() to the compressor failure path.
Quoted 20 lines
mm/zswap.c +2
-2@@ -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 ();
mm/zswap.c static bool zswap_compress(struct page *page, struct zswap_entry *entry, · line 902
Quoted 9 lines
mm/zswap.c +3
-0@@ -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;
[Severity: Medium]
Does restricting the incompressible page check to comp_ret == -ENOSPC cause a regression when using hardware acomp drivers that return -E2BIG or -EOVERFLOW on destination buffer overflow?
When zswap_store() -> zswap_store_page() -> zswap_compress() compresses an incompressible page into a PAGE_SIZE destination buffer, hardware acomp drivers in drivers/crypto/ do not return -ENOSPC when the compressed output exceeds the destination buffer:
drivers/crypto/intel/iaa/iaa_crypto_main.c:check_completion() {
if (comp->status == IAA_ANALYTICS_ERROR &&
comp->error_code == IAA_ERROR_COMP_BUF_OVERFLOW && compress) {
ret = -E2BIG;
dev_dbg(dev, "compressed > uncompressed size,"
" not compressing, size=0x%x\n",
comp->output_size);
update_completion_comp_buf_overflow_errs();
goto out;
}
}Similarly, the QAT acomp driver (which registers deflate and zstd with priority 4001) completes destination buffer overflows with -E2BIG in qat_comp_generic_callback() (and -E2BIG or -EOVERFLOW in qat_comp_lz4s_zstd_callback()):
drivers/crypto/intel/qat/qat_common/qat_comp_algs.c:qat_comp_generic_callback() {
if (unlikely(produced > qat_req->actual_dlen)) {
memset(inst->dc_data->ovf_buff, 0,
inst->dc_data->ovf_buff_sz);
dev_dbg(&GET_DEV(accel_dev),
"Actual buffer overflow: produced=%d, dlen=%d\n",
produced, qat_req->actual_dlen);
res = -E2BIG;
areq->dlen = 0;
goto end;
}
}With this change, -E2BIG and -EOVERFLOW fall into the else if (comp_ret || !dlen) branch and jump to unlock. When zswap writeback is enabled, incompressible pages will be rejected instead of being stored uncompressed in zsmalloc, and zswap_reject_compress_fail will be incremented instead of zswap_reject_compress_poor.
Should those hardware acomp drivers be updated to return -ENOSPC on destination overflow, or should zswap_compress() also handle -E2BIG and -EOVERFLOW as poor compression?
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
Re: [PATCH 2/2] mm/zswap: classify compression dst overflow as poor compression
In reply to Yosry AhmedQuoted 2 lines · Yosry Ahmed
On Thu, Oct 22, 2026 at 11:33 AM Suseendran Bhaskaran <[email protected]> wrote:
[...]
Quoted 34 lines
Quoted 34 lines
Since dlen is only valid when !comp_ret, we could normalize
comp_retfirst 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 4 lines
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?
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.
Yosry Ahmed
maintainerRe: [PATCH 1/2] crypto: return -ENOSPC on compression destination overflow
In reply to Yosry AhmedQuoted 43 lines · Yosry Ahmed
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, andlib/842/842_compress.creturns-ENOSPCwhen the destination buffer runs out of space, the single-buffer pathzstd_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-EINVALinstead of-ENOSPCon 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 thePAGE_SIZEdestination buffer due to compressor header/token overhead. Because these compressors return-EINVALrather than-ENOSPC, zswap cannot distinguish a destination buffer overflow from a compressor failure, causing broken debugfs error accounting (reject_compress_failvs.reject_compress_poor) or masking real compressor errors.Update
zstd_compress_one(), lz4, lz4hc, lzo, and lzo-rle to return-ENOSPCwhen the compressed output overflows the destination buffer, matching the streaming path inzstd_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?
Quoted 5 lines
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
-ENOSPCto 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!
Herbert Xu
maintainerRe: [PATCH 1/2] crypto: return -ENOSPC on compression destination overflow
In reply to Suseendran BhaskaranQuoted 28 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, andlib/842/842_compress.creturns-ENOSPCwhen the destination buffer runs out of space, the single-buffer pathzstd_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-EINVALinstead of-ENOSPCon 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 thePAGE_SIZEdestination buffer due to compressor header/token overhead. Because these compressors return-EINVALrather than-ENOSPC, zswap cannot distinguish a destination buffer overflow from a compressor failure, causing broken debugfs error accounting (reject_compress_failvs.reject_compress_poor) or masking real compressor errors.Update
zstd_compress_one(), lz4, lz4hc, lzo, and lzo-rle to return-ENOSPCwhen the compressed output overflows the destination buffer, matching the streaming path inzstd_compress(), deflate, and 842.Fixes f5ad93ffb541 ("crypto: zstd - convert to acomp")Signed-off-by Suseendran Bhaskaran <[email protected]>
Quoted 7 lines
--- 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(-)
Patch applied. Thanks.
Signature · 3 lines
Email: Herbert Xu <[email protected]> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt