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