[RFC PATCH] zram: avoid preemption with CPU-based compression backends

[RFC PATCH] zram: avoid preemption with CPU-based compression backends

[Barry Song (Xiaomi)]

Since commit 2efa9e9eb4db ("zram: permit preemption with active
compression stream"), a major Android regression has been reported.

The reason is that compression/decompression is now sleepable and
preemptible. This means a stream may be migrated to another CPU or
be preempted while holding the stream mutex. As a result, high
priority UI threads may get stuck waiting for the mutex during swap-in.

The worst case is when a stream is migrated from a big core to a
little core. On the little core, the stream can be repeatedly
preempted by other threads, while a high priority task running on a
big core is blocked waiting for the mutex to perform decompression
and swap-in.

We add an async flag (currently false for almost all backends) to
indicate whether a backend is asynchronous. For synchronous
backends, we use preempt_disable() in the !PREEMPT_RT case. A
zram_zs_malloc() wrapper is provided to support a two-stage
zs_malloc() path, allowing allocation to transition from a
non-sleepable context to a sleepable context.

Fixes: 2efa9e9eb4db ("zram: permit preemption with active compression stream")
Reported-by: Dongdong Zhang <zhangdongdong5@xiaomi.com>
Signed-off-by: Barry Song (Xiaomi) <baohua@kernel.org>
---
 drivers/block/zram/zcomp.c    | 14 ++++--
 drivers/block/zram/zcomp.h    |  3 +-
 drivers/block/zram/zram_drv.c | 87 +++++++++++++++++++----------------
 3 files changed, 61 insertions(+), 43 deletions(-)

diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
index 974c4691887e..dd4db4a8af73 100644
--- a/drivers/block/zram/zcomp.c
+++ b/drivers/block/zram/zcomp.c
@@ -128,14 +128,20 @@ struct zcomp_strm *zcomp_stream_get(struct zcomp *comp)
 		 * so then unlock and re-try on the current CPU.
 		 */
 		mutex_lock(&zstrm->lock);
+		if (!comp->ops->async && !IS_ENABLED(CONFIG_PREEMPT_RT))
+			preempt_disable();
 		if (likely(zstrm->buffer))
 			return zstrm;
 		mutex_unlock(&zstrm->lock);
 	}
 }
 
-void zcomp_stream_put(struct zcomp_strm *zstrm)
+void zcomp_stream_put(struct zcomp *comp)
 {
+	struct zcomp_strm *zstrm = raw_cpu_ptr(comp->stream);
+
+	if (!comp->ops->async && !IS_ENABLED(CONFIG_PREEMPT_RT))
+		preempt_enable();
 	mutex_unlock(&zstrm->lock);
 }
 
@@ -150,7 +156,8 @@ int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm,
 	};
 	int ret;
 
-	might_sleep();
+	if (comp->ops->async)
+		might_sleep();
 	ret = comp->ops->compress(comp->params, &zstrm->ctx, &req);
 	if (!ret)
 		*dst_len = req.dst_len;
@@ -167,7 +174,8 @@ int zcomp_decompress(struct zcomp *comp, struct zcomp_strm *zstrm,
 		.dst_len = PAGE_SIZE,
 	};
 
-	might_sleep();
+	if (comp->ops->async)
+		might_sleep();
 	return comp->ops->decompress(comp->params, &zstrm->ctx, &req);
 }
 
diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
index 81a0f3f6ff48..5293d638da8f 100644
--- a/drivers/block/zram/zcomp.h
+++ b/drivers/block/zram/zcomp.h
@@ -67,6 +67,7 @@ struct zcomp_ops {
 	void (*release_params)(struct zcomp_params *params);
 
 	const char *name;
+	bool async;
 };
 
 /* dynamic per-device compression frontend */
@@ -86,7 +87,7 @@ struct zcomp *zcomp_create(const char *alg, struct zcomp_params *params);
 void zcomp_destroy(struct zcomp *comp);
 
 struct zcomp_strm *zcomp_stream_get(struct zcomp *comp);
-void zcomp_stream_put(struct zcomp_strm *zstrm);
+void zcomp_stream_put(struct zcomp *comp);
 
 int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm,
 		   const void *src, unsigned int *dst_len);
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index cfa98846ac48..52b9fbe3e0d1 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -1364,7 +1364,7 @@ static int decompress_bdev_page(struct zram *zram, struct page *page, u32 index)
 	if (!ret)
 		copy_page(src, zstrm->local_copy);
 	kunmap_local(src);
-	zcomp_stream_put(zstrm);
+	zcomp_stream_put(zram->comps[prio]);
 	slot_unlock(zram, index);
 
 	return ret;
@@ -2098,7 +2098,7 @@ static int read_compressed_page(struct zram *zram, struct page *page, u32 index)
 	ret = zcomp_decompress(zram->comps[prio], zstrm, src, size, dst);
 	kunmap_local(dst);
 	zs_obj_read_end(zram->mem_pool, handle, size, src);
-	zcomp_stream_put(zstrm);
+	zcomp_stream_put(zram->comps[prio]);
 
 	return ret;
 }
@@ -2124,7 +2124,7 @@ static int read_from_zspool_raw(struct zram *zram, struct page *page, u32 index)
 				zstrm->local_copy);
 	memcpy_to_page(page, 0, src, size);
 	zs_obj_read_end(zram->mem_pool, handle, size, src);
-	zcomp_stream_put(zstrm);
+	zcomp_stream_put(zram->comps[ZRAM_PRIMARY_COMP]);
 
 	memzero_page(page, size, PAGE_SIZE - size);
 
@@ -2218,20 +2218,40 @@ static int write_same_filled_page(struct zram *zram, unsigned long fill,
 	return 0;
 }
 
