Re: [PATCH v3 10/11] zram: use rw_semaphore for decompression streams

[Bo Zhang]

On Thu, 15 Oct 2026 12:19:49 +0900 Sergey Senozhatsky <senozhatsky@xxxxxxxxxxxx> wrote:
> Or perhaps we can avoid vm_map/unmap.  A slightly different approach.
> kmalloc on fast path + memcpy (zsmalloc objects are strictly less than
> PAGE_SIZE bytes)

I applied the kmalloc+memcpy variant on-device and re-ran the same
Android A/B. It removes the exclusive path entirely in our workload.

                      9+10       9+10 + kmalloc fastpath
  read contended      ~1,959     0
  read wait_ns total  ~12 s      0
  read wait_ns max    72-96 ms   0
  shared              76.7%      100%
  exclusive           23.3%      0%
  dpin_contended      0          0

Every decompression now takes the shared (down_read) path, including the
cross-page objects that previously had to go exclusive. The ~1,959
residual contentions on the exclusive path are gone, and pin contention
stays at zero, so the full read side runs concurrently with no stream
contention.

Read-side contention end to end:

                 baseline   9+10      9+10 + kmalloc
  read contended 24,749     ~1,959    0
  shared         0%         76.7%     100%

This is a stateless-backend result (lzo-rle). zstd/deflate have
create_dctx and stay exclusive, so they don't benefit from shared reads
at all - which, as you noted, is the sad part about zstd on that list.

For those backends the serialization is unavoidable, but the actual
root cause of the long waits is still the holder being preempted or
migrated while holding the stream. So on top of 9+10+kmalloc I tried
a quick-and-dirty preempt-disable for synchronous backends. Allocation
can no longer sleep under the stream, so a two-stage zram_zs_malloc()
does a non-sleeping attempt first and, on failure, bounces into a kmalloc()
buffer, drops the stream and retries with a sleeping GFP_NOIO.

zstd backend, same A/B workload, on top of 9+10+kmalloc:

                       9+10+kmalloc   + preempt-disable
  cwrite_contended     14,880         48          (-99.7%)
  cwrite_wait_ns_total 137 s          0.32 s      (-99.8%)
  cwrite_wait_ns_max   243 ms         43 ms       (-82%)
  dexcl_contended      15,919         54          (-99.7%)
  dexcl_wait_ns_total  109.6 s        0.68 s      (-99.4%)
  dexcl_wait_ns_max    145 ms         34.6 ms     (-76%)

zstd stays 100% exclusive either way; the win comes purely from the
holder no longer being preempted/migrated, so the hold time stays short.
It's the same mechanism that took lzo-rle read contention from 24,749 to
406 earlier. So the two are complementary: shared reads remove
reader-vs-reader queuing for stateless backends, and preempt-disable
covers the exclusive/write serialization that stateful backends can't
avoid.

Quick and dirty, for discussion only - Patch below.

---
 drivers/block/zram/zcomp.c    | 17 +++++++++++---
 drivers/block/zram/zram_drv.c | 52 ++++++++++++++++++++++++++++++-------------
 2 files changed, 51 insertions(+), 18 deletions(-)

diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
index bb2a8101445a..5c31bd58c616 100644
--- a/drivers/block/zram/zcomp.c
+++ b/drivers/block/zram/zcomp.c
@@ -165,8 +165,11 @@ struct zcomp_cstrm *zcomp_cstrm_get(struct zcomp *comp)
 		 * so then unlock and re-try on the current CPU.
 		 */
 		mutex_lock(&zstrm->lock);
-		if (zstrm->buffer)
+		if (zstrm->buffer) {
+			if (!IS_ENABLED(CONFIG_PREEMPT_RT))
+				preempt_disable();
 			return zstrm;
+		}
 		mutex_unlock(&zstrm->lock);
 	}
 }
@@ -174,6 +177,8 @@ struct zcomp_cstrm *zcomp_cstrm_get(struct zcomp *comp)
 void zcomp_cstrm_put(struct zcomp_cstrm *zstrm)
 {
 	mutex_unlock(&zstrm->lock);
+	if (!IS_ENABLED(CONFIG_PREEMPT_RT))
+		preempt_enable();
 }
 
 bool zcomp_shared_decomp(struct zcomp *comp)
