Read slot's block id under slot-lock.  We release the
slot-lock for bdev read so, technically, slot still can
get freed in the meantime, but at least we will read
bdev block (page) that holds previous know slot data,
not from slot->handle bdev block, which can be anything
at that point.

Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
---
 drivers/block/zram/zram_drv.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index ecbd9b25dfed..1f2867cd587e 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -1994,14 +1994,14 @@ static int zram_read_page(struct zram *zram, struct page *page, u32 index,
 		ret = zram_read_from_zspool(zram, page, index);
 		zram_slot_unlock(zram, index);
 	} else {
+		unsigned long blk_idx = zram_get_handle(zram, index);
+
 		/*
 		 * The slot should be unlocked before reading from the backing
 		 * device.
 		 */
 		zram_slot_unlock(zram, index);
-
-		ret = read_from_bdev(zram, page, zram_get_handle(zram, index),
-				     parent);
+		ret = read_from_bdev(zram, page, blk_idx, parent);
 	}
 
 	/* Should NEVER happen. Return bio error if it does. */
-- 
2.52.0.rc1.455.g30608eb744-goog

On Thu, Nov 20, 2025 at 10:22 AM Sergey Senozhatsky
<senozhatsky@chromium.org> wrote:
>
> Read slot's block id under slot-lock.  We release the
> slot-lock for bdev read so, technically, slot still can
> get freed in the meantime, but at least we will read
> bdev block (page) that holds previous know slot data,
> not from slot->handle bdev block, which can be anything
> at that point.
>
> Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>

After taking another look I think this is a worthwhile improvement.

Reviewed-by: Brian Geffon <bgeffon@google.com>

> ---
>  drivers/block/zram/zram_drv.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index ecbd9b25dfed..1f2867cd587e 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -1994,14 +1994,14 @@ static int zram_read_page(struct zram *zram, struct page *page, u32 index,
>                 ret = zram_read_from_zspool(zram, page, index);
>                 zram_slot_unlock(zram, index);
>         } else {
> +               unsigned long blk_idx = zram_get_handle(zram, index);
> +
>                 /*
>                  * The slot should be unlocked before reading from the backing
>                  * device.
>                  */
>                 zram_slot_unlock(zram, index);
> -
> -               ret = read_from_bdev(zram, page, zram_get_handle(zram, index),
> -                                    parent);
> +               ret = read_from_bdev(zram, page, blk_idx, parent);
>         }
>
>         /* Should NEVER happen. Return bio error if it does. */
> --
> 2.52.0.rc1.455.g30608eb744-goog
>

On Thu, Nov 20, 2025 at 10:22 AM Sergey Senozhatsky
<senozhatsky@chromium.org> wrote:
>
> Read slot's block id under slot-lock.  We release the
> slot-lock for bdev read so, technically, slot still can
> get freed in the meantime, but at least we will read
> bdev block (page) that holds previous know slot data,
> not from slot->handle bdev block, which can be anything
> at that point.
>
> Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
> ---
>  drivers/block/zram/zram_drv.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index ecbd9b25dfed..1f2867cd587e 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -1994,14 +1994,14 @@ static int zram_read_page(struct zram *zram, struct page *page, u32 index,
>                 ret = zram_read_from_zspool(zram, page, index);
>                 zram_slot_unlock(zram, index);
>         } else {
> +               unsigned long blk_idx = zram_get_handle(zram, index);
> +
>                 /*
>                  * The slot should be unlocked before reading from the backing
>                  * device.
>                  */
>                 zram_slot_unlock(zram, index);
> -
> -               ret = read_from_bdev(zram, page, zram_get_handle(zram, index),
> -                                    parent);
> +               ret = read_from_bdev(zram, page, blk_idx, parent);

This patch doesn't really change things in a meaningful way, wouldn't
it be more correct to take the slot lock again after the read and
confirm the handle is the same and it's still ZRAM_WB?

>         }
>
>         /* Should NEVER happen. Return bio error if it does. */
> --
> 2.52.0.rc1.455.g30608eb744-goog
>