Re: [PATCH v4] mm/zswap: store <PAGE_SIZE compression failed page as-is

[Chris Li <chrisl@kernel.org>]

On Tue, Aug 19, 2025 at 6:13 PM Barry Song <21cnbao@gmail.com> wrote:
>
> [...]
> > +
> > +       /*
> > +        * 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) {
> > +               zswap_crypto_compress_fail++;
> > +               dlen = PAGE_SIZE;
> > +       }
>
> I’m not entirely sure about this. As long as we pass 2*PAGE_SIZE as
> dst_buf, any error returned by crypto drivers should indicate a bug in
> the driver that needs to be fixed.

That is what I have in mind for that counter, if that counter is not
zero it is something with the crypto has gone wrong. If we are sure
that it can never fail, maybe we shouldn't check the error return?

> There have also been attempts to unify crypto drivers so they consistently
> return -ENOSPC when the destination buffer would overflow. If that has
> been achieved, we might be able to reduce the buffer from 2*PAGE_SIZE to
> just PAGE_SIZE in zswap. There was a long discussion on this here:
> https://lore.kernel.org/linux-mm/Z7dnPh4tPxLO1UEo@google.com/
>
> I’m not sure of the current status — do all crypto drivers now return a
> consistent -ENOSPC when the compressed size exceeds PAGE_SIZE? From
> what I recall during that discussion, most drivers already behaved this
> way, but Sergey Senozhatsky pointed out one or two exceptions.
>
> Let’s sync with Herbert: have we reached the stage where all drivers
> reliably return -ENOSPC when dst_buf is PAGE_SIZE but the compressed
> size would exceed it?

I agree -ENOSPC should treat the compression ratio the same too low.
However, is the crypto library able to return any error other than
-ENOSPC? I am tempted to do something like BUG_ON() or WARN_ONCE() if
other errors we think are never possible. However even the BUG_ON and
WARN are discouraged in the kernel so we should handle the error. The
one error is we can log it and monitor it to make sure it never
happens.

If we are absolutely sure the other error should not happen. We should
remove the free form error from the interface to reflect that. Make
the function should just return bool success or failure as a buffer
too small.

Chris