Re: [RFC PATCHv5 5/6] zram: rework bdev block allocation

[Brian Geffon <bgeffon@google.com>]

On Thu, Nov 20, 2025 at 10:22 AM Sergey Senozhatsky
<senozhatsky@chromium.org> wrote:
>
> First, writeback bdev ->bitmap bits are set only from one
> context, as we can have only one single task performing
> writeback, so we cannot race with anything else.  Remove
> retry path.
>
> Second, we always check ZRAM_WB flag to distinguish writtenback
> slots, so we should not confuse 0 bdev block index and 0 handle.
> We can use first bdev block (0 bit) for writeback as well.
>
> While at it, give functions slightly more accurate names, as
> we don't alloc/free anything there, we reserve a block for
> async writeback or release the block.
>
> Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>

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

> ---
>  drivers/block/zram/zram_drv.c | 37 +++++++++++++++++------------------
>  1 file changed, 18 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index 671ef2ec9b11..ecbd9b25dfed 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -500,6 +500,8 @@ static ssize_t idle_store(struct device *dev,
>  }
>
>  #ifdef CONFIG_ZRAM_WRITEBACK
> +#define INVALID_BDEV_BLOCK             (~0UL)
> +
>  struct zram_wb_ctl {
>         struct list_head idle_reqs;
>         struct list_head done_reqs;
> @@ -746,23 +748,20 @@ static ssize_t backing_dev_store(struct device *dev,
>         return err;
>  }
>
> -static unsigned long alloc_block_bdev(struct zram *zram)
> +static unsigned long zram_reserve_bdev_block(struct zram *zram)
>  {
> -       unsigned long blk_idx = 1;
> -retry:
> -       /* skip 0 bit to confuse zram.handle = 0 */
> -       blk_idx = find_next_zero_bit(zram->bitmap, zram->nr_pages, blk_idx);
> -       if (blk_idx == zram->nr_pages)
> -               return 0;
> +       unsigned long blk_idx;
>
> -       if (test_and_set_bit(blk_idx, zram->bitmap))
> -               goto retry;
> +       blk_idx = find_next_zero_bit(zram->bitmap, zram->nr_pages, 0);
> +       if (blk_idx == zram->nr_pages)
> +               return INVALID_BDEV_BLOCK;
>
> +       set_bit(blk_idx, zram->bitmap);
>         atomic64_inc(&zram->stats.bd_count);
>         return blk_idx;
>  }
>
> -static void free_block_bdev(struct zram *zram, unsigned long blk_idx)
> +static void zram_release_bdev_block(struct zram *zram, unsigned long blk_idx)
>  {
>         int was_set;
>
> @@ -887,7 +886,7 @@ static int zram_writeback_complete(struct zram *zram, struct zram_wb_req *req)
>                  * (if enabled).
>                  */
>                 zram_account_writeback_rollback(zram);
> -               free_block_bdev(zram, req->blk_idx);
> +               zram_release_bdev_block(zram, req->blk_idx);
>                 return err;
>         }
>
> @@ -901,7 +900,7 @@ static int zram_writeback_complete(struct zram *zram, struct zram_wb_req *req)
>          * finishes.
>          */
>         if (!zram_test_flag(zram, index, ZRAM_PP_SLOT)) {
> -               free_block_bdev(zram, req->blk_idx);
> +               zram_release_bdev_block(zram, req->blk_idx);
>                 goto out;
>         }
>
> @@ -989,8 +988,8 @@ static int zram_writeback_slots(struct zram *zram,
>                                 struct zram_pp_ctl *ctl,
>                                 struct zram_wb_ctl *wb_ctl)
>  {
> +       unsigned long blk_idx = INVALID_BDEV_BLOCK;
>         struct zram_wb_req *req = NULL;
> -       unsigned long blk_idx = 0;
>         struct zram_pp_slot *pps;
>         int ret = 0, err = 0;
>         u32 index = 0;
> @@ -1022,9 +1021,9 @@ static int zram_writeback_slots(struct zram *zram,
>                                 ret = err;
>                 }
>
> -               if (!blk_idx) {
> -                       blk_idx = alloc_block_bdev(zram);
> -                       if (!blk_idx) {
> +               if (blk_idx == INVALID_BDEV_BLOCK) {
> +                       blk_idx = zram_reserve_bdev_block(zram);
> +                       if (blk_idx == INVALID_BDEV_BLOCK) {
>                                 ret = -ENOSPC;
>                                 break;
>                         }
> @@ -1058,7 +1057,7 @@ static int zram_writeback_slots(struct zram *zram,
>                 __bio_add_page(&req->bio, req->page, PAGE_SIZE, 0);
>
>                 zram_submit_wb_request(zram, wb_ctl, req);
> -               blk_idx = 0;
> +               blk_idx = INVALID_BDEV_BLOCK;
>                 req = NULL;
>                 cond_resched();
>                 continue;
> @@ -1365,7 +1364,7 @@ static int read_from_bdev(struct zram *zram, struct page *page,
>         return -EIO;
>  }
>
> -static void free_block_bdev(struct zram *zram, unsigned long blk_idx)
> +static void zram_release_bdev_block(struct zram *zram, unsigned long blk_idx)
>  {
>  }
>  #endif
> @@ -1889,7 +1888,7 @@ static void zram_free_page(struct zram *zram, size_t index)
>
>         if (zram_test_flag(zram, index, ZRAM_WB)) {
>                 zram_clear_flag(zram, index, ZRAM_WB);
> -               free_block_bdev(zram, zram_get_handle(zram, index));
> +               zram_release_bdev_block(zram, zram_get_handle(zram, index));
>                 goto out;
>         }
>
> --
> 2.52.0.rc1.455.g30608eb744-goog
>