[PATCH 0/2] mm: zswap: reduce request contention on loads

[PATCH 0/2] mm: zswap: reduce request contention on loads

[Usama Arif]

Stores and loads share a per-CPU acomp request and mutex. A low-priority
store can be preempted right after the compressor drops its stream
lock, while it still holds the zswap mutex, and a higher-priority load
on that CPU then waits for the store to run again. This follows the work
from Sergey Senozhatsky's zram series which splits it for the same
reason [1].

Patch 1 gives compression and decompression separate requests, waits
and mutexes, so loads no longer wait for stores, though they can still
wait for each other. Patch 2 decompresses with an on-stack request when
the algorithm is synchronous and needs no request context, which covers
all in-tree software compressors, so those loads take no zswap lock.
Asynchronous algorithms keep the per-CPU request and mutex. For software
compressors the series allocates the same number of requests as before;
each per-CPU context grows by 72 bytes, and the load path is about 270
bytes deeper on x86-64.

The series does not fix two related cases:
- Stores still serialize on the compression mutex, so a high-priority
  task that reclaims (direct reclaim, MADV_PAGEOUT) can still wait for
  a preempted store.
- On PREEMPT_RT the codec stream locks are preemptible, so a load can
  still wait for a preempted store inside the codec.

The numbers below are the slowest read per run, as a median (min-max)
of 5 runs. Each run is 12 seconds in a zstd VM with lazy preemption,
vm.page-cluster=0 and swap on /dev/ram0. With 1 vCPU, four nice +10
workers page memory out and read it back while a nice 0 task spins. A
nice -19 reader pages out its own buffer and measures how long each
read of it takes. With 8 vCPUs there are 16 workers, 8 spinning tasks
and 8 readers.

              Before series (ms)      With series (ms)
  1 vCPU      22.3 (21.6-22.6)        0.97 (0.72-1.4)
  8 vCPUs     314 (97-2542)           7.0 (5.0-98)

Reads over 10 ms fell from 26-35 per run to none with 1 vCPU, and from
3-18 per run to at most one with 8 vCPUs. The benchmark and test programs
were written with the help of an LLM.

[1] https://lore.kernel.org/all/20261005122036.718976-10-senozhatsky@chromium.org/
 
Usama Arif (2):
  mm: zswap: use separate compression and decompression requests
  mm: zswap: use stack requests for synchronous decompression

 mm/zswap.c | 136 ++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 88 insertions(+), 48 deletions(-)

-- 
2.53.0-Meta

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

[Usama Arif]

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. Loads can still wait for
each other on the decompression mutex, and stores still serialize on
the compression mutex.

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>
---
 mm/zswap.c | 89 +++++++++++++++++++++++++++++++-----------------------
 1 file changed, 52 insertions(+), 37 deletions(-)

diff --git a/mm/zswap.c b/mm/zswap.c
index ae19e301fced7..54187b1ef751d 100644
--- a/mm/zswap.c
+++ b/mm/zswap.c
@@ -137,14 +137,20 @@ bool zswap_never_enabled(void)
 * data structures
 **********************************/
 
