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

[Suseendran Bhaskaran <suseendranb@google.com>]

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?