[PATCH] zram: do not slot_free() written-back slots

[PATCH] zram: do not slot_free() written-back slots

[Sergey Senozhatsky]

slot_free() basically completely resets the slots by clearing
all of its flags and attributes.  While zram_writeback_complete()
restores some of flags back (those that are necessary for async
read decompression) we still lose a lot of slot's metadata.  For
example, slot's ac-time, or ZRAM_INCOMPRESSIBLE.

Do not slot_free() on writeback, instead clear only those
flags/attrs that we need to clear.

Fixes: d38fab605c667 ("zram: introduce compressed data writeback")
Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
---
 drivers/block/zram/zram_drv.c | 39 +++++++++++++----------------------
 1 file changed, 14 insertions(+), 25 deletions(-)

diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 1b0e1c02ece7..5ecc4ba40e9d 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -917,9 +917,8 @@ static void zram_account_writeback_submit(struct zram *zram)
 
 static int zram_writeback_complete(struct zram *zram, struct zram_wb_req *req)
 {
-	u32 size, index = req->pps->index;
-	int err, prio;
-	bool huge;
+	u32 index = req->pps->index;
+	int err;
 
 	err = blk_status_to_errno(req->bio.bi_status);
 	if (err) {
@@ -946,28 +945,13 @@ static int zram_writeback_complete(struct zram *zram, struct zram_wb_req *req)
 		goto out;
 	}
 
-	if (zram->compressed_wb) {
-		/*
-		 * ZRAM_WB slots get freed, we need to preserve data required
-		 * for read decompression.
-		 */
-		size = get_slot_size(zram, index);
-		prio = get_slot_comp_priority(zram, index);
-		huge = test_slot_flag(zram, index, ZRAM_HUGE);
-	}
-
-	slot_free(zram, index);
-	set_slot_flag(zram, index, ZRAM_WB);
+	clear_slot_flag(zram, index, ZRAM_IDLE);
+	if (test_slot_flag(zram, index, ZRAM_HUGE))
+		atomic64_dec(&zram->stats.huge_pages);
+	atomic64_sub(get_slot_size(zram, index), &zram->stats.compr_data_size);
+	zs_free(zram->mem_pool, get_slot_handle(zram, index));
 	set_slot_handle(zram, index, req->blk_idx);
-
-	if (zram->compressed_wb) {
-		if (huge)
-			set_slot_flag(zram, index, ZRAM_HUGE);
-		set_slot_size(zram, index, size);
-		set_slot_comp_priority(zram, index, prio);
-	}
-
-	atomic64_inc(&zram->stats.pages_stored);
+	set_slot_flag(zram, index, ZRAM_WB);
 
 out:
 	slot_unlock(zram, index);
@@ -2030,8 +2014,13 @@ static void slot_free(struct zram *zram, u32 index)
 	set_slot_comp_priority(zram, index, 0);
 
 	if (test_slot_flag(zram, index, ZRAM_HUGE)) {
+		/*
+		 * Writeback completion decrements ->huge_pages but keeps
+		 * ZRAM_HUGE flag for deferred decompression path.
+		 */
+		if (!test_slot_flag(zram, index, ZRAM_WB))
+			atomic64_dec(&zram->stats.huge_pages);
 		clear_slot_flag(zram, index, ZRAM_HUGE);
-		atomic64_dec(&zram->stats.huge_pages);
 	}
 
 	if (test_slot_flag(zram, index, ZRAM_WB)) {
-- 
2.53.0.851.ga537e3e6e9-goog

Re: [PATCH] zram: do not slot_free() written-back slots

[Sergey Senozhatsky]

As a side note:

On (26/03/19 12:44), Sergey Senozhatsky wrote:
> -	slot_free(zram, index);
[..]
> -		if (huge)
> -			set_slot_flag(zram, index, ZRAM_HUGE);

Original code had a bug.  slot_free() would decrement ->huge_pages,
then writeback completion would restore the HUGE flag, so when slot
would get freed for real slot_free() would decrement ->huge_pages
again, leading to underflow.

Re: [PATCH] zram: do not slot_free() written-back slots

[Richard Chang]

I think we also need clear_slot_flag(zram, index, ZRAM_PP_SLOT)

On Thu, Mar 19, 2026 at 12:06 PM Sergey Senozhatsky
<senozhatsky@chromium.org> wrote:
>
> As a side note:
>
> On (26/03/19 12:44), Sergey Senozhatsky wrote:
> > -     slot_free(zram, index);
> [..]
> > -             if (huge)
> > -                     set_slot_flag(zram, index, ZRAM_HUGE);
>
> Original code had a bug.  slot_free() would decrement ->huge_pages,
> then writeback completion would restore the HUGE flag, so when slot
> would get freed for real slot_free() would decrement ->huge_pages
> again, leading to underflow.

Re: [PATCH] zram: do not slot_free() written-back slots

[Sergey Senozhatsky]

On (26/03/19 14:47), Richard Chang wrote:
> 
> I think we also need clear_slot_flag(zram, index, ZRAM_PP_SLOT)

release_pp_slot() does that for us.

Re: [PATCH] zram: do not slot_free() written-back slots

[Minchan Kim]

On Thu, Mar 19, 2026 at 01:05:55PM +0900, Sergey Senozhatsky wrote:
> As a side note:
> 
> On (26/03/19 12:44), Sergey Senozhatsky wrote:
> > -	slot_free(zram, index);
> [..]
> > -		if (huge)
> > -			set_slot_flag(zram, index, ZRAM_HUGE);
> 
> Original code had a bug.  slot_free() would decrement ->huge_pages,
> then writeback completion would restore the HUGE flag, so when slot
> would get freed for real slot_free() would decrement ->huge_pages
> again, leading to underflow.

This should also go into description of the patch.

Re: [PATCH] zram: do not slot_free() written-back slots

[Minchan Kim]

On Thu, Mar 19, 2026 at 12:44:56PM +0900, Sergey Senozhatsky wrote:
> slot_free() basically completely resets the slots by clearing
> all of its flags and attributes.  While zram_writeback_complete()
> restores some of flags back (those that are necessary for async
> read decompression) we still lose a lot of slot's metadata.  For
> example, slot's ac-time, or ZRAM_INCOMPRESSIBLE.
> 
> Do not slot_free() on writeback, instead clear only those
> flags/attrs that we need to clear.
> 
> Fixes: d38fab605c667 ("zram: introduce compressed data writeback")
> Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>

Acked-by: Minchan Kim <minchan@kernel.org>

With small nit about the description in previous comment,