-struct crypto_acomp_ctx {
-	struct crypto_acomp *acomp;
+struct zswap_acomp_req {
 	struct acomp_req *req;
 	struct crypto_wait wait;
-	u8 *buffer;
 	struct mutex mutex;
 };
 
+/* Separate requests, so that decompression does not wait for compression. */
+struct crypto_acomp_ctx {
+	struct crypto_acomp *acomp;
+	struct zswap_acomp_req comp;
+	struct zswap_acomp_req decomp;
+	u8 *buffer;
+};
+
 /*
  * The lock ordering is zswap_tree.lock -> zswap_pool.lru_lock.
  * The only case where lru_lock is not acquired while holding tree.lock is
@@ -270,14 +276,10 @@ static void acomp_ctx_free(struct crypto_acomp_ctx *acomp_ctx)
 	if (!acomp_ctx)
 		return;
 
-	/*
-	 * If there was an error in allocating @acomp_ctx->req, it
-	 * would be set to NULL.
-	 */
-	if (acomp_ctx->req)
-		acomp_request_free(acomp_ctx->req);
-
-	acomp_ctx->req = NULL;
+	acomp_request_free(acomp_ctx->comp.req);
+	acomp_ctx->comp.req = NULL;
+	acomp_request_free(acomp_ctx->decomp.req);
+	acomp_ctx->decomp.req = NULL;
 
 	/*
 	 * We have to handle both cases here: an error pointer return from
@@ -796,6 +798,28 @@ static void zswap_entry_free(struct zswap_entry *entry)
 /*********************************
 * compressed storage functions
 **********************************/
+static int zswap_acomp_req_init(struct zswap_acomp_req *areq,
+				struct crypto_acomp *acomp)
+{
+	/* acomp_request_alloc() returns NULL in case of an error. */
+	areq->req = acomp_request_alloc(acomp);
+	if (!areq->req)
+		return -ENOMEM;
+
+	crypto_init_wait(&areq->wait);
+
+	/*
+	 * if the backend of acomp is async zip, crypto_req_done() will wakeup
+	 * crypto_wait_req(); if the backend of acomp is scomp, the callback
+	 * won't be called, crypto_wait_req() will return without blocking.
+	 */
+	acomp_request_set_callback(areq->req, CRYPTO_TFM_REQ_MAY_BACKLOG,
+				   crypto_req_done, &areq->wait);
+
+	mutex_init(&areq->mutex);
+	return 0;
+}
+
 static int zswap_cpu_comp_prepare(unsigned int cpu, struct hlist_node *node)
 {
 	struct zswap_pool *pool = hlist_entry(node, struct zswap_pool, node);
@@ -827,25 +851,13 @@ static int zswap_cpu_comp_prepare(unsigned int cpu, struct hlist_node *node)
 		goto fail;
 	}
 
-	/* acomp_request_alloc() returns NULL in case of an error. */
-	acomp_ctx->req = acomp_request_alloc(acomp_ctx->acomp);
-	if (!acomp_ctx->req) {
+	if (zswap_acomp_req_init(&acomp_ctx->comp, acomp_ctx->acomp) ||
+	    zswap_acomp_req_init(&acomp_ctx->decomp, acomp_ctx->acomp)) {
 		pr_err("could not alloc crypto acomp_request %s\n",
 		       pool->tfm_name);
 		goto fail;
 	}
 
-	crypto_init_wait(&acomp_ctx->wait);
-
-	/*
-	 * if the backend of acomp is async zip, crypto_req_done() will wakeup
-	 * crypto_wait_req(); if the backend of acomp is scomp, the callback
-	 * won't be called, crypto_wait_req() will return without blocking.
-	 */
-	acomp_request_set_callback(acomp_ctx->req, CRYPTO_TFM_REQ_MAY_BACKLOG,
-				   crypto_req_done, &acomp_ctx->wait);
-
-	mutex_init(&acomp_ctx->mutex);
 	return 0;
 
 fail:
@@ -866,14 +878,15 @@ static bool zswap_compress(struct folio *folio, long index,
 	bool mapped = false;
 
 	acomp_ctx = raw_cpu_ptr(pool->acomp_ctx);
-	mutex_lock(&acomp_ctx->mutex);
+	mutex_lock(&acomp_ctx->comp.mutex);
 
 	dst = acomp_ctx->buffer;
 	sg_init_table(&input, 1);
 	sg_set_folio(&input, folio, PAGE_SIZE, index * PAGE_SIZE);
 
 	sg_init_one(&output, dst, PAGE_SIZE);
-	acomp_request_set_params(acomp_ctx->req, &input, &output, PAGE_SIZE, dlen);
+	acomp_request_set_params(acomp_ctx->comp.req, &input, &output,
+				 PAGE_SIZE, dlen);
 
 	/*
 	 * it maybe looks a little bit silly that we send an asynchronous request,
@@ -885,10 +898,12 @@ static bool zswap_compress(struct folio *folio, long index,
 	 * existing method to send the second page before the first page is done
 	 * in one thread doing zswap.
 	 * but in different threads running on different cpu, we have different
-	 * acomp instance, so multiple threads can do (de)compression in parallel.
+	 * acomp instance, and compression and decompression use separate
+	 * requests, so multiple threads can do (de)compression in parallel.
 	 */
-	comp_ret = crypto_wait_req(crypto_acomp_compress(acomp_ctx->req), &acomp_ctx->wait);
-	dlen = acomp_ctx->req->dlen;
+	comp_ret = crypto_wait_req(crypto_acomp_compress(acomp_ctx->comp.req),
+				   &acomp_ctx->comp.wait);
+	dlen = acomp_ctx->comp.req->dlen;
 
 	/*
 	 * If a page cannot be compressed into a size smaller than PAGE_SIZE,
@@ -932,7 +947,7 @@ static bool zswap_compress(struct folio *folio, long index,
 	else if (alloc_ret)
 		zswap_reject_alloc_fail++;
 
-	mutex_unlock(&acomp_ctx->mutex);
+	mutex_unlock(&acomp_ctx->comp.mutex);
 	return comp_ret == 0 && alloc_ret == 0;
 }
 
@@ -948,7 +963,7 @@ static bool zswap_decompress(struct zswap_entry *entry, struct folio *folio)
 		return false;
 
 	acomp_ctx = raw_cpu_ptr(pool->acomp_ctx);
-	mutex_lock(&acomp_ctx->mutex);
+	mutex_lock(&acomp_ctx->decomp.mutex);
 	zs_obj_read_sg_begin(pool->zs_pool, entry->handle, input, entry->length);
 
 	/* zswap entries of length PAGE_SIZE are not compressed. */
@@ -965,15 +980,15 @@ static bool zswap_decompress(struct zswap_entry *entry, struct folio *folio)
 	} else {
 		sg_init_table(&output, 1);
 		sg_set_folio(&output, folio, PAGE_SIZE, 0);
-		acomp_request_set_params(acomp_ctx->req, input, &output,
+		acomp_request_set_params(acomp_ctx->decomp.req, input, &output,
 					 entry->length, PAGE_SIZE);
-		ret = crypto_acomp_decompress(acomp_ctx->req);
-		ret = crypto_wait_req(ret, &acomp_ctx->wait);
-		dlen = acomp_ctx->req->dlen;
+		ret = crypto_acomp_decompress(acomp_ctx->decomp.req);
+		ret = crypto_wait_req(ret, &acomp_ctx->decomp.wait);
+		dlen = acomp_ctx->decomp.req->dlen;
 	}
 
 	zs_obj_read_sg_end(pool->zs_pool, entry->handle);
-	mutex_unlock(&acomp_ctx->mutex);
+	mutex_unlock(&acomp_ctx->decomp.mutex);
 
 	if (!ret && dlen == PAGE_SIZE)
 		return true;
-- 
2.53.0-Meta

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

[Nhat Pham]

On Tue, Oct 6, 2026 at 2:23 AM 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. 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. Loads can still wait for
> each other on the decompression mutex, and stores still serialize on
> the compression mutex.
>
> This follows the proposal from Sergey Senozhatsky for the same split
> for zram [1].

Thanks, zram peeps :P

>
> [1] https://lore.kernel.org/all/20261005122036.718976-10-senozhatsky@chromium.org/
>
> Signed-off-by: Usama Arif <usama.arif@linux.dev>

Code mostly LGTM. Just one question:

[...]

> -        * If there was an error in allocating @acomp_ctx->req, it
> -        * would be set to NULL.
> -        */
> -       if (acomp_ctx->req)
> -               acomp_request_free(acomp_ctx->req);
> -
> -       acomp_ctx->req = NULL;
> +       acomp_request_free(acomp_ctx->comp.req);
> +       acomp_ctx->comp.req = NULL;
> +       acomp_request_free(acomp_ctx->decomp.req);
> +       acomp_ctx->decomp.req = NULL;

Hmm do we not have to null check here anymore? Does
acomp_request_free() handle NULL itself too?

For instance, taking the code blob below:

> -       /* acomp_request_alloc() returns NULL in case of an error. */
> -       acomp_ctx->req = acomp_request_alloc(acomp_ctx->acomp);
> -       if (!acomp_ctx->req) {
> +       if (zswap_acomp_req_init(&acomp_ctx->comp, acomp_ctx->acomp) ||
> +           zswap_acomp_req_init(&acomp_ctx->decomp, acomp_ctx->acomp)) {
>                 pr_err("could not alloc crypto acomp_request %s\n",
>                        pool->tfm_name);

Here, we can success with the comp's req but fail with the decomp's req, right?

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

[Sergey Senozhatsky]

On (26/10/06 11:47), Nhat Pham wrote:
> On Tue, Oct 6, 2026 at 2:23 AM 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. 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. Loads can still wait for
> > each other on the decompression mutex, and stores still serialize on
> > the compression mutex.
> >
> > This follows the proposal from Sergey Senozhatsky for the same split
> > for zram [1].
> 
> Thanks, zram peeps :P

:D

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

[Usama Arif]

On 06/10/2026 11:47, Nhat Pham wrote:
> On Tue, Oct 6, 2026 at 2:23 AM 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. 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. Loads can still wait for
>> each other on the decompression mutex, and stores still serialize on
>> the compression mutex.
>>
>> This follows the proposal from Sergey Senozhatsky for the same split
>> for zram [1].
> 
> Thanks, zram peeps :P
> 
>>
>> [1] https://lore.kernel.org/all/20261005122036.718976-10-senozhatsky@chromium.org/
>>
>> Signed-off-by: Usama Arif <usama.arif@linux.dev>
> 
> Code mostly LGTM. Just one question:
> 
> [...]
> 
>> -        * If there was an error in allocating @acomp_ctx->req, it
>> -        * would be set to NULL.
>> -        */
>> -       if (acomp_ctx->req)
>> -               acomp_request_free(acomp_ctx->req);
>> -
>> -       acomp_ctx->req = NULL;
>> +       acomp_request_free(acomp_ctx->comp.req);
>> +       acomp_ctx->comp.req = NULL;
>> +       acomp_request_free(acomp_ctx->decomp.req);
>> +       acomp_ctx->decomp.req = NULL;
> 
> Hmm do we not have to null check here anymore? Does
> acomp_request_free() handle NULL itself too?

Yes, since v6.15 it starts with "if (!req || ...) return;", so the
check in acomp_ctx_free() was redundant.


> 
> For instance, taking the code blob below:
> 
>> -       /* acomp_request_alloc() returns NULL in case of an error. */
>> -       acomp_ctx->req = acomp_request_alloc(acomp_ctx->acomp);
>> -       if (!acomp_ctx->req) {
>> +       if (zswap_acomp_req_init(&acomp_ctx->comp, acomp_ctx->acomp) ||
>> +           zswap_acomp_req_init(&acomp_ctx->decomp, acomp_ctx->acomp)) {
>>                 pr_err("could not alloc crypto acomp_request %s\n",
>>                        pool->tfm_name);
> 
> Here, we can success with the comp's req but fail with the decomp's req, right?

Right. decomp.req is then NULL, as zswap_acomp_req_init() stores what
acomp_request_alloc() returned, and acomp_ctx_free() frees comp.req
and skips decomp.req. After patch 2, decomp.req also stays NULL for
synchronous algorithms, since the per-CPU contexts are zeroed, and
acomp_ctx_free() relies on the same NULL handling.

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

[Yosry Ahmed]

On Mon, Oct 5, 2026 at 5:23 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. 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.

I am a bit uncomfortable with this. If future changes modify the
transform state while (de)compressing, it may result in nasty bugs.

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.

> Loads can still wait for
> each other on the decompression mutex, and stores still serialize on
> the compression mutex.
>
> 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>
> ---
>  mm/zswap.c | 89 +++++++++++++++++++++++++++++++-----------------------
>  1 file changed, 52 insertions(+), 37 deletions(-)
>
> diff --git a/mm/zswap.c b/mm/zswap.c
> index ae19e301fced7..54187b1ef751d 100644
> --- a/mm/zswap.c
> +++ b/mm/zswap.c
> @@ -137,14 +137,20 @@ bool zswap_never_enabled(void)
>  * data structures
>  **********************************/
>
> -struct crypto_acomp_ctx {
> -       struct crypto_acomp *acomp;
> +struct zswap_acomp_req {
>         struct acomp_req *req;
>         struct crypto_wait wait;
> -       u8 *buffer;
>         struct mutex mutex;
>  };
>
> +/* Separate requests, so that decompression does not wait for compression. */
> +struct crypto_acomp_ctx {
> +       struct crypto_acomp *acomp;
> +       struct zswap_acomp_req comp;
> +       struct zswap_acomp_req decomp;
> +       u8 *buffer;
> +};
> +
>  /*
>   * The lock ordering is zswap_tree.lock -> zswap_pool.lru_lock.
>   * The only case where lru_lock is not acquired while holding tree.lock is
> @@ -270,14 +276,10 @@ static void acomp_ctx_free(struct crypto_acomp_ctx *acomp_ctx)
>         if (!acomp_ctx)
>                 return;
>
> -       /*
> -        * If there was an error in allocating @acomp_ctx->req, it
> -        * would be set to NULL.
> -        */
> -       if (acomp_ctx->req)
> -               acomp_request_free(acomp_ctx->req);
> -
> -       acomp_ctx->req = NULL;
> +       acomp_request_free(acomp_ctx->comp.req);
> +       acomp_ctx->comp.req = NULL;
> +       acomp_request_free(acomp_ctx->decomp.req);
> +       acomp_ctx->decomp.req = NULL;
>
>         /*
>          * We have to handle both cases here: an error pointer return from
> @@ -796,6 +798,28 @@ static void zswap_entry_free(struct zswap_entry *entry)
>  /*********************************
>  * compressed storage functions
>  **********************************/
> +static int zswap_acomp_req_init(struct zswap_acomp_req *areq,
> +                               struct crypto_acomp *acomp)
> +{
> +       /* acomp_request_alloc() returns NULL in case of an error. */
> +       areq->req = acomp_request_alloc(acomp);
> +       if (!areq->req)
> +               return -ENOMEM;
> +
> +       crypto_init_wait(&areq->wait);
> +
> +       /*
> +        * if the backend of acomp is async zip, crypto_req_done() will wakeup
> +        * crypto_wait_req(); if the backend of acomp is scomp, the callback
> +        * won't be called, crypto_wait_req() will return without blocking.
> +        */
> +       acomp_request_set_callback(areq->req, CRYPTO_TFM_REQ_MAY_BACKLOG,
> +                                  crypto_req_done, &areq->wait);
> +
> +       mutex_init(&areq->mutex);
> +       return 0;
> +}
> +
>  static int zswap_cpu_comp_prepare(unsigned int cpu, struct hlist_node *node)
>  {
>         struct zswap_pool *pool = hlist_entry(node, struct zswap_pool, node);
> @@ -827,25 +851,13 @@ static int zswap_cpu_comp_prepare(unsigned int cpu, struct hlist_node *node)
>                 goto fail;
>         }
>
> -       /* acomp_request_alloc() returns NULL in case of an error. */
> -       acomp_ctx->req = acomp_request_alloc(acomp_ctx->acomp);
> -       if (!acomp_ctx->req) {
> +       if (zswap_acomp_req_init(&acomp_ctx->comp, acomp_ctx->acomp) ||
> +           zswap_acomp_req_init(&acomp_ctx->decomp, acomp_ctx->acomp)) {
>                 pr_err("could not alloc crypto acomp_request %s\n",
>                        pool->tfm_name);
>                 goto fail;
>         }
>
> -       crypto_init_wait(&acomp_ctx->wait);
> -
> -       /*
> -        * if the backend of acomp is async zip, crypto_req_done() will wakeup
> -        * crypto_wait_req(); if the backend of acomp is scomp, the callback
> -        * won't be called, crypto_wait_req() will return without blocking.
> -        */
> -       acomp_request_set_callback(acomp_ctx->req, CRYPTO_TFM_REQ_MAY_BACKLOG,
> -                                  crypto_req_done, &acomp_ctx->wait);
> -
> -       mutex_init(&acomp_ctx->mutex);
>         return 0;
>
>  fail:
> @@ -866,14 +878,15 @@ static bool zswap_compress(struct folio *folio, long index,
>         bool mapped = false;
>
>         acomp_ctx = raw_cpu_ptr(pool->acomp_ctx);
> -       mutex_lock(&acomp_ctx->mutex);
> +       mutex_lock(&acomp_ctx->comp.mutex);
>
>         dst = acomp_ctx->buffer;
>         sg_init_table(&input, 1);
>         sg_set_folio(&input, folio, PAGE_SIZE, index * PAGE_SIZE);
>
>         sg_init_one(&output, dst, PAGE_SIZE);
> -       acomp_request_set_params(acomp_ctx->req, &input, &output, PAGE_SIZE, dlen);
> +       acomp_request_set_params(acomp_ctx->comp.req, &input, &output,
> +                                PAGE_SIZE, dlen);
>
>         /*
>          * it maybe looks a little bit silly that we send an asynchronous request,
> @@ -885,10 +898,12 @@ static bool zswap_compress(struct folio *folio, long index,
>          * existing method to send the second page before the first page is done
>          * in one thread doing zswap.
>          * but in different threads running on different cpu, we have different
> -        * acomp instance, so multiple threads can do (de)compression in parallel.
> +        * acomp instance, and compression and decompression use separate
> +        * requests, so multiple threads can do (de)compression in parallel.
>          */
> -       comp_ret = crypto_wait_req(crypto_acomp_compress(acomp_ctx->req), &acomp_ctx->wait);
> -       dlen = acomp_ctx->req->dlen;
> +       comp_ret = crypto_wait_req(crypto_acomp_compress(acomp_ctx->comp.req),
> +                                  &acomp_ctx->comp.wait);
> +       dlen = acomp_ctx->comp.req->dlen;
>
>         /*
>          * If a page cannot be compressed into a size smaller than PAGE_SIZE,
> @@ -932,7 +947,7 @@ static bool zswap_compress(struct folio *folio, long index,
>         else if (alloc_ret)
>                 zswap_reject_alloc_fail++;
>
> -       mutex_unlock(&acomp_ctx->mutex);
> +       mutex_unlock(&acomp_ctx->comp.mutex);
>         return comp_ret == 0 && alloc_ret == 0;
>  }
>
> @@ -948,7 +963,7 @@ static bool zswap_decompress(struct zswap_entry *entry, struct folio *folio)
>                 return false;
>
>         acomp_ctx = raw_cpu_ptr(pool->acomp_ctx);
> -       mutex_lock(&acomp_ctx->mutex);
> +       mutex_lock(&acomp_ctx->decomp.mutex);
>         zs_obj_read_sg_begin(pool->zs_pool, entry->handle, input, entry->length);
>
>         /* zswap entries of length PAGE_SIZE are not compressed. */
> @@ -965,15 +980,15 @@ static bool zswap_decompress(struct zswap_entry *entry, struct folio *folio)
>         } else {
>                 sg_init_table(&output, 1);
>                 sg_set_folio(&output, folio, PAGE_SIZE, 0);
> -               acomp_request_set_params(acomp_ctx->req, input, &output,
> +               acomp_request_set_params(acomp_ctx->decomp.req, input, &output,
>                                          entry->length, PAGE_SIZE);
> -               ret = crypto_acomp_decompress(acomp_ctx->req);
> -               ret = crypto_wait_req(ret, &acomp_ctx->wait);
> -               dlen = acomp_ctx->req->dlen;
> +               ret = crypto_acomp_decompress(acomp_ctx->decomp.req);
> +               ret = crypto_wait_req(ret, &acomp_ctx->decomp.wait);
> +               dlen = acomp_ctx->decomp.req->dlen;
>         }
>
>         zs_obj_read_sg_end(pool->zs_pool, entry->handle);
> -       mutex_unlock(&acomp_ctx->mutex);
> +       mutex_unlock(&acomp_ctx->decomp.mutex);
>
>         if (!ret && dlen == PAGE_SIZE)
>                 return true;
> --
> 2.53.0-Meta
>

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

[Usama Arif]

On 07/10/2026 23:01, Yosry Ahmed wrote:
> On Mon, Oct 5, 2026 at 5:23 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. 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.


> 
> 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.

> 
>> Loads can still wait for
>> each other on the decompression mutex, and stores still serialize on
>> the compression mutex.
>>
>> 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>
>> ---
>>  mm/zswap.c | 89 +++++++++++++++++++++++++++++++-----------------------
>>  1 file changed, 52 insertions(+), 37 deletions(-)
>>
>> diff --git a/mm/zswap.c b/mm/zswap.c
>> index ae19e301fced7..54187b1ef751d 100644
>> --- a/mm/zswap.c
>> +++ b/mm/zswap.c
>> @@ -137,14 +137,20 @@ bool zswap_never_enabled(void)
>>  * data structures
>>  **********************************/
>>
>> -struct crypto_acomp_ctx {
>> -       struct crypto_acomp *acomp;
>> +struct zswap_acomp_req {
>>         struct acomp_req *req;
>>         struct crypto_wait wait;
>> -       u8 *buffer;
>>         struct mutex mutex;
>>  };
>>
>> +/* Separate requests, so that decompression does not wait for compression. */
>> +struct crypto_acomp_ctx {
>> +       struct crypto_acomp *acomp;
>> +       struct zswap_acomp_req comp;
>> +       struct zswap_acomp_req decomp;
>> +       u8 *buffer;
>> +};
>> +
>>  /*
>>   * The lock ordering is zswap_tree.lock -> zswap_pool.lru_lock.
>>   * The only case where lru_lock is not acquired while holding tree.lock is
>> @@ -270,14 +276,10 @@ static void acomp_ctx_free(struct crypto_acomp_ctx *acomp_ctx)
>>         if (!acomp_ctx)
>>                 return;
>>
>> -       /*
>> -        * If there was an error in allocating @acomp_ctx->req, it
>> -        * would be set to NULL.
>> -        */
>> -       if (acomp_ctx->req)
>> -               acomp_request_free(acomp_ctx->req);
>> -
>> -       acomp_ctx->req = NULL;
>> +       acomp_request_free(acomp_ctx->comp.req);
>> +       acomp_ctx->comp.req = NULL;
>> +       acomp_request_free(acomp_ctx->decomp.req);
>> +       acomp_ctx->decomp.req = NULL;
>>
>>         /*
>>          * We have to handle both cases here: an error pointer return from
>> @@ -796,6 +798,28 @@ static void zswap_entry_free(struct zswap_entry *entry)
>>  /*********************************
>>  * compressed storage functions
>>  **********************************/
>> +static int zswap_acomp_req_init(struct zswap_acomp_req *areq,
>> +                               struct crypto_acomp *acomp)
>> +{
>> +       /* acomp_request_alloc() returns NULL in case of an error. */
>> +       areq->req = acomp_request_alloc(acomp);
>> +       if (!areq->req)
>> +               return -ENOMEM;
>> +
>> +       crypto_init_wait(&areq->wait);
>> +
>> +       /*
>> +        * if the backend of acomp is async zip, crypto_req_done() will wakeup
>> +        * crypto_wait_req(); if the backend of acomp is scomp, the callback
>> +        * won't be called, crypto_wait_req() will return without blocking.
>> +        */
>> +       acomp_request_set_callback(areq->req, CRYPTO_TFM_REQ_MAY_BACKLOG,
>> +                                  crypto_req_done, &areq->wait);
>> +
>> +       mutex_init(&areq->mutex);
>> +       return 0;
>> +}
>> +
>>  static int zswap_cpu_comp_prepare(unsigned int cpu, struct hlist_node *node)
>>  {
>>         struct zswap_pool *pool = hlist_entry(node, struct zswap_pool, node);
>> @@ -827,25 +851,13 @@ static int zswap_cpu_comp_prepare(unsigned int cpu, struct hlist_node *node)
>>                 goto fail;
>>         }
>>
>> -       /* acomp_request_alloc() returns NULL in case of an error. */
>> -       acomp_ctx->req = acomp_request_alloc(acomp_ctx->acomp);
>> -       if (!acomp_ctx->req) {
>> +       if (zswap_acomp_req_init(&acomp_ctx->comp, acomp_ctx->acomp) ||
>> +           zswap_acomp_req_init(&acomp_ctx->decomp, acomp_ctx->acomp)) {
>>                 pr_err("could not alloc crypto acomp_request %s\n",
>>                        pool->tfm_name);
>>                 goto fail;
>>         }
>>
>> -       crypto_init_wait(&acomp_ctx->wait);
>> -
>> -       /*
>> -        * if the backend of acomp is async zip, crypto_req_done() will wakeup
>> -        * crypto_wait_req(); if the backend of acomp is scomp, the callback
>> -        * won't be called, crypto_wait_req() will return without blocking.
>> -        */
>> -       acomp_request_set_callback(acomp_ctx->req, CRYPTO_TFM_REQ_MAY_BACKLOG,
>> -                                  crypto_req_done, &acomp_ctx->wait);
>> -
>> -       mutex_init(&acomp_ctx->mutex);
>>         return 0;
>>
>>  fail:
>> @@ -866,14 +878,15 @@ static bool zswap_compress(struct folio *folio, long index,
>>         bool mapped = false;
>>
>>         acomp_ctx = raw_cpu_ptr(pool->acomp_ctx);
>> -       mutex_lock(&acomp_ctx->mutex);
>> +       mutex_lock(&acomp_ctx->comp.mutex);
>>
>>         dst = acomp_ctx->buffer;
>>         sg_init_table(&input, 1);
>>         sg_set_folio(&input, folio, PAGE_SIZE, index * PAGE_SIZE);
>>
>>         sg_init_one(&output, dst, PAGE_SIZE);
>> -       acomp_request_set_params(acomp_ctx->req, &input, &output, PAGE_SIZE, dlen);
>> +       acomp_request_set_params(acomp_ctx->comp.req, &input, &output,
>> +                                PAGE_SIZE, dlen);
>>
>>         /*
>>          * it maybe looks a little bit silly that we send an asynchronous request,
>> @@ -885,10 +898,12 @@ static bool zswap_compress(struct folio *folio, long index,
>>          * existing method to send the second page before the first page is done
>>          * in one thread doing zswap.
>>          * but in different threads running on different cpu, we have different
>> -        * acomp instance, so multiple threads can do (de)compression in parallel.
>> +        * acomp instance, and compression and decompression use separate
>> +        * requests, so multiple threads can do (de)compression in parallel.
>>          */
>> -       comp_ret = crypto_wait_req(crypto_acomp_compress(acomp_ctx->req), &acomp_ctx->wait);
>> -       dlen = acomp_ctx->req->dlen;
>> +       comp_ret = crypto_wait_req(crypto_acomp_compress(acomp_ctx->comp.req),
>> +                                  &acomp_ctx->comp.wait);
>> +       dlen = acomp_ctx->comp.req->dlen;
>>
>>         /*
>>          * If a page cannot be compressed into a size smaller than PAGE_SIZE,
>> @@ -932,7 +947,7 @@ static bool zswap_compress(struct folio *folio, long index,
>>         else if (alloc_ret)
>>                 zswap_reject_alloc_fail++;
>>
>> -       mutex_unlock(&acomp_ctx->mutex);
>> +       mutex_unlock(&acomp_ctx->comp.mutex);
>>         return comp_ret == 0 && alloc_ret == 0;
>>  }
>>
>> @@ -948,7 +963,7 @@ static bool zswap_decompress(struct zswap_entry *entry, struct folio *folio)
>>                 return false;
>>
>>         acomp_ctx = raw_cpu_ptr(pool->acomp_ctx);
>> -       mutex_lock(&acomp_ctx->mutex);
>> +       mutex_lock(&acomp_ctx->decomp.mutex);
>>         zs_obj_read_sg_begin(pool->zs_pool, entry->handle, input, entry->length);
>>
>>         /* zswap entries of length PAGE_SIZE are not compressed. */
>> @@ -965,15 +980,15 @@ static bool zswap_decompress(struct zswap_entry *entry, struct folio *folio)
>>         } else {
>>                 sg_init_table(&output, 1);
>>                 sg_set_folio(&output, folio, PAGE_SIZE, 0);
>> -               acomp_request_set_params(acomp_ctx->req, input, &output,
>> +               acomp_request_set_params(acomp_ctx->decomp.req, input, &output,
>>                                          entry->length, PAGE_SIZE);
>> -               ret = crypto_acomp_decompress(acomp_ctx->req);
>> -               ret = crypto_wait_req(ret, &acomp_ctx->wait);
>> -               dlen = acomp_ctx->req->dlen;
>> +               ret = crypto_acomp_decompress(acomp_ctx->decomp.req);
>> +               ret = crypto_wait_req(ret, &acomp_ctx->decomp.wait);
>> +               dlen = acomp_ctx->decomp.req->dlen;
>>         }
>>
>>         zs_obj_read_sg_end(pool->zs_pool, entry->handle);
>> -       mutex_unlock(&acomp_ctx->mutex);
>> +       mutex_unlock(&acomp_ctx->decomp.mutex);
>>
>>         if (!ret && dlen == PAGE_SIZE)
>>                 return true;
>> --
>> 2.53.0-Meta
>>

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@linux.dev> wrote:
>
>
>
> On 07/10/2026 23:01, Yosry Ahmed wrote:
> > On Mon, Oct 5, 2026 at 5:23 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. 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!

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

[Sergey Senozhatsky]

On (26/10/05 17:22), Usama Arif 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. Loads can still wait for
> each other on the decompression mutex, and stores still serialize on
> the compression mutex.
> 
> This follows the proposal from Sergey Senozhatsky for the same split
> for zram [1].

Greetings zswap peeps,
We pushed things a little further for even more gains [1]

Catch me if you can ;P

[1] https://lore.kernel.org/all/20261009071157.3730698-12-senozhatsky@chromium.org

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

[Usama Arif]

On 09/10/2026 09:16, Sergey Senozhatsky wrote:
> On (26/10/05 17:22), Usama Arif 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. Loads can still wait for
>> each other on the decompression mutex, and stores still serialize on
>> the compression mutex.
>>
>> This follows the proposal from Sergey Senozhatsky for the same split
>> for zram [1].
> 
> Greetings zswap peeps,

Hello!


> We pushed things a little further for even more gains [1]

:)

Thanks for the inital patches!


> 
> Catch me if you can ;P
> 
> [1] https://lore.kernel.org/all/20261009071157.3730698-12-senozhatsky@chromium.org

I think it wont help zstd, right? It would help stateless decompressors like
lz4 I think.

We could that as a followup to the series for zswap.

[PATCH 2/2] mm: zswap: use stack requests for synchronous decompression

[Usama Arif]

With separate requests for compression and decompression, loads still
serialize on the per-CPU decompression mutex. A low-priority load that
is preempted after the codec drops its stream lock keeps holding the mutex
and stalls every other load on that CPU, including higher-priority ones.

Synchronous algorithms whose requests need no extra context can use an
on-stack request, so decompress with one and take no zswap lock. All
in-tree software compressors qualify. Asynchronous algorithms, and
synchronous ones with request context, keep the per-CPU request and
mutex, which is still taken before the zsmalloc read lock.

Reading the per-CPU context without the mutex is safe. Since
commit ef3c0f6cb798e ("mm: zswap: tie per-CPU acomp_ctx lifetime to the
pool"), it is set up before its CPU comes online and is not torn down
until the pool is destroyed. The codecs keep their own stream locks, and
crypto_acomp_decompress() rejects on-stack requests only for
asynchronous transforms, which never take this path.

For software compressors this drops the heap request added by the
previous patch. The on-stack request and wait take 216 bytes, which
makes the load path about 270 bytes deeper on x86-64. Asynchronous
algorithms pay this too.

Signed-off-by: Usama Arif <usama.arif@linux.dev>
---
 mm/zswap.c | 65 +++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 45 insertions(+), 20 deletions(-)

diff --git a/mm/zswap.c b/mm/zswap.c
index 54187b1ef751d..7e7fb6e7ec24c 100644
--- a/mm/zswap.c
+++ b/mm/zswap.c
@@ -147,7 +147,7 @@ struct zswap_acomp_req {
 struct crypto_acomp_ctx {
 	struct crypto_acomp *acomp;
 	struct zswap_acomp_req comp;
-	struct zswap_acomp_req decomp;
+	struct zswap_acomp_req decomp;	/* unused by synchronous algorithms */
 	u8 *buffer;
 };
 
@@ -851,15 +851,20 @@ static int zswap_cpu_comp_prepare(unsigned int cpu, struct hlist_node *node)
 		goto fail;
 	}
 
-	if (zswap_acomp_req_init(&acomp_ctx->comp, acomp_ctx->acomp) ||
-	    zswap_acomp_req_init(&acomp_ctx->decomp, acomp_ctx->acomp)) {
-		pr_err("could not alloc crypto acomp_request %s\n",
-		       pool->tfm_name);
-		goto fail;
+	if (zswap_acomp_req_init(&acomp_ctx->comp, acomp_ctx->acomp))
+		goto req_fail;
+
+	/* Synchronous algorithms decompress with an on-stack request. */
+	if (acomp_is_async(acomp_ctx->acomp) ||
+	    crypto_acomp_reqsize(acomp_ctx->acomp) > MAX_SYNC_COMP_REQSIZE) {
+		if (zswap_acomp_req_init(&acomp_ctx->decomp, acomp_ctx->acomp))
+			goto req_fail;
 	}
 
 	return 0;
 
+req_fail:
+	pr_err("could not alloc crypto acomp_request %s\n", pool->tfm_name);
 fail:
 	acomp_ctx_free(acomp_ctx);
 	return ret;
@@ -951,19 +956,14 @@ static bool zswap_compress(struct folio *folio, long index,
 	return comp_ret == 0 && alloc_ret == 0;
 }
 
-static bool zswap_decompress(struct zswap_entry *entry, struct folio *folio)
+static bool __zswap_decompress(struct zswap_entry *entry,
+			       struct zswap_pool *pool, struct acomp_req *req,
+			       struct crypto_wait *wait, struct folio *folio)
 {
-	struct zswap_pool *pool = zswap_entry_pool(entry);
 	struct scatterlist input[2]; /* zsmalloc returns an SG list 1-2 entries */
 	struct scatterlist output;
-	struct crypto_acomp_ctx *acomp_ctx;
 	int ret = 0, dlen;
 
-	if (WARN_ON_ONCE(!pool))
-		return false;
-
-	acomp_ctx = raw_cpu_ptr(pool->acomp_ctx);
-	mutex_lock(&acomp_ctx->decomp.mutex);
 	zs_obj_read_sg_begin(pool->zs_pool, entry->handle, input, entry->length);
 
 	/* zswap entries of length PAGE_SIZE are not compressed. */
@@ -980,15 +980,14 @@ static bool zswap_decompress(struct zswap_entry *entry, struct folio *folio)
 	} else {
 		sg_init_table(&output, 1);
 		sg_set_folio(&output, folio, PAGE_SIZE, 0);
-		acomp_request_set_params(acomp_ctx->decomp.req, input, &output,
-					 entry->length, PAGE_SIZE);
-		ret = crypto_acomp_decompress(acomp_ctx->decomp.req);
-		ret = crypto_wait_req(ret, &acomp_ctx->decomp.wait);
-		dlen = acomp_ctx->decomp.req->dlen;
+		acomp_request_set_params(req, input, &output, entry->length,
+					 PAGE_SIZE);
+		ret = crypto_acomp_decompress(req);
+		ret = crypto_wait_req(ret, wait);
+		dlen = req->dlen;
 	}
 
 	zs_obj_read_sg_end(pool->zs_pool, entry->handle);
-	mutex_unlock(&acomp_ctx->decomp.mutex);
 
 	if (!ret && dlen == PAGE_SIZE)
 		return true;
@@ -1002,6 +1001,32 @@ static bool zswap_decompress(struct zswap_entry *entry, struct folio *folio)
 	return false;
 }
 
+static bool zswap_decompress(struct zswap_entry *entry, struct folio *folio)
+{
+	struct zswap_pool *pool = zswap_entry_pool(entry);
+	struct crypto_acomp_ctx *acomp_ctx;
+	bool ret;
+
+	if (WARN_ON_ONCE(!pool))
+		return false;
+
+	acomp_ctx = raw_cpu_ptr(pool->acomp_ctx);
+	if (!acomp_ctx->decomp.req) {
+		ACOMP_REQUEST_ON_STACK(req, acomp_ctx->acomp);
+		DECLARE_CRYPTO_WAIT(wait);
+
+		acomp_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
+					   crypto_req_done, &wait);
+		return __zswap_decompress(entry, pool, req, &wait, folio);
+	}
+
+	mutex_lock(&acomp_ctx->decomp.mutex);
+	ret = __zswap_decompress(entry, pool, acomp_ctx->decomp.req,
+				 &acomp_ctx->decomp.wait, folio);
+	mutex_unlock(&acomp_ctx->decomp.mutex);
+	return ret;
+}
+
 /*********************************
 * writeback code
 **********************************/
-- 
2.53.0-Meta

Re: [PATCH 2/2] mm: zswap: use stack requests for synchronous decompression

[Nhat Pham]

On Tue, Oct 6, 2026 at 2:23 AM Usama Arif <usama.arif@linux.dev> wrote:
>
> With separate requests for compression and decompression, loads still
> serialize on the per-CPU decompression mutex. A low-priority load that
> is preempted after the codec drops its stream lock keeps holding the mutex
> and stalls every other load on that CPU, including higher-priority ones.
>
> Synchronous algorithms whose requests need no extra context can use an
> on-stack request, so decompress with one and take no zswap lock. All
> in-tree software compressors qualify. Asynchronous algorithms, and
> synchronous ones with request context, keep the per-CPU request and
> mutex, which is still taken before the zsmalloc read lock.
>
> Reading the per-CPU context without the mutex is safe. Since
> commit ef3c0f6cb798e ("mm: zswap: tie per-CPU acomp_ctx lifetime to the
> pool"), it is set up before its CPU comes online and is not torn down
> until the pool is destroyed. The codecs keep their own stream locks, and
> crypto_acomp_decompress() rejects on-stack requests only for
> asynchronous transforms, which never take this path.
>
> For software compressors this drops the heap request added by the
> previous patch. The on-stack request and wait take 216 bytes, which
> makes the load path about 270 bytes deeper on x86-64. Asynchronous
> algorithms pay this too.
>
> Signed-off-by: Usama Arif <usama.arif@linux.dev>

LGTM!

Acked-by: Nhat Pham <nphamcs@gmail.com>

> +               ACOMP_REQUEST_ON_STACK(req, acomp_ctx->acomp);
> +               DECLARE_CRYPTO_WAIT(wait);
> +
> +               acomp_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
> +                                          crypto_req_done, &wait);


Lol this got me reading crypto API again.

Re: [PATCH 2/2] mm: zswap: use stack requests for synchronous decompression

[Yosry Ahmed]

On Mon, Oct 5, 2026 at 5:23 PM Usama Arif <usama.arif@linux.dev> wrote:
>
> With separate requests for compression and decompression, loads still
> serialize on the per-CPU decompression mutex. A low-priority load that
> is preempted after the codec drops its stream lock keeps holding the mutex
> and stalls every other load on that CPU, including higher-priority ones.
>
> Synchronous algorithms whose requests need no extra context can use an
> on-stack request, so decompress with one and take no zswap lock. All
> in-tree software compressors qualify. Asynchronous algorithms, and
> synchronous ones with request context, keep the per-CPU request and
> mutex, which is still taken before the zsmalloc read lock.
>
> Reading the per-CPU context without the mutex is safe. Since
> commit ef3c0f6cb798e ("mm: zswap: tie per-CPU acomp_ctx lifetime to the
> pool"), it is set up before its CPU comes online and is not torn down
> until the pool is destroyed. The codecs keep their own stream locks, and
> crypto_acomp_decompress() rejects on-stack requests only for
> asynchronous transforms, which never take this path.
>
> For software compressors this drops the heap request added by the
> previous patch. The on-stack request and wait take 216 bytes, which
> makes the load path about 270 bytes deeper on x86-64. Asynchronous
> algorithms pay this too.
>
> Signed-off-by: Usama Arif <usama.arif@linux.dev>
> ---
>  mm/zswap.c | 65 +++++++++++++++++++++++++++++++++++++-----------------
>  1 file changed, 45 insertions(+), 20 deletions(-)
>
> diff --git a/mm/zswap.c b/mm/zswap.c
> index 54187b1ef751d..7e7fb6e7ec24c 100644
> --- a/mm/zswap.c
> +++ b/mm/zswap.c
> @@ -147,7 +147,7 @@ struct zswap_acomp_req {
>  struct crypto_acomp_ctx {
>         struct crypto_acomp *acomp;
>         struct zswap_acomp_req comp;
> -       struct zswap_acomp_req decomp;
> +       struct zswap_acomp_req decomp;  /* unused by synchronous algorithms */
>         u8 *buffer;
>  };
>
> @@ -851,15 +851,20 @@ static int zswap_cpu_comp_prepare(unsigned int cpu, struct hlist_node *node)
>                 goto fail;
>         }
>
> -       if (zswap_acomp_req_init(&acomp_ctx->comp, acomp_ctx->acomp) ||
> -           zswap_acomp_req_init(&acomp_ctx->decomp, acomp_ctx->acomp)) {
> -               pr_err("could not alloc crypto acomp_request %s\n",
> -                      pool->tfm_name);
> -               goto fail;
> +       if (zswap_acomp_req_init(&acomp_ctx->comp, acomp_ctx->acomp))
> +               goto req_fail;
> +
> +       /* Synchronous algorithms decompress with an on-stack request. */
> +       if (acomp_is_async(acomp_ctx->acomp) ||
> +           crypto_acomp_reqsize(acomp_ctx->acomp) > MAX_SYNC_COMP_REQSIZE) {

Where does the requirement on crypto_acomp_reqsize() come from? I see
crypto_acomp_compress() and crypto_acomp_decompress() only checking
acomp_is_async().

Regardless, these are crypto-specific details that shouldn't be
checked directly by zswap. Ideally we'd have something like
acomp_can_use_stack_req() or something.

Also, could you please CC Herbert on future iterations? I would like
to get his eyes on any crypto-related changes if possible.

> +               if (zswap_acomp_req_init(&acomp_ctx->decomp, acomp_ctx->acomp))
> +                       goto req_fail;
>         }
>
>         return 0;
>
> +req_fail:
> +       pr_err("could not alloc crypto acomp_request %s\n", pool->tfm_name);
>  fail:
>         acomp_ctx_free(acomp_ctx);
>         return ret;
> @@ -951,19 +956,14 @@ static bool zswap_compress(struct folio *folio, long index,
>         return comp_ret == 0 && alloc_ret == 0;
>  }
>
> -static bool zswap_decompress(struct zswap_entry *entry, struct folio *folio)
> +static bool __zswap_decompress(struct zswap_entry *entry,
> +                              struct zswap_pool *pool, struct acomp_req *req,
> +                              struct crypto_wait *wait, struct folio *folio)
>  {
> -       struct zswap_pool *pool = zswap_entry_pool(entry);
>         struct scatterlist input[2]; /* zsmalloc returns an SG list 1-2 entries */
>         struct scatterlist output;
> -       struct crypto_acomp_ctx *acomp_ctx;
>         int ret = 0, dlen;
>
> -       if (WARN_ON_ONCE(!pool))
> -               return false;
> -
> -       acomp_ctx = raw_cpu_ptr(pool->acomp_ctx);
> -       mutex_lock(&acomp_ctx->decomp.mutex);
>         zs_obj_read_sg_begin(pool->zs_pool, entry->handle, input, entry->length);
>
>         /* zswap entries of length PAGE_SIZE are not compressed. */
> @@ -980,15 +980,14 @@ static bool zswap_decompress(struct zswap_entry *entry, struct folio *folio)
>         } else {
>                 sg_init_table(&output, 1);
>                 sg_set_folio(&output, folio, PAGE_SIZE, 0);
> -               acomp_request_set_params(acomp_ctx->decomp.req, input, &output,
> -                                        entry->length, PAGE_SIZE);
> -               ret = crypto_acomp_decompress(acomp_ctx->decomp.req);
> -               ret = crypto_wait_req(ret, &acomp_ctx->decomp.wait);
> -               dlen = acomp_ctx->decomp.req->dlen;
> +               acomp_request_set_params(req, input, &output, entry->length,
> +                                        PAGE_SIZE);

Just use a single line :)

> +               ret = crypto_acomp_decompress(req);
> +               ret = crypto_wait_req(ret, wait);
> +               dlen = req->dlen;
>         }
>
>         zs_obj_read_sg_end(pool->zs_pool, entry->handle);
> -       mutex_unlock(&acomp_ctx->decomp.mutex);
>
>         if (!ret && dlen == PAGE_SIZE)
>                 return true;
> @@ -1002,6 +1001,32 @@ static bool zswap_decompress(struct zswap_entry *entry, struct folio *folio)
>         return false;
>  }
>
> +static bool zswap_decompress(struct zswap_entry *entry, struct folio *folio)
> +{
> +       struct zswap_pool *pool = zswap_entry_pool(entry);
> +       struct crypto_acomp_ctx *acomp_ctx;
> +       bool ret;
> +
> +       if (WARN_ON_ONCE(!pool))
> +               return false;
> +
> +       acomp_ctx = raw_cpu_ptr(pool->acomp_ctx);

We probably want a comment here explaining the two possible paths?

> +       if (!acomp_ctx->decomp.req) {
> +               ACOMP_REQUEST_ON_STACK(req, acomp_ctx->acomp);
> +               DECLARE_CRYPTO_WAIT(wait);
> +
> +               acomp_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
> +                                          crypto_req_done, &wait);

I would rather add a wrapper for this (e.g.
zswap_set_acomp_req_callback()) to avoid the mental toil of checking
that we are passing in the same things as zswap_cpu_comp_prepare().


> +               return __zswap_decompress(entry, pool, req, &wait, folio);

Hmm would it be more readable if we create a dummy zswap_acomp_req
object here and have __zswap_decompress() take in __zswap_decompress
instead of taking in the req and wait separately?

> +       }
> +
> +       mutex_lock(&acomp_ctx->decomp.mutex);
> +       ret = __zswap_decompress(entry, pool, acomp_ctx->decomp.req,
> +                                &acomp_ctx->decomp.wait, folio);
> +       mutex_unlock(&acomp_ctx->decomp.mutex);
> +       return ret;
> +}
> +
>  /*********************************
>  * writeback code
>  **********************************/
> --
> 2.53.0-Meta
>

Re: [PATCH 2/2] mm: zswap: use stack requests for synchronous decompression

[Usama Arif]

On 07/10/2026 23:28, Yosry Ahmed wrote:
> On Mon, Oct 5, 2026 at 5:23 PM Usama Arif <usama.arif@linux.dev> wrote:
>>
>> With separate requests for compression and decompression, loads still
>> serialize on the per-CPU decompression mutex. A low-priority load that
>> is preempted after the codec drops its stream lock keeps holding the mutex
>> and stalls every other load on that CPU, including higher-priority ones.
>>
>> Synchronous algorithms whose requests need no extra context can use an
>> on-stack request, so decompress with one and take no zswap lock. All
>> in-tree software compressors qualify. Asynchronous algorithms, and
>> synchronous ones with request context, keep the per-CPU request and
>> mutex, which is still taken before the zsmalloc read lock.
>>
>> Reading the per-CPU context without the mutex is safe. Since
>> commit ef3c0f6cb798e ("mm: zswap: tie per-CPU acomp_ctx lifetime to the
>> pool"), it is set up before its CPU comes online and is not torn down
>> until the pool is destroyed. The codecs keep their own stream locks, and
>> crypto_acomp_decompress() rejects on-stack requests only for
>> asynchronous transforms, which never take this path.
>>
>> For software compressors this drops the heap request added by the
>> previous patch. The on-stack request and wait take 216 bytes, which
>> makes the load path about 270 bytes deeper on x86-64. Asynchronous
>> algorithms pay this too.
>>
>> Signed-off-by: Usama Arif <usama.arif@linux.dev>
>> ---
>>  mm/zswap.c | 65 +++++++++++++++++++++++++++++++++++++-----------------
>>  1 file changed, 45 insertions(+), 20 deletions(-)
>>
>> diff --git a/mm/zswap.c b/mm/zswap.c
>> index 54187b1ef751d..7e7fb6e7ec24c 100644
>> --- a/mm/zswap.c
>> +++ b/mm/zswap.c
>> @@ -147,7 +147,7 @@ struct zswap_acomp_req {
>>  struct crypto_acomp_ctx {
>>         struct crypto_acomp *acomp;
>>         struct zswap_acomp_req comp;
>> -       struct zswap_acomp_req decomp;
>> +       struct zswap_acomp_req decomp;  /* unused by synchronous algorithms */
>>         u8 *buffer;
>>  };
>>
>> @@ -851,15 +851,20 @@ static int zswap_cpu_comp_prepare(unsigned int cpu, struct hlist_node *node)
>>                 goto fail;
>>         }
>>
>> -       if (zswap_acomp_req_init(&acomp_ctx->comp, acomp_ctx->acomp) ||
>> -           zswap_acomp_req_init(&acomp_ctx->decomp, acomp_ctx->acomp)) {
>> -               pr_err("could not alloc crypto acomp_request %s\n",
>> -                      pool->tfm_name);
>> -               goto fail;
>> +       if (zswap_acomp_req_init(&acomp_ctx->comp, acomp_ctx->acomp))
>> +               goto req_fail;
>> +
>> +       /* Synchronous algorithms decompress with an on-stack request. */
>> +       if (acomp_is_async(acomp_ctx->acomp) ||
>> +           crypto_acomp_reqsize(acomp_ctx->acomp) > MAX_SYNC_COMP_REQSIZE) {
> 
> Where does the requirement on crypto_acomp_reqsize() come from? I see
> crypto_acomp_compress() and crypto_acomp_decompress() only checking
> acomp_is_async().

ACOMP_REQUEST_ON_STACK() reserves sizeof(struct acomp_req) plus
MAX_SYNC_COMP_REQSIZE, which is currently zero. A synchronous acomp
algorithm can advertise a larger request context via cra_reqsize. Checking
only acomp_is_async() would then leave that context outside the reserved
stack storage.

The core already applies the same size restriction to synchronous fallback
transforms in crypto_acomp_init_tfm(). The operation functions only reject
asynchronous stack requests; they do not validate the amount of storage
provided.

> 
> Regardless, these are crypto-specific details that shouldn't be
> checked directly by zswap. Ideally we'd have something like
> acomp_can_use_stack_req() or something.
> 

Done for v2. Added documented acomp_can_use_stack_req() in the public acomp
header. It checks both synchronous completion and request-context size;
zswap now uses that helper.


> Also, could you please CC Herbert on future iterations? I would like
> to get his eyes on any crypto-related changes if possible.

Will do! Thanks! get_maintainers.pl didnt add him.

> 
>> +               if (zswap_acomp_req_init(&acomp_ctx->decomp, acomp_ctx->acomp))
>> +                       goto req_fail;
>>         }
>>
>>         return 0;
>>
>> +req_fail:
>> +       pr_err("could not alloc crypto acomp_request %s\n", pool->tfm_name);
>>  fail:
>>         acomp_ctx_free(acomp_ctx);
>>         return ret;
>> @@ -951,19 +956,14 @@ static bool zswap_compress(struct folio *folio, long index,
>>         return comp_ret == 0 && alloc_ret == 0;
>>  }
>>
>> -static bool zswap_decompress(struct zswap_entry *entry, struct folio *folio)
>> +static bool __zswap_decompress(struct zswap_entry *entry,
>> +                              struct zswap_pool *pool, struct acomp_req *req,
>> +                              struct crypto_wait *wait, struct folio *folio)
>>  {
>> -       struct zswap_pool *pool = zswap_entry_pool(entry);
>>         struct scatterlist input[2]; /* zsmalloc returns an SG list 1-2 entries */
>>         struct scatterlist output;
>> -       struct crypto_acomp_ctx *acomp_ctx;
>>         int ret = 0, dlen;
>>
>> -       if (WARN_ON_ONCE(!pool))
>> -               return false;
>> -
>> -       acomp_ctx = raw_cpu_ptr(pool->acomp_ctx);
>> -       mutex_lock(&acomp_ctx->decomp.mutex);
>>         zs_obj_read_sg_begin(pool->zs_pool, entry->handle, input, entry->length);
>>
>>         /* zswap entries of length PAGE_SIZE are not compressed. */
>> @@ -980,15 +980,14 @@ static bool zswap_decompress(struct zswap_entry *entry, struct folio *folio)
>>         } else {
>>                 sg_init_table(&output, 1);
>>                 sg_set_folio(&output, folio, PAGE_SIZE, 0);
>> -               acomp_request_set_params(acomp_ctx->decomp.req, input, &output,
>> -                                        entry->length, PAGE_SIZE);
>> -               ret = crypto_acomp_decompress(acomp_ctx->decomp.req);
>> -               ret = crypto_wait_req(ret, &acomp_ctx->decomp.wait);
>> -               dlen = acomp_ctx->decomp.req->dlen;
>> +               acomp_request_set_params(req, input, &output, entry->length,
>> +                                        PAGE_SIZE);
> 
> Just use a single line :)
> 
>> +               ret = crypto_acomp_decompress(req);
>> +               ret = crypto_wait_req(ret, wait);
>> +               dlen = req->dlen;
>>         }
>>
>>         zs_obj_read_sg_end(pool->zs_pool, entry->handle);
>> -       mutex_unlock(&acomp_ctx->decomp.mutex);
>>
>>         if (!ret && dlen == PAGE_SIZE)
>>                 return true;
>> @@ -1002,6 +1001,32 @@ static bool zswap_decompress(struct zswap_entry *entry, struct folio *folio)
>>         return false;
>>  }
>>
>> +static bool zswap_decompress(struct zswap_entry *entry, struct folio *folio)
>> +{
>> +       struct zswap_pool *pool = zswap_entry_pool(entry);
>> +       struct crypto_acomp_ctx *acomp_ctx;
>> +       bool ret;
>> +
>> +       if (WARN_ON_ONCE(!pool))
>> +               return false;
>> +
>> +       acomp_ctx = raw_cpu_ptr(pool->acomp_ctx);
> 
> We probably want a comment here explaining the two possible paths?

Added for v2.

> 
>> +       if (!acomp_ctx->decomp.req) {
>> +               ACOMP_REQUEST_ON_STACK(req, acomp_ctx->acomp);
>> +               DECLARE_CRYPTO_WAIT(wait);
>> +
>> +               acomp_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
>> +                                          crypto_req_done, &wait);
> 
> I would rather add a wrapper for this (e.g.
> zswap_set_acomp_req_callback()) to avoid the mental toil of checking
> that we are passing in the same things as zswap_cpu_comp_prepare().

Added for v2.

> 
> 
>> +               return __zswap_decompress(entry, pool, req, &wait, folio);
> 
> Hmm would it be more readable if we create a dummy zswap_acomp_req
> object here and have __zswap_decompress() take in __zswap_decompress
> instead of taking in the req and wait separately?


I would keep req and wait as separate arguments. zswap_acomp_req includes a
mutex that stack decompression never uses, and taking the address of its
wait can still reserve the entire wrapper on the stack. Removing the mutex
from that type would add more restructuring. The shared callback helper and
path comment make the setup consistent while keeping the existing stack
footprint.


> 
>> +       }
>> +
>> +       mutex_lock(&acomp_ctx->decomp.mutex);
>> +       ret = __zswap_decompress(entry, pool, acomp_ctx->decomp.req,
>> +                                &acomp_ctx->decomp.wait, folio);
>> +       mutex_unlock(&acomp_ctx->decomp.mutex);
>> +       return ret;
>> +}
>> +
>>  /*********************************
>>  * writeback code
>>  **********************************/
>> --
>> 2.53.0-Meta
>>

Re: [PATCH 2/2] mm: zswap: use stack requests for synchronous decompression

[Yosry Ahmed]

On Fri, Oct 9, 2026 at 8:27 AM Usama Arif <usama.arif@linux.dev> wrote:
>
>
>
> On 07/10/2026 23:28, Yosry Ahmed wrote:
> > On Mon, Oct 5, 2026 at 5:23 PM Usama Arif <usama.arif@linux.dev> wrote:
> >>
> >> With separate requests for compression and decompression, loads still
> >> serialize on the per-CPU decompression mutex. A low-priority load that
> >> is preempted after the codec drops its stream lock keeps holding the mutex
> >> and stalls every other load on that CPU, including higher-priority ones.
> >>
> >> Synchronous algorithms whose requests need no extra context can use an
> >> on-stack request, so decompress with one and take no zswap lock. All
> >> in-tree software compressors qualify. Asynchronous algorithms, and
> >> synchronous ones with request context, keep the per-CPU request and
> >> mutex, which is still taken before the zsmalloc read lock.
> >>
> >> Reading the per-CPU context without the mutex is safe. Since
> >> commit ef3c0f6cb798e ("mm: zswap: tie per-CPU acomp_ctx lifetime to the
> >> pool"), it is set up before its CPU comes online and is not torn down
> >> until the pool is destroyed. The codecs keep their own stream locks, and
> >> crypto_acomp_decompress() rejects on-stack requests only for
> >> asynchronous transforms, which never take this path.
> >>
> >> For software compressors this drops the heap request added by the
> >> previous patch. The on-stack request and wait take 216 bytes, which
> >> makes the load path about 270 bytes deeper on x86-64. Asynchronous
> >> algorithms pay this too.
> >>
> >> Signed-off-by: Usama Arif <usama.arif@linux.dev>
> >> ---
> >>  mm/zswap.c | 65 +++++++++++++++++++++++++++++++++++++-----------------
> >>  1 file changed, 45 insertions(+), 20 deletions(-)
> >>
> >> diff --git a/mm/zswap.c b/mm/zswap.c
> >> index 54187b1ef751d..7e7fb6e7ec24c 100644
> >> --- a/mm/zswap.c
> >> +++ b/mm/zswap.c
> >> @@ -147,7 +147,7 @@ struct zswap_acomp_req {
> >>  struct crypto_acomp_ctx {
> >>         struct crypto_acomp *acomp;
> >>         struct zswap_acomp_req comp;
> >> -       struct zswap_acomp_req decomp;
> >> +       struct zswap_acomp_req decomp;  /* unused by synchronous algorithms */
> >>         u8 *buffer;
> >>  };
> >>
> >> @@ -851,15 +851,20 @@ static int zswap_cpu_comp_prepare(unsigned int cpu, struct hlist_node *node)
> >>                 goto fail;
> >>         }
> >>
> >> -       if (zswap_acomp_req_init(&acomp_ctx->comp, acomp_ctx->acomp) ||
> >> -           zswap_acomp_req_init(&acomp_ctx->decomp, acomp_ctx->acomp)) {
> >> -               pr_err("could not alloc crypto acomp_request %s\n",
> >> -                      pool->tfm_name);
> >> -               goto fail;
> >> +       if (zswap_acomp_req_init(&acomp_ctx->comp, acomp_ctx->acomp))
> >> +               goto req_fail;
> >> +
> >> +       /* Synchronous algorithms decompress with an on-stack request. */
> >> +       if (acomp_is_async(acomp_ctx->acomp) ||
> >> +           crypto_acomp_reqsize(acomp_ctx->acomp) > MAX_SYNC_COMP_REQSIZE) {
> >
> > Where does the requirement on crypto_acomp_reqsize() come from? I see
> > crypto_acomp_compress() and crypto_acomp_decompress() only checking
> > acomp_is_async().
>
> ACOMP_REQUEST_ON_STACK() reserves sizeof(struct acomp_req) plus
> MAX_SYNC_COMP_REQSIZE, which is currently zero. A synchronous acomp
> algorithm can advertise a larger request context via cra_reqsize. Checking
> only acomp_is_async() would then leave that context outside the reserved
> stack storage.
>
> The core already applies the same size restriction to synchronous fallback
> transforms in crypto_acomp_init_tfm(). The operation functions only reject
> asynchronous stack requests; they do not validate the amount of storage
> provided.
>
> >
> > Regardless, these are crypto-specific details that shouldn't be
> > checked directly by zswap. Ideally we'd have something like
> > acomp_can_use_stack_req() or something.
> >
>
> Done for v2. Added documented acomp_can_use_stack_req() in the public acomp
> header. It checks both synchronous completion and request-context size;
> zswap now uses that helper.

Thanks!

>
>
> > Also, could you please CC Herbert on future iterations? I would like
> > to get his eyes on any crypto-related changes if possible.
>
> Will do! Thanks! get_maintainers.pl didnt add him.

Yeah because it didn't actually change crypto files, but I usually
prefer someone who actually understands crypto (aka not me) to take a
look :P

[..]
>
> >
> >
> >> +               return __zswap_decompress(entry, pool, req, &wait, folio);
> >
> > Hmm would it be more readable if we create a dummy zswap_acomp_req
> > object here and have __zswap_decompress() take in __zswap_decompress
> > instead of taking in the req and wait separately?
>
>
> I would keep req and wait as separate arguments. zswap_acomp_req includes a
> mutex that stack decompression never uses, and taking the address of its
> wait can still reserve the entire wrapper on the stack. Removing the mutex
> from that type would add more restructuring. The shared callback helper and
> path comment make the setup consistent while keeping the existing stack
> footprint.

I am fine with that.

Re: [PATCH 0/2] mm: zswap: reduce request contention on loads

[Usama Arif]

On 06/10/2026 01:22, Usama Arif wrote:
> Stores and loads share a per-CPU acomp request and mutex. A low-priority
> store can be preempted right after the compressor drops its stream
> lock, while it still holds the zswap mutex, and a higher-priority load
> on that CPU then waits for the store to run again. This follows the work
> from Sergey Senozhatsky's zram series which splits it for the same
> reason [1].
> 
> Patch 1 gives compression and decompression separate requests, waits
> and mutexes, so loads no longer wait for stores, though they can still
> wait for each other. Patch 2 decompresses with an on-stack request when
> the algorithm is synchronous and needs no request context, which covers
> all in-tree software compressors, so those loads take no zswap lock.
> Asynchronous algorithms keep the per-CPU request and mutex. For software
> compressors the series allocates the same number of requests as before;
> each per-CPU context grows by 72 bytes, and the load path is about 270
> bytes deeper on x86-64.
> 
> The series does not fix two related cases:
> - Stores still serialize on the compression mutex, so a high-priority
>   task that reclaims (direct reclaim, MADV_PAGEOUT) can still wait for
>   a preempted store.
> - On PREEMPT_RT the codec stream locks are preemptible, so a load can
>   still wait for a preempted store inside the codec.
> 
> The numbers below are the slowest read per run, as a median (min-max)
> of 5 runs. Each run is 12 seconds in a zstd VM with lazy preemption,
> vm.page-cluster=0 and swap on /dev/ram0. With 1 vCPU, four nice +10
> workers page memory out and read it back while a nice 0 task spins. A
> nice -19 reader pages out its own buffer and measures how long each
> read of it takes. With 8 vCPUs there are 16 workers, 8 spinning tasks
> and 8 readers.
> 
>               Before series (ms)      With series (ms)
>   1 vCPU      22.3 (21.6-22.6)        0.97 (0.72-1.4)
>   8 vCPUs     314 (97-2542)           7.0 (5.0-98)
> 
> Reads over 10 ms fell from 26-35 per run to none with 1 vCPU, and from
> 3-18 per run to at most one with 8 vCPUs. The benchmark and test programs
> were written with the help of an LLM.
> 
In Meta fleet, looking at lock profiler in the last day, the longest observed
mutex hold was 137.6 ms, including 137.5 ms during which the holder was runnable
but off-CPU.

Re: [PATCH 0/2] mm: zswap: reduce request contention on loads

[Nhat Pham]

On Tue, Oct 6, 2026 at 2:23 AM Usama Arif <usama.arif@linux.dev> wrote:
>
> Stores and loads share a per-CPU acomp request and mutex. A low-priority
> store can be preempted right after the compressor drops its stream
> lock, while it still holds the zswap mutex, and a higher-priority load
> on that CPU then waits for the store to run again. This follows the work
> from Sergey Senozhatsky's zram series which splits it for the same
> reason [1].
>
> Patch 1 gives compression and decompression separate requests, waits
> and mutexes, so loads no longer wait for stores, though they can still
> wait for each other. Patch 2 decompresses with an on-stack request when
> the algorithm is synchronous and needs no request context, which covers
> all in-tree software compressors, so those loads take no zswap lock.
> Asynchronous algorithms keep the per-CPU request and mutex. For software
> compressors the series allocates the same number of requests as before;
> each per-CPU context grows by 72 bytes, and the load path is about 270
> bytes deeper on x86-64.
>
> The series does not fix two related cases:
> - Stores still serialize on the compression mutex, so a high-priority
>   task that reclaims (direct reclaim, MADV_PAGEOUT) can still wait for
>   a preempted store.

Any reasons why we cannot tackle this too? Or just one at a time?

> - On PREEMPT_RT the codec stream locks are preemptible, so a load can
>   still wait for a preempted store inside the codec.

Acked.

>
> The numbers below are the slowest read per run, as a median (min-max)
> of 5 runs. Each run is 12 seconds in a zstd VM with lazy preemption,
> vm.page-cluster=0 and swap on /dev/ram0. With 1 vCPU, four nice +10
> workers page memory out and read it back while a nice 0 task spins. A
> nice -19 reader pages out its own buffer and measures how long each
> read of it takes. With 8 vCPUs there are 16 workers, 8 spinning tasks
> and 8 readers.
>
>               Before series (ms)      With series (ms)
>   1 vCPU      22.3 (21.6-22.6)        0.97 (0.72-1.4)
>   8 vCPUs     314 (97-2542)           7.0 (5.0-98)
>
> Reads over 10 ms fell from 26-35 per run to none with 1 vCPU, and from
> 3-18 per run to at most one with 8 vCPUs. The benchmark and test programs
> were written with the help of an LLM.

Great find, Usama!

Re: [PATCH 0/2] mm: zswap: reduce request contention on loads

[Usama Arif]

On 06/10/2026 11:35, Nhat Pham wrote:
> On Tue, Oct 6, 2026 at 2:23 AM Usama Arif <usama.arif@linux.dev> wrote:
>>
>> Stores and loads share a per-CPU acomp request and mutex. A low-priority
>> store can be preempted right after the compressor drops its stream
>> lock, while it still holds the zswap mutex, and a higher-priority load
>> on that CPU then waits for the store to run again. This follows the work
>> from Sergey Senozhatsky's zram series which splits it for the same
>> reason [1].
>>
>> Patch 1 gives compression and decompression separate requests, waits
>> and mutexes, so loads no longer wait for stores, though they can still
>> wait for each other. Patch 2 decompresses with an on-stack request when
>> the algorithm is synchronous and needs no request context, which covers
>> all in-tree software compressors, so those loads take no zswap lock.
>> Asynchronous algorithms keep the per-CPU request and mutex. For software
>> compressors the series allocates the same number of requests as before;
>> each per-CPU context grows by 72 bytes, and the load path is about 270
>> bytes deeper on x86-64.
>>
>> The series does not fix two related cases:
>> - Stores still serialize on the compression mutex, so a high-priority
>>   task that reclaims (direct reclaim, MADV_PAGEOUT) can still wait for
>>   a preempted store.
> 
> Any reasons why we cannot tackle this too? Or just one at a time?

Stores need more than a request. The compression mutex also protects
the per-CPU PAGE_SIZE output buffer, which has to stay ours until
zs_obj_write() copies it out, since zs_malloc() needs the compressed
length first. Loads stopped using that buffer in e2c3b6b21c77f, so an
on-stack request was enough for them, but the buffer is too big for
the stack.

> 
>> - On PREEMPT_RT the codec stream locks are preemptible, so a load can
>>   still wait for a preempted store inside the codec.
> 
> Acked.
> 
>>
>> The numbers below are the slowest read per run, as a median (min-max)
>> of 5 runs. Each run is 12 seconds in a zstd VM with lazy preemption,
>> vm.page-cluster=0 and swap on /dev/ram0. With 1 vCPU, four nice +10
>> workers page memory out and read it back while a nice 0 task spins. A
>> nice -19 reader pages out its own buffer and measures how long each
>> read of it takes. With 8 vCPUs there are 16 workers, 8 spinning tasks
>> and 8 readers.
>>
>>               Before series (ms)      With series (ms)
>>   1 vCPU      22.3 (21.6-22.6)        0.97 (0.72-1.4)
>>   8 vCPUs     314 (97-2542)           7.0 (5.0-98)
>>
>> Reads over 10 ms fell from 26-35 per run to none with 1 vCPU, and from
>> 3-18 per run to at most one with 8 vCPUs. The benchmark and test programs
>> were written with the help of an LLM.
> 
> Great find, Usama!

Thanks for the reviews!