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

[Chris Li <chrisl@kernel.org>]

Acked-by: Chris Li <chrisl@kernel.org>

Chris

On Thu, Aug 28, 2025 at 9:39 AM SeongJae Park <sj@kernel.org> wrote:
>
> On Wed, 27 Aug 2025 14:16:37 -0700 Chris Li <chrisl@kernel.org> wrote:
>
> > On Wed, Aug 27, 2025 at 10:45 AM SeongJae Park <sj@kernel.org> wrote:
> [...]
> > Anyway, I just opened the editor to check again. Yes, if we goto the
> > read_done, the if condition using dlen can introduce a new incoming
> > edge that has len uninitialized value to be used. Yes, need to
> > initialize dlen = PAGE_SIZE or you initialize it at the memcpy of
> > page.
>
> Thank you for confirming.
>
> >
> > > I will post the fixup by tomorrow morning (Pacific time) unless I
> > > hear other opinions or find my mistakes on the above plan by tonight.
> >
> > You are too humble, that is the normal reviewing process. You can take
> > your time.
>
> No worry, I just wanted to give you an expectation of the time line :)
>
> Andrew, could you please pick the below attached cleanup patch as a fixup of
> the original patch?  Please feel free to let me know if you prefer posting a
> new version of the patch instead or any other approach.
>
>
> Thanks,
> SJ
>
> [...]
>
> ==== Attachment 0 (0001-mm-zswap-cleanup-incompressible-pages-handling-code.patch) ====
> From 867c3789fa80ac163427f1d7804bf2c8667684ca Mon Sep 17 00:00:00 2001
> From: SeongJae Park <sj@kernel.org>
> Date: Wed, 27 Aug 2025 13:18:38 -0700
> Subject: [PATCH] mm/zswap: cleanup incompressible pages handling code
>
> Following Chris Li's suggestions[1], make the code easier to read and
> manage.
>
> [1] https://lore.kernel.org/CACePvbWGPApYr7G29FzbmWzRw-BJE39WH7kUHSaHs+Lnw8=-qQ@mail.gmail.com
>
> Signed-off-by: SeongJae Park <sj@kernel.org>
> Acked-by: Chris Li <chrisl@kernel.org>
> ---
>  mm/zswap.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/mm/zswap.c b/mm/zswap.c
> index f865a930dc88..e5e1f5687f5e 100644
> --- a/mm/zswap.c
> +++ b/mm/zswap.c
> @@ -952,6 +952,7 @@ static bool zswap_compress(struct page *page, struct zswap_entry *entry,
>         struct zpool *zpool;
>         gfp_t gfp;
>         u8 *dst;
> +       bool mapped = false;
>
>         acomp_ctx = acomp_ctx_get_cpu_lock(pool);
>         dst = acomp_ctx->buffer;
> @@ -983,9 +984,8 @@ 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)
> +       if (comp_ret || !dlen || dlen >= PAGE_SIZE) {
>                 dlen = PAGE_SIZE;
> -       if (dlen >= PAGE_SIZE) {
>                 if (!mem_cgroup_zswap_writeback_enabled(
>                                         folio_memcg(page_folio(page)))) {
>                         comp_ret = comp_ret ? comp_ret : -EINVAL;
> @@ -994,6 +994,7 @@ static bool zswap_compress(struct page *page, struct zswap_entry *entry,
>                 comp_ret = 0;
>                 dlen = PAGE_SIZE;
>                 dst = kmap_local_page(page);
> +               mapped = true;
>         }
>
>         zpool = pool->zpool;
> @@ -1007,7 +1008,7 @@ static bool zswap_compress(struct page *page, struct zswap_entry *entry,
>         entry->length = dlen;
>
>  unlock:
> -       if (dst != acomp_ctx->buffer)
> +       if (mapped)
>                 kunmap_local(dst);
>         if (comp_ret == -ENOSPC || alloc_ret == -ENOSPC)
>                 zswap_reject_compress_poor++;
> @@ -1025,7 +1026,7 @@ static bool zswap_decompress(struct zswap_entry *entry, struct folio *folio)
>         struct zpool *zpool = entry->pool->zpool;
>         struct scatterlist input, output;
>         struct crypto_acomp_ctx *acomp_ctx;
> -       int decomp_ret, dlen;
> +       int decomp_ret = 0, dlen = PAGE_SIZE;
>         u8 *src, *obj;
>
>         acomp_ctx = acomp_ctx_get_cpu_lock(entry->pool);
> @@ -1034,9 +1035,7 @@ static bool zswap_decompress(struct zswap_entry *entry, struct folio *folio)
>         /* zswap entries of length PAGE_SIZE are not compressed. */
>         if (entry->length == PAGE_SIZE) {
>                 memcpy_to_folio(folio, 0, obj, entry->length);
> -               zpool_obj_read_end(zpool, entry->handle, obj);
> -               acomp_ctx_put_unlock(acomp_ctx);
> -               return true;
> +               goto read_done;
>         }
>
>         /*
> @@ -1059,6 +1058,7 @@ static bool zswap_decompress(struct zswap_entry *entry, struct folio *folio)
>         decomp_ret = crypto_wait_req(crypto_acomp_decompress(acomp_ctx->req), &acomp_ctx->wait);
>         dlen = acomp_ctx->req->dlen;
>
> +read_done:
>         zpool_obj_read_end(zpool, entry->handle, obj);
>         acomp_ctx_put_unlock(acomp_ctx);
>
> --
> 2.39.5
>
>