All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Barry Song (Xiaomi)" <baohua@kernel.org>
To: baohua@kernel.org, senozhatsky@chromium.org
Cc: akpm@linux-foundation.org, bigeasy@linutronix.de,
	hdanton@sina.com, linux-kernel@vger.kernel.org,
	linux-mm@kvack.org, minchan@kernel.org, ryncsn@gmail.com,
	suleiman@google.com, surenb@google.com, yosry.ahmed@linux.dev,
	zhangdongdong5@xiaomi.com
Subject: Re: [RFC PATCH] zram: avoid preemption with CPU-based compression backends
Date: Wed,  5 Aug 2026 18:07:40 +0800	[thread overview]
Message-ID: <20260805100740.71994-1-baohua@kernel.org> (raw)
In-Reply-To: <CAGsJ_4xU5VN3abCPzACZoS92MqaUrMHn9EQ1FCJD+wy=upbbMA@mail.gmail.com>

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)


  parent reply	other threads:[~2026-08-05 10:07 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-05  0:55 [RFC PATCH] zram: avoid preemption with CPU-based compression backends Barry Song (Xiaomi)
2026-08-05  1:27 ` Sergey Senozhatsky
2026-08-05  1:57   ` Barry Song
2026-08-05  2:09     ` Sergey Senozhatsky
2026-08-05  5:09       ` Barry Song
2026-08-05  5:21         ` Sergey Senozhatsky
2026-08-05  7:50           ` Barry Song
2026-08-05  8:46             ` Sergey Senozhatsky
2026-08-05  9:01               ` Sergey Senozhatsky
2026-08-05 10:07             ` Barry Song (Xiaomi) [this message]
2026-08-05 10:25             ` Sergey Senozhatsky
2026-08-05 10:34               ` Barry Song
2026-08-05 10:37                 ` Sergey Senozhatsky
2026-08-05  2:19 ` Bo Zhang
2026-08-05  2:31   ` Barry Song
2026-08-05  7:30 ` Sergey Senozhatsky

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260805100740.71994-1-baohua@kernel.org \
    --to=baohua@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=bigeasy@linutronix.de \
    --cc=hdanton@sina.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=minchan@kernel.org \
    --cc=ryncsn@gmail.com \
    --cc=senozhatsky@chromium.org \
    --cc=suleiman@google.com \
    --cc=surenb@google.com \
    --cc=yosry.ahmed@linux.dev \
    --cc=zhangdongdong5@xiaomi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.