@@ -207,16 +212,22 @@ struct zcomp_dstrm *zcomp_dstrm_get(struct zcomp *comp,
 		 * Exclusive requests additionally own ->local_copy and
 		 * ->ctx and hence can only wait for each other.
 		 */
-		if (mode == ZCOMP_DSTRM_EXCLUSIVE)
+		if (mode == ZCOMP_DSTRM_EXCLUSIVE) {
 			mutex_lock(&zstrm->excl_lock);
+			if (!IS_ENABLED(CONFIG_PREEMPT_RT))
+				preempt_disable();
+		}
 		return zstrm;
 	}
 }
 
 void zcomp_dstrm_put(struct zcomp_dstrm *zstrm, enum zcomp_dstrm_mode mode)
 {
-	if (mode == ZCOMP_DSTRM_EXCLUSIVE)
+	if (mode == ZCOMP_DSTRM_EXCLUSIVE) {
 		mutex_unlock(&zstrm->excl_lock);
+		if (!IS_ENABLED(CONFIG_PREEMPT_RT))
+			preempt_enable();
+	}
 	up_read(&zstrm->lock);
 }
 
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index b429c52cb37d..079796b17bde 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -2297,13 +2297,37 @@ static int write_incompressible_page(struct zram *zram, struct page *page,
 	return 0;
 }
 
+static unsigned long zram_zs_malloc(struct zram *zram, struct zcomp_cstrm *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);
+
+	/* Drop the stream (re-enables preemption) before a sleeping alloc. */
+	zcomp_cstrm_put(zstrm);
+	handle = zs_malloc(zram->mem_pool, comp_len,
+			GFP_NOIO | __GFP_NOWARN |
+			__GFP_HIGHMEM | __GFP_MOVABLE, nid);
+	return handle;
+}
+
 static int zram_write_page(struct zram *zram, struct page *page,
 			   unsigned long index)
 {
 	int ret = 0;
 	unsigned long handle;
 	unsigned int comp_len;
-	void *mem;
+	void *mem, *bounce = NULL;
 	struct zcomp_cstrm *zstrm;
 	unsigned long element;
 	bool same_filled;
@@ -2331,22 +2355,20 @@ static int zram_write_page(struct zram *zram, struct page *page,
 		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_cstrm_put(zstrm);
+		bounce ? kfree(bounce) : zcomp_cstrm_put(zstrm);
 		return PTR_ERR((void *)handle);
 	}
 
 	if (!zram_can_store_page(zram)) {
-		zcomp_cstrm_put(zstrm);
+		bounce ? kfree(bounce) : zcomp_cstrm_put(zstrm);
 		zs_free(zram->mem_pool, handle);
 		return -ENOMEM;
 	}
 
-	zs_obj_write(zram->mem_pool, handle, zstrm->buffer, comp_len);
-	zcomp_cstrm_put(zstrm);
+	zs_obj_write(zram->mem_pool, handle, bounce ? bounce : zstrm->buffer, comp_len);
+	bounce ? kfree(bounce) : zcomp_cstrm_put(zstrm);
 
 	slot_lock(zram, index);
 	slot_free(zram, index);
@@ -2463,6 +2485,7 @@ static int recompress_slot(struct zram *zram, unsigned long index,
 	unsigned int class_index_old;
 	unsigned int class_index_new;
 	void *src;
+	void *bounce = NULL;
 	int ret = 0;
 
 	handle_old = get_slot_handle(zram, index);
@@ -2535,17 +2558,16 @@ static int recompress_slot(struct zram *zram, unsigned long index,
 	 * 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, zstrm, comp_len_new,
+				    page_to_nid(page), &bounce);
 	if (IS_ERR_VALUE(handle_new)) {
-		zcomp_cstrm_put(zstrm);
+		bounce ? kfree(bounce) : zcomp_cstrm_put(zstrm);
 		return PTR_ERR((void *)handle_new);
 	}
 
-	zs_obj_write(zram->mem_pool, handle_new, zstrm->buffer, comp_len_new);
-	zcomp_cstrm_put(zstrm);
+	zs_obj_write(zram->mem_pool, handle_new,
+		     bounce ? bounce : zstrm->buffer, comp_len_new);
+	bounce ? kfree(bounce) : zcomp_cstrm_put(zstrm);
 
 	slot_free(zram, index);
 	set_slot_handle(zram, index, handle_new);

Thanks,
Bo