+/*
+ * try non-sleepable allocation for !async backend, then try
+ * sleepable allocation; for async backend, we always begin
+ * from sleepable allocation
+ */
+static unsigned long zram_zs_malloc(struct zram *zram, size_t comp_len,
+		bool async, const int nid)
+{
+	unsigned long handle;
+
+	if (!async && !IS_ENABLED(CONFIG_PREEMPT_RT)) {
+		handle = zs_malloc(zram->mem_pool, comp_len,
+				__GFP_KSWAPD_RECLAIM | __GFP_NOWARN |
+				__GFP_HIGHMEM | __GFP_MOVABLE, nid);
+		if (!IS_ERR_VALUE(handle))
+			return handle;
+		preempt_enable();
+	}
+
+	handle = zs_malloc(zram->mem_pool, comp_len,
+			   GFP_NOIO | __GFP_NOWARN |
+			   __GFP_HIGHMEM | __GFP_MOVABLE, nid);
+	if (!async && !IS_ENABLED(CONFIG_PREEMPT_RT))
+		preempt_disable();
+	return handle;
+}
+
 static int write_incompressible_page(struct zram *zram, struct page *page,
-				     u32 index)
+				     u32 index, bool async)
 {
 	unsigned long handle;
 	void *src;
 
-	/*
-	 * This function is called from preemptible context so we don't need
-	 * to do optimistic and fallback to pessimistic handle allocation,
-	 * like we do for compressible pages.
-	 */
-	handle = zs_malloc(zram->mem_pool, PAGE_SIZE,
-			   GFP_NOIO | __GFP_NOWARN |
-			   __GFP_HIGHMEM | __GFP_MOVABLE, page_to_nid(page));
+	handle = zram_zs_malloc(zram, PAGE_SIZE, async, page_to_nid(page));
 	if (IS_ERR_VALUE(handle))
 		return PTR_ERR((void *)handle);
 
@@ -2268,6 +2288,7 @@ static int zram_write_page(struct zram *zram, struct page *page, u32 index)
 	struct zcomp_strm *zstrm;
 	unsigned long element;
 	bool same_filled;
+	bool async;
 
 	mem = kmap_local_page(page);
 	same_filled = page_same_filled(mem, &element);
@@ -2275,6 +2296,7 @@ static int zram_write_page(struct zram *zram, struct page *page, u32 index)
 	if (same_filled)
 		return write_same_filled_page(zram, element, index);
 
+	async = zram->comps[ZRAM_PRIMARY_COMP]->ops->async;
 	zstrm = zcomp_stream_get(zram->comps[ZRAM_PRIMARY_COMP]);
 	mem = kmap_local_page(page);
 	ret = zcomp_compress(zram->comps[ZRAM_PRIMARY_COMP], zstrm,
@@ -2282,32 +2304,30 @@ static int zram_write_page(struct zram *zram, struct page *page, u32 index)
 	kunmap_local(mem);
 
 	if (unlikely(ret)) {
-		zcomp_stream_put(zstrm);
+		zcomp_stream_put(zram->comps[ZRAM_PRIMARY_COMP]);
 		pr_err("Compression failed! err=%d\n", ret);
 		return ret;
 	}
 
 	if (comp_len >= huge_class_size) {
-		zcomp_stream_put(zstrm);
-		return write_incompressible_page(zram, page, index);
+		zcomp_stream_put(zram->comps[ZRAM_PRIMARY_COMP]);
+		return write_incompressible_page(zram, page, index, async);
 	}
 
-	handle = zs_malloc(zram->mem_pool, comp_len,
-			   GFP_NOIO | __GFP_NOWARN |
-			   __GFP_HIGHMEM | __GFP_MOVABLE, page_to_nid(page));
+	handle = zram_zs_malloc(zram, comp_len, async, page_to_nid(page));
 	if (IS_ERR_VALUE(handle)) {
-		zcomp_stream_put(zstrm);
+		zcomp_stream_put(zram->comps[ZRAM_PRIMARY_COMP]);
 		return PTR_ERR((void *)handle);
 	}
 
 	if (!zram_can_store_page(zram)) {
-		zcomp_stream_put(zstrm);
+		zcomp_stream_put(zram->comps[ZRAM_PRIMARY_COMP]);
 		zs_free(zram->mem_pool, handle);
 		return -ENOMEM;
 	}
 
 	zs_obj_write(zram->mem_pool, handle, zstrm->buffer, comp_len);
-	zcomp_stream_put(zstrm);
+	zcomp_stream_put(zram->comps[ZRAM_PRIMARY_COMP]);
 
 	slot_lock(zram, index);
 	slot_free(zram, index);
@@ -2423,6 +2443,7 @@ static int recompress_slot(struct zram *zram, u32 index, struct page *page,
 	unsigned int class_index_old;
 	unsigned int class_index_new;
 	void *src;
+	bool async;
 	int ret = 0;
 
 	handle_old = get_slot_handle(zram, index);
@@ -2447,6 +2468,7 @@ static int recompress_slot(struct zram *zram, u32 index, struct page *page,
 	 */
 	clear_slot_flag(zram, index, ZRAM_IDLE);
 
+	async = zram->comps[prio]->ops->async;
 	zstrm = zcomp_stream_get(zram->comps[prio]);
 	src = kmap_local_page(page);
 	ret = zcomp_compress(zram->comps[prio], zstrm, src, &comp_len_new);
@@ -2462,7 +2484,7 @@ static int recompress_slot(struct zram *zram, u32 index, struct page *page,
 		*num_recomp_pages -= 1;
 
 	if (ret) {
-		zcomp_stream_put(zstrm);
+		zcomp_stream_put(zram->comps[prio]);
 		return ret;
 	}
 
@@ -2471,7 +2493,7 @@ static int recompress_slot(struct zram *zram, u32 index, struct page *page,
 
 	if (class_index_new >= class_index_old ||
 	    (threshold && comp_len_new >= threshold)) {
-		zcomp_stream_put(zstrm);
+		zcomp_stream_put(zram->comps[prio]);
 
 		/*
 		 * Secondary algorithms failed to re-compress the page
@@ -2485,27 +2507,14 @@ static int recompress_slot(struct zram *zram, u32 index, struct page *page,
 		return 0;
 	}
 
-	/*
-	 * We are holding per-CPU stream mutex and entry lock so better
-	 * avoid direct reclaim.  Allocation error is not fatal since
-	 * we still have the old object in the mem_pool.
-	 *
-	 * XXX: technically, the node we really want here is the node that
-	 * holds the original compressed data. But that would require us to
-	 * modify zsmalloc API to return this information. For now, we will
-	 * make do with the node of the page allocated for recompression.
-	 */
-	handle_new = zs_malloc(zram->mem_pool, comp_len_new,
-			       GFP_NOIO | __GFP_NOWARN |
-			       __GFP_HIGHMEM | __GFP_MOVABLE,
-			       page_to_nid(page));
+	handle_new = zram_zs_malloc(zram, comp_len_new, async, page_to_nid(page));
 	if (IS_ERR_VALUE(handle_new)) {
-		zcomp_stream_put(zstrm);
+		zcomp_stream_put(zram->comps[prio]);
 		return PTR_ERR((void *)handle_new);
 	}
 
 	zs_obj_write(zram->mem_pool, handle_new, zstrm->buffer, comp_len_new);
-	zcomp_stream_put(zstrm);
+	zcomp_stream_put(zram->comps[prio]);
 
 	slot_free(zram, index);
 	set_slot_handle(zram, index, handle_new);
-- 
2.39.3 (Apple Git-146)

Re: [RFC PATCH] zram: avoid preemption with CPU-based compression backends

[Sergey Senozhatsky]

On (26/08/05 08:55), Barry Song (Xiaomi) wrote:
> Since commit 2efa9e9eb4db ("zram: permit preemption with active
> compression stream"), a major Android regression has been reported.
> 
> The reason is that compression/decompression is now sleepable and
> preemptible. This means a stream may be migrated to another CPU or
> be preempted while holding the stream mutex. As a result, high
> priority UI threads may get stuck waiting for the mutex during swap-in.
[..]
> We add an async flag (currently false for almost all backends) to
> indicate whether a backend is asynchronous. For synchronous
> backends, we use preempt_disable() in the !PREEMPT_RT case. A

I wonder what does that report say.  Is that what I think it is
(we discussed something RT related privately recently)?

Re: [RFC PATCH] zram: avoid preemption with CPU-based compression backends

[Barry Song]

On Wed, Aug 5, 2026 at 9:27 AM Sergey Senozhatsky
<senozhatsky@chromium.org> wrote:
>
> On (26/08/05 08:55), Barry Song (Xiaomi) wrote:
> > Since commit 2efa9e9eb4db ("zram: permit preemption with active
> > compression stream"), a major Android regression has been reported.
> >
> > The reason is that compression/decompression is now sleepable and
> > preemptible. This means a stream may be migrated to another CPU or
> > be preempted while holding the stream mutex. As a result, high
> > priority UI threads may get stuck waiting for the mutex during swap-in.
> [..]
> > We add an async flag (currently false for almost all backends) to
> > indicate whether a backend is asynchronous. For synchronous
> > backends, we use preempt_disable() in the !PREEMPT_RT case. A
>
> I wonder what does that report say.  Is that what I think it is
> (we discussed something RT related privately recently)?

This report shows that the zram mutex has become the top lock
contributing to UI frame drops, even surpassing mmap_lock, which we
are also addressing in multiple threads. :-)

Locally, we have reverted the patch, but for upstream we still need
to find a proper solution.

Re: [RFC PATCH] zram: avoid preemption with CPU-based compression backends

[Sergey Senozhatsky]

Cc-ing Suleiman

On (26/08/05 09:57), Barry Song wrote:
> > On (26/08/05 08:55), Barry Song (Xiaomi) wrote:
> > > Since commit 2efa9e9eb4db ("zram: permit preemption with active
> > > compression stream"), a major Android regression has been reported.
> > >
> > > The reason is that compression/decompression is now sleepable and
> > > preemptible. This means a stream may be migrated to another CPU or
> > > be preempted while holding the stream mutex. As a result, high
> > > priority UI threads may get stuck waiting for the mutex during swap-in.
> > [..]
> > > We add an async flag (currently false for almost all backends) to
> > > indicate whether a backend is asynchronous. For synchronous
> > > backends, we use preempt_disable() in the !PREEMPT_RT case. A
> >
> > I wonder what does that report say.  Is that what I think it is
> > (we discussed something RT related privately recently)?
> 
> This report shows that the zram mutex has become the top lock
> contributing to UI frame drops, even surpassing mmap_lock, which we
> are also addressing in multiple threads. :-)

Any chance you can share more details?  Are there perhaps RT tasks
in the mix, priority inversion, starvations and so on?  Can proxy
execution address any of those (if it has relevance to the report
you are looking at)?

Re: [RFC PATCH] zram: avoid preemption with CPU-based compression backends

[Barry Song]

On Wed, Aug 5, 2026 at 2:09 PM Sergey Senozhatsky
<senozhatsky@chromium.org> wrote:
>
> Cc-ing Suleiman
>
> On (26/08/05 09:57), Barry Song wrote:
> > > On (26/08/05 08:55), Barry Song (Xiaomi) wrote:
> > > > Since commit 2efa9e9eb4db ("zram: permit preemption with active
> > > > compression stream"), a major Android regression has been reported.
> > > >
> > > > The reason is that compression/decompression is now sleepable and
> > > > preemptible. This means a stream may be migrated to another CPU or
> > > > be preempted while holding the stream mutex. As a result, high
> > > > priority UI threads may get stuck waiting for the mutex during swap-in.
> > > [..]
> > > > We add an async flag (currently false for almost all backends) to
> > > > indicate whether a backend is asynchronous. For synchronous
> > > > backends, we use preempt_disable() in the !PREEMPT_RT case. A
> > >
> > > I wonder what does that report say.  Is that what I think it is
> > > (we discussed something RT related privately recently)?
> >
> > This report shows that the zram mutex has become the top lock
> > contributing to UI frame drops, even surpassing mmap_lock, which we
> > are also addressing in multiple threads. :-)
>
> Any chance you can share more details?  Are there perhaps RT tasks
> in the mix, priority inversion, starvations and so on?  Can proxy
> execution address any of those (if it has relevance to the report
> you are looking at)?

Hi Sergey,

talked with our engineers reporting the issue. i believe it is all
about priority inversion.
proxy execution wont resolve it as we have a sleepable zs-malloc
within the mutex.
 i believe i need v2 to release the mutex before doing the 2nd stage
zs_malloc with
direct reclaim.

proxy execution could only defer the preemption for the compression
and decompression, it wont address the potential long sleep of
zs-malloc. we should not do sleepable zs-malloc
within the mutex.

i will send v2 with more details.

Best Regards
Barry

Re: [RFC PATCH] zram: avoid preemption with CPU-based compression backends

[Sergey Senozhatsky]

Hi Barry,

On (26/08/05 17:09), Barry Song wrote:
> > > This report shows that the zram mutex has become the top lock
> > > contributing to UI frame drops, even surpassing mmap_lock, which we
> > > are also addressing in multiple threads. :-)
> >
> > Any chance you can share more details?  Are there perhaps RT tasks
> > in the mix, priority inversion, starvations and so on?  Can proxy
> > execution address any of those (if it has relevance to the report
> > you are looking at)?
> 
> Hi Sergey,
> 
> talked with our engineers reporting the issue. i believe it is all
> about priority inversion.
> proxy execution wont resolve it as we have a sleepable zs-malloc
> within the mutex.
>  i believe i need v2 to release the mutex before doing the 2nd stage
> zs_malloc with
> direct reclaim.

Well, we cannot just drop the stream mutex and do sleepable zsmalloc
allocation, because this will invalidate compression buffer.  So we
then will need to do re-compression.  Something that I was really
happy to drop [1].

Is there any we can do apart from making zram and zsmalloc atomic
again?  It's hard to believe that this priority inversion hits
only zram and no other locks in the system.

I really really really don't want to return back to atomic
zram/zsmalloc.

Is the report you are talking about some test or is it a real
world scenario?

[1] https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/block/zram/zram_drv.c?h=v6.1.180#n1376

Re: [RFC PATCH] zram: avoid preemption with CPU-based compression backends

[Barry Song]

[-- Attachment #1: Type: text/plain, Size: 3038 bytes --]

On Wed, Aug 5, 2026 at 1:21 PM Sergey Senozhatsky
<senozhatsky@chromium.org> wrote:
>
> Hi Barry,
>
> On (26/08/05 17:09), Barry Song wrote:
> > > > This report shows that the zram mutex has become the top lock
> > > > contributing to UI frame drops, even surpassing mmap_lock, which we
> > > > are also addressing in multiple threads. :-)
> > >
> > > Any chance you can share more details?  Are there perhaps RT tasks
> > > in the mix, priority inversion, starvations and so on?  Can proxy
> > > execution address any of those (if it has relevance to the report
> > > you are looking at)?
> >
> > Hi Sergey,
> >
> > talked with our engineers reporting the issue. i believe it is all
> > about priority inversion.
> > proxy execution wont resolve it as we have a sleepable zs-malloc
> > within the mutex.
> >  i believe i need v2 to release the mutex before doing the 2nd stage
> > zs_malloc with
> > direct reclaim.
>
> Well, we cannot just drop the stream mutex and do sleepable zsmalloc
> allocation, because this will invalidate compression buffer.  So we
> then will need to do re-compression.  Something that I was really
> happy to drop [1].

We used to do that by an temp GFP_ATOMIC buffer and memcpy:
https://lore.kernel.org/all/1611035683-12732-2-git-send-email-tiantao6@hisilicon.com/

As long as we copy `zstrm->buffer` to a temporary buffer, we are
free to go anywhere afterwards.

BTW, I wonder if compression and decompression could use separate
mutexes. That way, a sleepable zs_malloc() in the compression path
would not block decompression, which is the more latency-sensitive
operation.

>
> Is there any we can do apart from making zram and zsmalloc atomic
> again?  It's hard to believe that this priority inversion hits
> only zram and no other locks in the system.

It affects binder, mmap, and other paths. Some of them have already
been addressed in various ways. One common approach is to move
slow operations out of critical sections.

>
> I really really really don't want to return back to atomic
> zram/zsmalloc.

Let's not go back to atomic again. :-)

From what I can see, mm/zswap.c uses a non-sleepable zs_malloc(),
which likely helps avoid this problem. If that allocation fails,
zswap can still fall back to writing the page back to disk.
For zram, however, a failure of the non-sleepable zs_malloc() would
mean the swapout() operation itself fails.

>
> Is the report you are talking about some test or is it a real
> world scenario?

It happens in real-world scenarios as well as during testing.
For example, if you flash a Pixel 6 with an image containing those
non-atomic commits, the zram mutex becomes a bottleneck. Hongru
reported it as the top source of UI frame drops on Pixel 6, as shown
in the attached zram-mutex.png.

On our OEM devices, it is also a serious problem.

>
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/block/zram/zram_drv.c?h=v6.1.180#n1376

Thanks
Barry

[-- Attachment #2: zram-mutex.png --]
[-- Type: image/png, Size: 203156 bytes --]

Re: [RFC PATCH] zram: avoid preemption with CPU-based compression backends

[Sergey Senozhatsky]

On (26/08/05 15:50), Barry Song wrote:
> > > talked with our engineers reporting the issue. i believe it is all
> > > about priority inversion.
> > > proxy execution wont resolve it as we have a sleepable zs-malloc
> > > within the mutex.
> > >  i believe i need v2 to release the mutex before doing the 2nd stage
> > > zs_malloc with
> > > direct reclaim.
> >
> > Well, we cannot just drop the stream mutex and do sleepable zsmalloc
> > allocation, because this will invalidate compression buffer.  So we
> > then will need to do re-compression.  Something that I was really
> > happy to drop [1].
> 

[..]

> BTW, I wonder if compression and decompression could use separate
> mutexes. That way, a sleepable zs_malloc() in the compression path
> would not block decompression, which is the more latency-sensitive
> operation.

This sounds interesting. I think all of the S/W backends that we use
have stateless decompression, so we probably can just split per-CPU
stream mutex for R and W paths w/o the need for any additional scratch
buffers.  Wanna give it a try?

Back to preemption:
Is there maybe a common hot preemption point where stream mutex owners
get scheduled out? E.g. inside zs_malloc()?

Re: [RFC PATCH] zram: avoid preemption with CPU-based compression backends

[Sergey Senozhatsky]

On (26/08/05 17:46), Sergey Senozhatsky wrote:
> On (26/08/05 15:50), Barry Song wrote:


> This sounds interesting. I think all of the S/W backends that we use
> have stateless decompression

OK, that doesn't seem to be true, but just for initial testing
on lzo/lz4 we can simply split the lock.  zstd/deflate need
context for decompression, this doesn't look stateless.

Re: [RFC PATCH] zram: avoid preemption with CPU-based compression backends

[Barry Song (Xiaomi)]

On Wed, Aug 5, 2026 at 3:50 PM Barry Song <baohua@kernel.org> wrote:
>
> On Wed, Aug 5, 2026 at 1:21 PM Sergey Senozhatsky
> <senozhatsky@chromium.org> wrote:
> >
> > Hi Barry,
> >
> > On (26/08/05 17:09), Barry Song wrote:
> > > > > This report shows that the zram mutex has become the top lock
> > > > > contributing to UI frame drops, even surpassing mmap_lock, which we
> > > > > are also addressing in multiple threads. :-)
> > > >
> > > > Any chance you can share more details?  Are there perhaps RT tasks
> > > > in the mix, priority inversion, starvations and so on?  Can proxy
> > > > execution address any of those (if it has relevance to the report
> > > > you are looking at)?
> > >
> > > Hi Sergey,
> > >
> > > talked with our engineers reporting the issue. i believe it is all
> > > about priority inversion.
> > > proxy execution wont resolve it as we have a sleepable zs-malloc
> > > within the mutex.
> > >  i believe i need v2 to release the mutex before doing the 2nd stage
> > > zs_malloc with
> > > direct reclaim.
> >
> > Well, we cannot just drop the stream mutex and do sleepable zsmalloc
> > allocation, because this will invalidate compression buffer.  So we
> > then will need to do re-compression.  Something that I was really
> > happy to drop [1].
>
> We used to do that by an temp GFP_ATOMIC buffer and memcpy:
> https://lore.kernel.org/all/1611035683-12732-2-git-send-email-tiantao6@hisilicon.com/
>
> As long as we copy `zstrm->buffer` to a temporary buffer, we are
> free to go anywhere afterwards.
>

Hi Sergey,

Just as a proof of concept, I changed one path and it seems to work.
We release the mutex before calling zs_malloc(), which may enter
direct reclaim, and we no longer need the mutex afterwards.
also, we can avoid re-compression:

From d1a4cbe63fc2f7336c23bc268b1dffe15b0e7444 Mon Sep 17 00:00:00 2001
From: "Barry Song (Xiaomi)" <baohua@kernel.org>
Date: Wed, 5 Aug 2026 17:53:49 +0800
Subject: [PATCH] zram: avoid doing zs_malloc() with direct reclaim within
 mutex

Signed-off-by: Barry Song (Xiaomi) <baohua@kernel.org>
---
 drivers/block/zram/zram_drv.c | 43 ++++++++++++++++++++++++++++-------
 1 file changed, 35 insertions(+), 8 deletions(-)

diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index cfa98846ac48..e00d896a101f 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -2218,6 +2218,35 @@ static int write_same_filled_page(struct zram *zram, unsigned long fill,
 	return 0;
 }
 
+/*
+ * try non-sleepable allocation for !async backend, then try
+ * sleepable allocation; for async backend, we always begin
+ * from sleepable allocation
+ */
+static unsigned long zram_zs_malloc(struct zram *zram, struct zcomp_strm *zstrm,
+		size_t comp_len, const int nid, void **bounce)
+{
+	unsigned long handle;
+
+	handle = zs_malloc(zram->mem_pool, comp_len,
+			__GFP_KSWAPD_RECLAIM | __GFP_NOWARN |
+			__GFP_HIGHMEM | __GFP_MOVABLE, nid);
+	if (!IS_ERR_VALUE(handle))
+		return handle;
+
+	*bounce = kmalloc(comp_len, GFP_ATOMIC);
+	if (!*bounce)
+		return (unsigned long)ERR_PTR(-ENOMEM);
+	memcpy(*bounce, zstrm->buffer, comp_len);
+
+	/* Don't hold mutex to do a sleepable allocation */
+	zcomp_stream_put(zstrm);
+	handle = zs_malloc(zram->mem_pool, comp_len,
+			GFP_NOIO | __GFP_NOWARN |
+			__GFP_HIGHMEM | __GFP_MOVABLE, nid);
+	return handle;
+}
+
 static int write_incompressible_page(struct zram *zram, struct page *page,
 				     u32 index)
 {
@@ -2264,7 +2293,7 @@ static int zram_write_page(struct zram *zram, struct page *page, u32 index)
 	int ret = 0;
 	unsigned long handle;
 	unsigned int comp_len;
-	void *mem;
+	void *mem, *bounce = NULL;
 	struct zcomp_strm *zstrm;
 	unsigned long element;
 	bool same_filled;
@@ -2292,22 +2321,20 @@ static int zram_write_page(struct zram *zram, struct page *page, u32 index)
 		return write_incompressible_page(zram, page, index);
 	}
 
-	handle = zs_malloc(zram->mem_pool, comp_len,
-			   GFP_NOIO | __GFP_NOWARN |
-			   __GFP_HIGHMEM | __GFP_MOVABLE, page_to_nid(page));
+	handle = zram_zs_malloc(zram, zstrm, comp_len, page_to_nid(page), &bounce);
 	if (IS_ERR_VALUE(handle)) {
-		zcomp_stream_put(zstrm);
+		bounce ? kfree(bounce) : zcomp_stream_put(zstrm);
 		return PTR_ERR((void *)handle);
 	}
 
 	if (!zram_can_store_page(zram)) {
-		zcomp_stream_put(zstrm);
+		bounce ? kfree(bounce) : zcomp_stream_put(zstrm);
 		zs_free(zram->mem_pool, handle);
 		return -ENOMEM;
 	}
 
-	zs_obj_write(zram->mem_pool, handle, zstrm->buffer, comp_len);
-	zcomp_stream_put(zstrm);
+	zs_obj_write(zram->mem_pool, handle, bounce ? : zstrm->buffer, comp_len);
+	bounce ? kfree(bounce) : zcomp_stream_put(zstrm);
 
 	slot_lock(zram, index);
 	slot_free(zram, index);
-- 
2.39.3 (Apple Git-146)

Re: [RFC PATCH] zram: avoid preemption with CPU-based compression backends

[Sergey Senozhatsky]

Hi Barry,

On (26/08/05 15:50), Barry Song wrote:
> BTW, I wonder if compression and decompression could use separate
> mutexes. That way, a sleepable zs_malloc() in the compression path
> would not block decompression, which is the more latency-sensitive
> operation.

quick and dirty patch.  Just curious if this improves anything on your
side.

We also maybe can have more that num_online_cpus() stream, if we
switch to idle streams list instead [1]

[1] https://lore.kernel.org/lkml/20250130111105.2861324-3-senozhatsky@chromium.org/

----

diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
index 974c4691887e..3c523ea0dc27 100644
--- a/drivers/block/zram/zcomp.c
+++ b/drivers/block/zram/zcomp.c
@@ -112,21 +112,28 @@ ssize_t zcomp_available_show(const char *comp, char *buf, ssize_t at)
 	return at;
 }
 
-struct zcomp_strm *zcomp_stream_get(struct zcomp *comp)
+struct zcomp_strm *zcomp_stream_get_write(struct zcomp *comp)
 {
 	for (;;) {
-		struct zcomp_strm *zstrm = raw_cpu_ptr(comp->stream);
-
-		/*
-		 * Inspired by zswap
-		 *
-		 * stream is returned with ->mutex locked which prevents
-		 * cpu_dead() from releasing this stream under us, however
-		 * there is still a race window between raw_cpu_ptr() and
-		 * mutex_lock(), during which we could have been migrated
-		 * from a CPU that has already destroyed its stream.  If
-		 * so then unlock and re-try on the current CPU.
-		 */
+		struct zcomp_strm *zstrm = raw_cpu_ptr(comp->stream_write);
+
+		mutex_lock(&zstrm->lock);
+		if (likely(zstrm->buffer))
+			return zstrm;
+		mutex_unlock(&zstrm->lock);
+	}
+}
+
+void zcomp_stream_put_write(struct zcomp_strm *zstrm)
+{
+	mutex_unlock(&zstrm->lock);
+}
+
+struct zcomp_strm *zcomp_stream_get_read(struct zcomp *comp)
+{
+	for (;;) {
+		struct zcomp_strm *zstrm = raw_cpu_ptr(comp->stream_read);
+
 		mutex_lock(&zstrm->lock);
 		if (likely(zstrm->buffer))
 			return zstrm;
@@ -134,7 +141,7 @@ struct zcomp_strm *zcomp_stream_get(struct zcomp *comp)
 	}
 }
 
-void zcomp_stream_put(struct zcomp_strm *zstrm)
+void zcomp_stream_put_read(struct zcomp_strm *zstrm)
 {
 	mutex_unlock(&zstrm->lock);
 }
@@ -174,23 +181,39 @@ int zcomp_decompress(struct zcomp *comp, struct zcomp_strm *zstrm,
 int zcomp_cpu_up_prepare(unsigned int cpu, struct hlist_node *node)
 {
 	struct zcomp *comp = hlist_entry(node, struct zcomp, node);
-	struct zcomp_strm *zstrm = per_cpu_ptr(comp->stream, cpu);
+	struct zcomp_strm *zstrm_w = per_cpu_ptr(comp->stream_write, cpu);
+	struct zcomp_strm *zstrm_r = per_cpu_ptr(comp->stream_read, cpu);
 	int ret;
 
-	ret = zcomp_strm_init(comp, zstrm);
-	if (ret)
-		pr_err("Can't allocate a compression stream\n");
-	return ret;
+	ret = zcomp_strm_init(comp, zstrm_w);
+	if (ret) {
+		pr_err("Can't allocate a compression write stream\n");
+		return ret;
+	}
+
+	ret = zcomp_strm_init(comp, zstrm_r);
+	if (ret) {
+		pr_err("Can't allocate a compression read stream\n");
+		zcomp_strm_free(comp, zstrm_w);
+		return ret;
+	}
+
+	return 0;
 }
 
 int zcomp_cpu_dead(unsigned int cpu, struct hlist_node *node)
 {
 	struct zcomp *comp = hlist_entry(node, struct zcomp, node);
-	struct zcomp_strm *zstrm = per_cpu_ptr(comp->stream, cpu);
+	struct zcomp_strm *zstrm_w = per_cpu_ptr(comp->stream_write, cpu);
+	struct zcomp_strm *zstrm_r = per_cpu_ptr(comp->stream_read, cpu);
 
-	mutex_lock(&zstrm->lock);
-	zcomp_strm_free(comp, zstrm);
-	mutex_unlock(&zstrm->lock);
+	mutex_lock(&zstrm_w->lock);
+	zcomp_strm_free(comp, zstrm_w);
+	mutex_unlock(&zstrm_w->lock);
+
+	mutex_lock(&zstrm_r->lock);
+	zcomp_strm_free(comp, zstrm_r);
+	mutex_unlock(&zstrm_r->lock);
 	return 0;
 }
 
@@ -198,17 +221,25 @@ static int zcomp_init(struct zcomp *comp, struct zcomp_params *params)
 {
 	int ret, cpu;
 
-	comp->stream = alloc_percpu(struct zcomp_strm);
-	if (!comp->stream)
+	comp->stream_write = alloc_percpu(struct zcomp_strm);
+	if (!comp->stream_write)
 		return -ENOMEM;
 
+	comp->stream_read = alloc_percpu(struct zcomp_strm);
+	if (!comp->stream_read) {
+		free_percpu(comp->stream_write);
+		return -ENOMEM;
+	}
+
 	comp->params = params;
 	ret = comp->ops->setup_params(comp->params);
 	if (ret)
 		goto cleanup;
 
-	for_each_possible_cpu(cpu)
-		mutex_init(&per_cpu_ptr(comp->stream, cpu)->lock);
+	for_each_possible_cpu(cpu) {
+		mutex_init(&per_cpu_ptr(comp->stream_write, cpu)->lock);
+		mutex_init(&per_cpu_ptr(comp->stream_read, cpu)->lock);
+	}
 
 	ret = cpuhp_state_add_instance(CPUHP_ZCOMP_PREPARE, &comp->node);
 	if (ret < 0)
@@ -218,7 +249,8 @@ static int zcomp_init(struct zcomp *comp, struct zcomp_params *params)
 
 cleanup:
 	comp->ops->release_params(comp->params);
-	free_percpu(comp->stream);
+	free_percpu(comp->stream_read);
+	free_percpu(comp->stream_write);
 	return ret;
 }
 
@@ -226,7 +258,8 @@ void zcomp_destroy(struct zcomp *comp)
 {
 	cpuhp_state_remove_instance(CPUHP_ZCOMP_PREPARE, &comp->node);
 	comp->ops->release_params(comp->params);
-	free_percpu(comp->stream);
+	free_percpu(comp->stream_read);
+	free_percpu(comp->stream_write);
 	kfree(comp);
 }
 
diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
index 81a0f3f6ff48..fd919571d8b7 100644
--- a/drivers/block/zram/zcomp.h
+++ b/drivers/block/zram/zcomp.h
@@ -71,7 +71,8 @@ struct zcomp_ops {
 
 /* dynamic per-device compression frontend */
 struct zcomp {
-	struct zcomp_strm __percpu *stream;
+	struct zcomp_strm __percpu *stream_write;
+	struct zcomp_strm __percpu *stream_read;
 	const struct zcomp_ops *ops;
 	struct zcomp_params *params;
 	struct hlist_node node;
@@ -85,8 +86,11 @@ const char *zcomp_lookup_backend_name(const char *comp);
 struct zcomp *zcomp_create(const char *alg, struct zcomp_params *params);
 void zcomp_destroy(struct zcomp *comp);
 
-struct zcomp_strm *zcomp_stream_get(struct zcomp *comp);
-void zcomp_stream_put(struct zcomp_strm *zstrm);
+struct zcomp_strm *zcomp_stream_get_write(struct zcomp *comp);
+void zcomp_stream_put_write(struct zcomp_strm *zstrm);
+
+struct zcomp_strm *zcomp_stream_get_read(struct zcomp *comp);
+void zcomp_stream_put_read(struct zcomp_strm *zstrm);
 
 int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm,
 		   const void *src, unsigned int *dst_len);
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 3b9dfcae9317..6a564a5e0041 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -1358,14 +1358,14 @@ static int decompress_bdev_page(struct zram *zram, struct page *page, u32 index)
 	size = get_slot_size(zram, index);
 	prio = get_slot_comp_priority(zram, index);
 
-	zstrm = zcomp_stream_get(zram->comps[prio]);
+	zstrm = zcomp_stream_get_read(zram->comps[prio]);
 	src = kmap_local_page(page);
 	ret = zcomp_decompress(zram->comps[prio], zstrm, src, size,
 			       zstrm->local_copy);
 	if (!ret)
 		copy_page(src, zstrm->local_copy);
 	kunmap_local(src);
-	zcomp_stream_put(zstrm);
+	zcomp_stream_put_read(zstrm);
 	slot_unlock(zram, index);
 
 	return ret;
@@ -2101,14 +2101,14 @@ static int read_compressed_page(struct zram *zram, struct page *page, u32 index)
 	size = get_slot_size(zram, index);
 	prio = get_slot_comp_priority(zram, index);
 
-	zstrm = zcomp_stream_get(zram->comps[prio]);
+	zstrm = zcomp_stream_get_read(zram->comps[prio]);
 	src = zs_obj_read_begin(zram->mem_pool, handle, size,
 				zstrm->local_copy);
 	dst = kmap_local_page(page);
 	ret = zcomp_decompress(zram->comps[prio], zstrm, src, size, dst);
 	kunmap_local(dst);
 	zs_obj_read_end(zram->mem_pool, handle, size, src);
-	zcomp_stream_put(zstrm);
+	zcomp_stream_put_read(zstrm);
 
 	return ret;
 }
@@ -2129,12 +2129,12 @@ static int read_from_zspool_raw(struct zram *zram, struct page *page, u32 index)
 	 * case if object spans two physical pages. No decompression
 	 * takes place here, as we read raw compressed data.
 	 */
-	zstrm = zcomp_stream_get(zram->comps[ZRAM_PRIMARY_COMP]);
+	zstrm = zcomp_stream_get_read(zram->comps[ZRAM_PRIMARY_COMP]);
 	src = zs_obj_read_begin(zram->mem_pool, handle, size,
 				zstrm->local_copy);
 	memcpy_to_page(page, 0, src, size);
 	zs_obj_read_end(zram->mem_pool, handle, size, src);
-	zcomp_stream_put(zstrm);
+	zcomp_stream_put_read(zstrm);
 
 	memzero_page(page, size, PAGE_SIZE - size);
 
@@ -2285,20 +2285,20 @@ static int zram_write_page(struct zram *zram, struct page *page, u32 index)
 	if (same_filled)
 		return write_same_filled_page(zram, element, index);
 
-	zstrm = zcomp_stream_get(zram->comps[ZRAM_PRIMARY_COMP]);
+	zstrm = zcomp_stream_get_write(zram->comps[ZRAM_PRIMARY_COMP]);
 	mem = kmap_local_page(page);
 	ret = zcomp_compress(zram->comps[ZRAM_PRIMARY_COMP], zstrm,
 			     mem, &comp_len);
 	kunmap_local(mem);
 
 	if (unlikely(ret)) {
-		zcomp_stream_put(zstrm);
+		zcomp_stream_put_write(zstrm);
 		pr_err("Compression failed! err=%d\n", ret);
 		return ret;
 	}
 
 	if (comp_len >= huge_class_size) {
-		zcomp_stream_put(zstrm);
+		zcomp_stream_put_write(zstrm);
 		return write_incompressible_page(zram, page, index);
 	}
 
@@ -2306,18 +2306,18 @@ static int zram_write_page(struct zram *zram, struct page *page, u32 index)
 			   GFP_NOIO | __GFP_NOWARN |
 			   __GFP_HIGHMEM | __GFP_MOVABLE, page_to_nid(page));
 	if (IS_ERR_VALUE(handle)) {
-		zcomp_stream_put(zstrm);
+		zcomp_stream_put_write(zstrm);
 		return PTR_ERR((void *)handle);
 	}
 
 	if (!zram_can_store_page(zram)) {
-		zcomp_stream_put(zstrm);
+		zcomp_stream_put_write(zstrm);
 		zs_free(zram->mem_pool, handle);
 		return -ENOMEM;
 	}
 
 	zs_obj_write(zram->mem_pool, handle, zstrm->buffer, comp_len);
-	zcomp_stream_put(zstrm);
+	zcomp_stream_put_write(zstrm);
 
 	slot_lock(zram, index);
 	slot_free(zram, index);
@@ -2457,7 +2457,7 @@ static int recompress_slot(struct zram *zram, u32 index, struct page *page,
 	 */
 	clear_slot_flag(zram, index, ZRAM_IDLE);
 
-	zstrm = zcomp_stream_get(zram->comps[prio]);
+	zstrm = zcomp_stream_get_write(zram->comps[prio]);
 	src = kmap_local_page(page);
 	ret = zcomp_compress(zram->comps[prio], zstrm, src, &comp_len_new);
 	kunmap_local(src);
@@ -2472,7 +2472,7 @@ static int recompress_slot(struct zram *zram, u32 index, struct page *page,
 		*num_recomp_pages -= 1;
 
 	if (ret) {
-		zcomp_stream_put(zstrm);
+		zcomp_stream_put_write(zstrm);
 		return ret;
 	}
 
@@ -2481,7 +2481,7 @@ static int recompress_slot(struct zram *zram, u32 index, struct page *page,
 
 	if (class_index_new >= class_index_old ||
 	    (threshold && comp_len_new >= threshold)) {
-		zcomp_stream_put(zstrm);
+		zcomp_stream_put_write(zstrm);
 
 		/*
 		 * Secondary algorithms failed to re-compress the page
@@ -2510,12 +2510,12 @@ static int recompress_slot(struct zram *zram, u32 index, struct page *page,
 			       __GFP_HIGHMEM | __GFP_MOVABLE,
 			       page_to_nid(page));
 	if (IS_ERR_VALUE(handle_new)) {
-		zcomp_stream_put(zstrm);
+		zcomp_stream_put_write(zstrm);
 		return PTR_ERR((void *)handle_new);
 	}
 
 	zs_obj_write(zram->mem_pool, handle_new, zstrm->buffer, comp_len_new);
-	zcomp_stream_put(zstrm);
+	zcomp_stream_put_write(zstrm);
 
 	slot_free(zram, index);
 	set_slot_handle(zram, index, handle_new);

Re: [RFC PATCH] zram: avoid preemption with CPU-based compression backends

[Barry Song]

On Wed, Aug 5, 2026 at 6:25 PM Sergey Senozhatsky
<senozhatsky@chromium.org> wrote:
>
> Hi Barry,
>
> On (26/08/05 15:50), Barry Song wrote:
> > BTW, I wonder if compression and decompression could use separate
> > mutexes. That way, a sleepable zs_malloc() in the compression path
> > would not block decompression, which is the more latency-sensitive
> > operation.
>
> quick and dirty patch.  Just curious if this improves anything on your
> side.
>

Thanks very much for your quick patch, Sergey.

We are going to run three experiments:

1. Use the approach I just sent, which avoids calling zs_malloc()
   with direct reclaim while holding the mutex, and extend it to
   the other zs_malloc() call sites:

https://lore.kernel.org/all/20260805100740.71994-1-baohua@kernel.org/

2. Experiment 1 plus preemption disabled.

3. Your patch, which separates the compression and decompression
   mutexes.

I'd like to understand which change has the biggest impact on
performance.

Let me gather some data, and I'll get back to you.

> We also maybe can have more that num_online_cpus() stream, if we
> switch to idle streams list instead [1]
>
> [1] https://lore.kernel.org/lkml/20250130111105.2861324-3-senozhatsky@chromium.org/
>
> ----
>
> diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
> index 974c4691887e..3c523ea0dc27 100644
> --- a/drivers/block/zram/zcomp.c
> +++ b/drivers/block/zram/zcomp.c
> @@ -112,21 +112,28 @@ ssize_t zcomp_available_show(const char *comp, char *buf, ssize_t at)
>         return at;
>  }
>
> -struct zcomp_strm *zcomp_stream_get(struct zcomp *comp)
> +struct zcomp_strm *zcomp_stream_get_write(struct zcomp *comp)
>  {
>         for (;;) {
> -               struct zcomp_strm *zstrm = raw_cpu_ptr(comp->stream);
> -
> -               /*
> -                * Inspired by zswap
> -                *
> -                * stream is returned with ->mutex locked which prevents
> -                * cpu_dead() from releasing this stream under us, however
> -                * there is still a race window between raw_cpu_ptr() and
> -                * mutex_lock(), during which we could have been migrated
> -                * from a CPU that has already destroyed its stream.  If
> -                * so then unlock and re-try on the current CPU.
> -                */
> +               struct zcomp_strm *zstrm = raw_cpu_ptr(comp->stream_write);
> +
> +               mutex_lock(&zstrm->lock);
> +               if (likely(zstrm->buffer))
> +                       return zstrm;
> +               mutex_unlock(&zstrm->lock);
> +       }
> +}
> +
> +void zcomp_stream_put_write(struct zcomp_strm *zstrm)
> +{
> +       mutex_unlock(&zstrm->lock);
> +}
> +
> +struct zcomp_strm *zcomp_stream_get_read(struct zcomp *comp)
> +{
> +       for (;;) {
> +               struct zcomp_strm *zstrm = raw_cpu_ptr(comp->stream_read);
> +
>                 mutex_lock(&zstrm->lock);
>                 if (likely(zstrm->buffer))
>                         return zstrm;
> @@ -134,7 +141,7 @@ struct zcomp_strm *zcomp_stream_get(struct zcomp *comp)
>         }
>  }
>
[...]

Best Regards
Barry

Re: [RFC PATCH] zram: avoid preemption with CPU-based compression backends

[Sergey Senozhatsky]

On (26/08/05 18:34), Barry Song wrote:
> Thanks very much for your quick patch, Sergey.
> 
> We are going to run three experiments:
> 
> 1. Use the approach I just sent, which avoids calling zs_malloc()
>    with direct reclaim while holding the mutex, and extend it to
>    the other zs_malloc() call sites:
> 
> https://lore.kernel.org/all/20260805100740.71994-1-baohua@kernel.org/
> 
> 2. Experiment 1 plus preemption disabled.
> 
> 3. Your patch, which separates the compression and decompression
>    mutexes.
> 
> I'd like to understand which change has the biggest impact on
> performance.
> 
> Let me gather some data, and I'll get back to you.

Thanks a lot!  Take your time!

Re: [RFC PATCH] zram: avoid preemption with CPU-based compression backends

[Bo Zhang]

On Wed, Aug 05, 2026 at 08:55:45AM +0800, Barry Song (Xiaomi) wrote:
> Since commit 2efa9e9eb4db ("zram: permit preemption with active
> compression stream"), a major Android regression has been reported.
>
> We add an async flag (currently false for almost all backends) to
> indicate whether a backend is asynchronous. For synchronous
> backends, we use preempt_disable() in the !PREEMPT_RT case. A
> zram_zs_malloc() wrapper is provided to support a two-stage
> zs_malloc() path, allowing allocation to transition from a
> non-sleepable context to a sleepable context.

Hi Barry,

Thanks for working on this. The priority inversion issue on Android is
real and we've hit it too.

However, I think there's a bug in the zcomp_stream_put() change.

The new zcomp_stream_put() uses raw_cpu_ptr() to find the stream:

  void zcomp_stream_put(struct zcomp *comp)
  {
      struct zcomp_strm *zstrm = raw_cpu_ptr(comp->stream);
      if (!comp->ops->async && !IS_ENABLED(CONFIG_PREEMPT_RT))
          preempt_enable();
      mutex_unlock(&zstrm->lock);
  }

It relies on the thread still being on the same CPU where
zcomp_stream_get() locked the stream. In the normal path it works
fine, because preempt is disabled the entire time.

But zram_zs_malloc() breaks this assumption in the fallback path:

  static unsigned long zram_zs_malloc(...)
  {
      if (!async && !IS_ENABLED(CONFIG_PREEMPT_RT)) {
          handle = zs_malloc(..., __GFP_KSWAPD_RECLAIM | ...);
          if (!IS_ERR_VALUE(handle))
              return handle;
          preempt_enable();        // preempt is now enabled
      }

      handle = zs_malloc(..., GFP_NOIO | ...);  // may sleep, may migrate
      if (!async && !IS_ENABLED(CONFIG_PREEMPT_RT))
          preempt_disable(); // Preempt disabled, but it may work on another CPU
      return handle;
  }

If the first zs_malloc() fails, preempt is enabled and the second
zs_malloc(GFP_NOIO) can sleep. During this window the task can migrate
to another CPU. After preempt_disable(), we continue on the new CPU.

When zcomp_stream_put() is later called, raw_cpu_ptr() returns the new
CPU's stream, which is not the one that was originally locked. Which may
cause a mutex_unlock() on the wrong CPU.

The original code didn't have this problem because zcomp_stream_put()
took the zstrm pointer directly:

  void zcomp_stream_put(struct zcomp_strm *zstrm)
  {
      mutex_unlock(&zstrm->lock);
  }

The caller always passed the saved pointer from zcomp_stream_get(),
so regardless of CPU migration, the correct mutex was always unlocked.

I think the fix is to keep passing the zstrm pointer:

  void zcomp_stream_put(struct zcomp *comp, struct zcomp_strm *zstrm)
  {
      if (!comp->ops->async && !IS_ENABLED(CONFIG_PREEMPT_RT))
          preempt_enable();
      mutex_unlock(&zstrm->lock);
  }

All callers already have zstrm available. Then It works as expected.

Bo

Re: [RFC PATCH] zram: avoid preemption with CPU-based compression backends

[Barry Song]

On Wed, Aug 5, 2026 at 10:19 AM Bo Zhang <zhangbo0325@gmail.com> wrote:
>
> On Wed, Aug 05, 2026 at 08:55:45AM +0800, Barry Song (Xiaomi) wrote:
> > Since commit 2efa9e9eb4db ("zram: permit preemption with active
> > compression stream"), a major Android regression has been reported.
> >
> > We add an async flag (currently false for almost all backends) to
> > indicate whether a backend is asynchronous. For synchronous
> > backends, we use preempt_disable() in the !PREEMPT_RT case. A
> > zram_zs_malloc() wrapper is provided to support a two-stage
> > zs_malloc() path, allowing allocation to transition from a
> > non-sleepable context to a sleepable context.
>
> Hi Barry,
>
> Thanks for working on this. The priority inversion issue on Android is
> real and we've hit it too.
>
> However, I think there's a bug in the zcomp_stream_put() change.
>
> The new zcomp_stream_put() uses raw_cpu_ptr() to find the stream:
>
>   void zcomp_stream_put(struct zcomp *comp)
>   {
>       struct zcomp_strm *zstrm = raw_cpu_ptr(comp->stream);
>       if (!comp->ops->async && !IS_ENABLED(CONFIG_PREEMPT_RT))
>           preempt_enable();
>       mutex_unlock(&zstrm->lock);
>   }
>
> It relies on the thread still being on the same CPU where
> zcomp_stream_get() locked the stream. In the normal path it works
> fine, because preempt is disabled the entire time.
>
> But zram_zs_malloc() breaks this assumption in the fallback path:
>
>   static unsigned long zram_zs_malloc(...)
>   {
>       if (!async && !IS_ENABLED(CONFIG_PREEMPT_RT)) {
>           handle = zs_malloc(..., __GFP_KSWAPD_RECLAIM | ...);
>           if (!IS_ERR_VALUE(handle))
>               return handle;
>           preempt_enable();        // preempt is now enabled
>       }
>
>       handle = zs_malloc(..., GFP_NOIO | ...);  // may sleep, may migrate
>       if (!async && !IS_ENABLED(CONFIG_PREEMPT_RT))
>           preempt_disable(); // Preempt disabled, but it may work on another CPU
>       return handle;
>   }
>
> If the first zs_malloc() fails, preempt is enabled and the second
> zs_malloc(GFP_NOIO) can sleep. During this window the task can migrate
> to another CPU. After preempt_disable(), we continue on the new CPU.
>
> When zcomp_stream_put() is later called, raw_cpu_ptr() returns the new
> CPU's stream, which is not the one that was originally locked. Which may
> cause a mutex_unlock() on the wrong CPU.
>
> The original code didn't have this problem because zcomp_stream_put()
> took the zstrm pointer directly:
>
>   void zcomp_stream_put(struct zcomp_strm *zstrm)
>   {
>       mutex_unlock(&zstrm->lock);
>   }
>
> The caller always passed the saved pointer from zcomp_stream_get(),
> so regardless of CPU migration, the correct mutex was always unlocked.
>
> I think the fix is to keep passing the zstrm pointer:
>
>   void zcomp_stream_put(struct zcomp *comp, struct zcomp_strm *zstrm)
>   {
>       if (!comp->ops->async && !IS_ENABLED(CONFIG_PREEMPT_RT))
>           preempt_enable();
>       mutex_unlock(&zstrm->lock);
>   }
>
> All callers already have zstrm available. Then It works as expected.

Good catch. You're absolutely right. We need the following
(will fix in v2):

diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
index dd4db4a8af73..b7f5127c05c8 100644
--- a/drivers/block/zram/zcomp.c
+++ b/drivers/block/zram/zcomp.c
@@ -136,10 +136,8 @@ struct zcomp_strm *zcomp_stream_get(struct zcomp *comp)
        }
 }

-void zcomp_stream_put(struct zcomp *comp)
+void zcomp_stream_put(struct zcomp_strm *zstrm, struct zcomp *comp)
 {
-       struct zcomp_strm *zstrm = raw_cpu_ptr(comp->stream);
-
        if (!comp->ops->async && !IS_ENABLED(CONFIG_PREEMPT_RT))
                preempt_enable();
        mutex_unlock(&zstrm->lock);
diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
index 5293d638da8f..aa2fd3646018 100644
--- a/drivers/block/zram/zcomp.h
+++ b/drivers/block/zram/zcomp.h
@@ -87,7 +87,7 @@ struct zcomp *zcomp_create(const char *alg, struct
zcomp_params *params);
 void zcomp_destroy(struct zcomp *comp);

 struct zcomp_strm *zcomp_stream_get(struct zcomp *comp);
-void zcomp_stream_put(struct zcomp *comp);
+void zcomp_stream_put(struct zcomp_strm *zstrm, struct zcomp *comp);

 int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm,
                   const void *src, unsigned int *dst_len);
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 52b9fbe3e0d1..8f36ca122e1c 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -1364,7 +1364,7 @@ static int decompress_bdev_page(struct zram
*zram, struct page *page, u32 index)
        if (!ret)
                copy_page(src, zstrm->local_copy);
        kunmap_local(src);
-       zcomp_stream_put(zram->comps[prio]);
+       zcomp_stream_put(zstrm, zram->comps[prio]);
        slot_unlock(zram, index);

        return ret;
@@ -2098,7 +2098,7 @@ static int read_compressed_page(struct zram
*zram, struct page *page, u32 index)
        ret = zcomp_decompress(zram->comps[prio], zstrm, src, size, dst);
        kunmap_local(dst);
        zs_obj_read_end(zram->mem_pool, handle, size, src);
-       zcomp_stream_put(zram->comps[prio]);
+       zcomp_stream_put(zstrm, zram->comps[prio]);

        return ret;
 }
@@ -2124,7 +2124,7 @@ static int read_from_zspool_raw(struct zram
*zram, struct page *page, u32 index)
                                zstrm->local_copy);
        memcpy_to_page(page, 0, src, size);
        zs_obj_read_end(zram->mem_pool, handle, size, src);
-       zcomp_stream_put(zram->comps[ZRAM_PRIMARY_COMP]);
+       zcomp_stream_put(zstrm, zram->comps[ZRAM_PRIMARY_COMP]);

        memzero_page(page, size, PAGE_SIZE - size);

@@ -2304,30 +2304,30 @@ static int zram_write_page(struct zram *zram,
struct page *page, u32 index)
        kunmap_local(mem);

        if (unlikely(ret)) {
-               zcomp_stream_put(zram->comps[ZRAM_PRIMARY_COMP]);
+               zcomp_stream_put(zstrm, zram->comps[ZRAM_PRIMARY_COMP]);
                pr_err("Compression failed! err=%d\n", ret);
                return ret;
        }

        if (comp_len >= huge_class_size) {
-               zcomp_stream_put(zram->comps[ZRAM_PRIMARY_COMP]);
+               zcomp_stream_put(zstrm, zram->comps[ZRAM_PRIMARY_COMP]);
                return write_incompressible_page(zram, page, index, async);
        }

        handle = zram_zs_malloc(zram, comp_len, async, page_to_nid(page));
        if (IS_ERR_VALUE(handle)) {
-               zcomp_stream_put(zram->comps[ZRAM_PRIMARY_COMP]);
+               zcomp_stream_put(zstrm, zram->comps[ZRAM_PRIMARY_COMP]);
                return PTR_ERR((void *)handle);
        }

        if (!zram_can_store_page(zram)) {
-               zcomp_stream_put(zram->comps[ZRAM_PRIMARY_COMP]);
+               zcomp_stream_put(zstrm, zram->comps[ZRAM_PRIMARY_COMP]);
                zs_free(zram->mem_pool, handle);
                return -ENOMEM;
        }

        zs_obj_write(zram->mem_pool, handle, zstrm->buffer, comp_len);
-       zcomp_stream_put(zram->comps[ZRAM_PRIMARY_COMP]);
+       zcomp_stream_put(zstrm, zram->comps[ZRAM_PRIMARY_COMP]);

        slot_lock(zram, index);
        slot_free(zram, index);
@@ -2484,7 +2484,7 @@ static int recompress_slot(struct zram *zram,
u32 index, struct page *page,
                *num_recomp_pages -= 1;

        if (ret) {
-               zcomp_stream_put(zram->comps[prio]);
+               zcomp_stream_put(zstrm, zram->comps[prio]);
                return ret;
        }

@@ -2493,7 +2493,7 @@ static int recompress_slot(struct zram *zram,
u32 index, struct page *page,

        if (class_index_new >= class_index_old ||
            (threshold && comp_len_new >= threshold)) {
-               zcomp_stream_put(zram->comps[prio]);
+               zcomp_stream_put(zstrm, zram->comps[prio]);

                /*
                 * Secondary algorithms failed to re-compress the page
@@ -2509,12 +2509,12 @@ static int recompress_slot(struct zram *zram,
u32 index, struct page *page,

        handle_new = zram_zs_malloc(zram, comp_len_new, async,
page_to_nid(page));
        if (IS_ERR_VALUE(handle_new)) {
-               zcomp_stream_put(zram->comps[prio]);
+               zcomp_stream_put(zstrm, zram->comps[prio]);
                return PTR_ERR((void *)handle_new);
        }

        zs_obj_write(zram->mem_pool, handle_new, zstrm->buffer, comp_len_new);
-       zcomp_stream_put(zram->comps[prio]);
+       zcomp_stream_put(zstrm, zram->comps[prio]);

        slot_free(zram, index);
        set_slot_handle(zram, index, handle_new);

Re: [RFC PATCH] zram: avoid preemption with CPU-based compression backends

[Sergey Senozhatsky]

On (26/08/05 08:55), Barry Song (Xiaomi) wrote:
> @@ -128,14 +128,20 @@ struct zcomp_strm *zcomp_stream_get(struct zcomp *comp)
>  		 * so then unlock and re-try on the current CPU.
>  		 */
>  		mutex_lock(&zstrm->lock);
> +		if (!comp->ops->async && !IS_ENABLED(CONFIG_PREEMPT_RT))
> +			preempt_disable();

We take slot_lock(), which is preemptible, under stream lock on
some paths.

>  		if (likely(zstrm->buffer))
>  			return zstrm;
>  		mutex_unlock(&zstrm->lock);
>  	}
>  }