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

[Usama Arif <usama.arif@linux.dev>]

On Mon, 12 Oct 2026 17:21:40 -0700 "Kanchana P. Sridhar" <kanchanapsridhar2026@gmail.com> wrote:

> On Fri, Oct 9, 2026 at 6:31 PM Usama Arif <usama.arif@linux.dev> 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, so group it with the compression request. Independent
> > requests can share the per-CPU transform; the codecs and drivers
> > synchronize their shared state internally. Loads can still wait for
> > each other on the decompression mutex, and stores still serialize on
> > the compression mutex.
> >
> > acomp_request_free() ignores NULL requests, so drop the check in
> > acomp_ctx_free().
> >
> > This follows the proposal from Sergey Senozhatsky for the same split
> > for zram [1].
> >
> > [1] https://lore.kernel.org/all/20261005122036.718976-10-senozhatsky@chromium.org/
> >
> > Signed-off-by: Usama Arif <usama.arif@linux.dev>
> 
> I have a question with regards to this patch. Is the intent to allow a
> decompress thread to run concurrently with a store thread for a given process?

The request is shared by tasks using the same CPU and zswap pool.
These may be different processes or threads of the same process.
Each operation still runs synchronously in its caller.

The motivating case can happen entirely on one CPU:
1. Task A compresses page X and releases the codec's stream lock.
2. A is preempted while still holding the zswap request mutex.
3. Task B runs and faults in an already stored page Y.

With the shared request, B blocks until A runs again and releases the
mutex. Separate requests let B load Y while A's store is unfinished.
The codec/driver still synchronizes its own shared state.

> If so, I wonder if incurring context switch/PTE/TLB shootdowns performance
> penalty is worth it?

The switch to B already occurs with the existing code. This patch adds
no worker threads, forced migration, PTE changes or TLB shootdowns.
It avoids B's subsequent wait on A's request mutex. The 1-vCPU
benchmark in the cover letter exercises this without CPU migration.

> 
> With the proposed change, will zswap_store() continue to run on the "old" CPU
> (i.e. the one to which the process got migrated), or is it a requirement that
> the "old" process is in sleep state? If so, should we codify/document this
> requirement?
> 

There is no requirement for A to sleep. A can remain runnable while
preempted. Its zswap_store() invocation resumes in the same task, on
whichever CPU the scheduler selects. If it migrates after selecting
the context, it retains that pointer and the corresponding mutex,
as the existing code does. The same task cannot execute on two CPUs
at once.

> On the new CPU, is the same process "runnable", and if so, how do we ensure that
> the "higher priority" decompress is handled while the zswap_store() prior to
> preemption is still not done?

Normal scheduler policy decides when B runs. Once it runs, the split
removes its dependency on A releasing the shared request mutex.
A load starting on another CPU already selects a different context
today; migration is unnecessary for the benefit described here.

Existing folio/swap-cache locking still orders accesses to the same
page. The example above concerns two independent pages.

Thanks for the review!
Usama

> 
> Thanks,
> Kanchana
>