Re: [PATCH] zram: do not forget to endio for partial discard requests

[Qu Wenruo <wqu@suse.com>]

在 2026/3/31 17:45, Sergey Senozhatsky 写道:
> As reported by Qu Wenruo, the following
> 
>   getconf PAGESIZE
>   65536
>   blkdiscard -p 4k /dev/zram0
> 
> takes literally forever to complete.  zram doesn't support
> partial discards and just returns immediately w/o doing any
> discard work in such cases.  The problem is that we forget
> to endio on our way out, so blkdiscard sleeps forever in
> submit_bio_wait().  Fix this by adding a missing bio_endio()
> call.
> 
> Fixes: 0120dd6e4e202 ("zram: make zram_bio_discard more self-contained")
> Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
> Reported-by: Qu Wenruo <wqu@suse.com>
> Closes: https://lore.kernel.org/linux-block/92361cd3-fb8b-482e-bc89-15ff1acb9a59@suse.com
> Cc: Christoph Hellwig <hch@lst.de>

Test-by: Qu Wenruo <wqu@suse.com>

Now all discard related works like mkfs.btrfs and mounting btrfs with 
async discard on zram devices works fine.

Thanks a lot for such a quick debugging and fix!


Although I'm still seeing not ideal performance if a btrfs with 4K block 
size on that zram device.

On a regular block device (LVM):

  # mkfs.btrfs -f /dev/test/scratch1
  # mount /dev/test/scratch1 /mnt/btrfs
  # time sudo ./xfstests-dev/ltp/fsstress -d /mnt/btrfs/ -n 10000 \
                                          -s 1774231493 -w

  real	0m3.435s
  user	0m0.005s
  sys	0m0.009s

On a zram block device: (mkfs.btrfs defaults to 4K block size)

  # mkfs.btrfs -f /dev/zram0
  # mount /dev/test/scratch1 /mnt/btrfs
  # time sudo ./xfstests-dev/ltp/fsstress -d /mnt/btrfs/ -n 10000 \
                                          -s 1774231493 -w

  real	0m10.726s
  user	0m0.005s
  sys	0m0.009s

Which is 3 times slower than regular block devices.

And after that short fsstress run:

  NAME       ALGORITHM DISKSIZE   DATA COMPR TOTAL STREAMS MOUNTPOINT
  /dev/zram0 zstd            1G 271.2M   10M 20.4M

I guess this time the overhead is just in the compression so that's 
something expected?

Thanks,
Qu

> ---
>   drivers/block/zram/zram_drv.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index dcea703a6766..b0637423953b 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -2683,8 +2683,10 @@ static void zram_bio_discard(struct zram *zram, struct bio *bio)
>   	 * skipping this logical block is appropriate here.
>   	 */
>   	if (offset) {
> -		if (n <= (PAGE_SIZE - offset))
> +		if (n <= (PAGE_SIZE - offset)) {
> +			bio_endio(bio);
>   			return;
> +		}
>   
>   		n -= (PAGE_SIZE - offset);
>   		index++;