Re: [PATCH 1/2] mm: zswap: use separate compression and decompression requests

[Yosry Ahmed]

On Fri, Oct 9, 2026 at 8:07 AM Usama Arif <usama.arif@xxxxxxxxx> wrote:
>
>
>
> On 07/10/2026 23:01, Yosry Ahmed wrote:
> > On Mon, Oct 5, 2026 at 5:23 PM Usama Arif <usama.arif@xxxxxxxxx> wrote:
> >>
> >> Stores and loads serialize on the same per-CPU acomp request and mutex.
> >> A low-priority store can be preempted as soon as the compressor drops
> >> its stream lock, while it still holds the mutex. A higher-priority load
> >> on that CPU then waits until the store runs again, which can take a
> >> long time when other tasks are runnable.
> >>
> >> Give compression and decompression their own request, completion wait
> >> and mutex. Since commit e2c3b6b21c77f ("mm: zswap: use SG list
> >> decompression APIs from zsmalloc"), the per-CPU buffer is only used for
> >> compression. The two requests can share the per-CPU transform: no
> >> in-tree implementation modifies transform state while (de)compressing,
> >> and shared codec state has its own locking.
>
>
> Hello Yosry!
>
> Thanks for the reviews!
>
> >
> > I am a bit uncomfortable with this. If future changes modify the
> > transform state while (de)compressing, it may result in nasty bugs.
>
> Independent acomp requests can share a transform. UBIFS already uses its
> shared compr->cc for compression and decompression without caller
> serialization. IPComp also submits per-packet requests on a shared
> transform, and EROFS allows concurrent decompression on one transform.
>
> Drivers synchronize their shared state internally. For example, HiSilicon
> protects its transform-owned request bitmap with req_lock. Unsynchronized
> shared state would break those existing callers too. Separate compression
> and decompression transforms would still leave concurrent stack
> decompressions sharing a transform.

I guess others relying on this makes me feel better about it. It would
be nice if we have protection against it. Not asking you to do this,
but constifying the transform everywhere after it's initialized is one
way to solidify this.

>
>
> >
> > As for the buffer, I would also prefer some protection, but I feel
> > less strongly about this. For example, we can put it inside
> > zswap_acomp_req and not initialize it for the decompression request.
> > Alternatively, we can have an intermediary struct that contains
> > zswap_acomp_req + buffer, and use that for the compression request.
>
>
> Done for next revision. Added zswap_comp_ctx containing the compression
> request and output buffer. Allocation, use and cleanup now go through that
> context. The compression mutex protects the buffer until zs_obj_write()
> finishes, and decompression has no buffer member.

Thanks!