Re: [PATCH v14 26/26] mm: zswap: Batched zswap_compress() for compress batching of large folios.

[Yosry Ahmed]

On Mon, Jul 27, 2026 at 8:05 PM He Jixin <1270243756@xxxxxx> wrote:
>
> On Sat, 24 Jan 2026 19:35:37 -0800, Kanchana P Sridhar wrote:
> > +                        if (dlen < 0) {
> > +                                dlen = PAGE_SIZE;
> > +                                dst = kmap_local_page(folio_page(folio,
> > +                                                      folio_start + batch_iter));
> > +                        }
> > +
> > +                        handle = zs_malloc(zs_pool, dlen, gfp, nid);
> > +
> > +                        if (unlikely(IS_ERR_VALUE(handle))) {
> > +                                if (PTR_ERR((void *)handle) == -ENOSPC)
> > +                                        zswap_reject_compress_poor++;
> > +                                else
> > +                                        zswap_reject_alloc_fail++;
> > +
> > +                                goto err_unlock;
> > +                        }
> > ...
> > +err_unlock:
> >          mutex_unlock(&acomp_ctx->mutex);
> > -        return comp_ret == 0 && alloc_ret == 0;
> > +        return false;
>
> I think there might be a resource leak when both conditions below happen:
>
> 1. `dlen < 0` enters the first if block and does `kmap_local_page()` into `dst`
> 2. Then `zs_malloc()` fails, hitting the `IS_ERR_VALUE(handle)` path and jumping to `err_unlock`
>
> In this path, `dst` points to a kmapped page that does not appear to get `kunmap_local()` before `err_unlock`.
> The old code had a `mapped` flag and a common unlock path, so kunmap_local() was called on all exits after kmap_local_page().
> After the batching rewrite, the successful path still unmaps `dst`, but the zs_malloc() failure path appears to bypass that cleanup.
>
> If my understanding is correct, the fix could be either:
>
> - Option A: Add `kunmap_local(dst)` right before `goto err_unlock` when `dlen < 0` was taken.
> - Option B: Restructure so that `kmap_local_page()` happens after `zs_malloc()` succeeds, only for the compression-failed entries.
>
> I'm new to the mm subsystem, so please let me know if I missed something that makes this safe.

I think we are indeed missing kunmap_local() in that cleanup path.
Unfortunately, there hasn't been much progress on this patch series
recently, I don't think we'll see a new version soon :/