Re: [PATCH] zloop: zero out only the unread tail of short reads

[Damien Le Moal <dlemoal@kernel.org>]

On 2026/07/30 14:58, Shin'ichiro Kawasaki wrote:
> zloop_complete_rq() handles a short read from a zone's backing file by
> zero-filling every bio in the request. This is correct when the entire
> request is beyond the written part of the zone. When a read request
> straddles the boundary between written and unwritten data, read_iter()
> returns a partial byte count and zloop then zeroes the whole request
> even for the partially read area with valid data.
> 
> Such an unexpected zero data read was observed with fio test script
> t/zbd/test-zbd-support and its test case 69, failing with "bad magic
> header 0".
> 
> Avoid the unexpected zero data read by zeroing out only the unread tail
> part. When read data size cmd->ret is smaller than the request size,
> call the new helper function zloop_fill_zero_rq() that walks through
> each bio_vec and fills zeros from the specified start offset.
> 
> Fixes: eb0570c7df23 ("block: new zoned loop block device driver")
> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> ---
> The fio test case 69 of t/zbd/test-zbd-support started failing since the
> recent kernel commit 9cbbac29d752 ("block: Remove redundant plug in
> __submit_bio()"). My understanding is that this commit in block layer
> changed request merge behavior, and unveiled the bug in zloop.
> 
>  drivers/block/zloop.c | 30 +++++++++++++++++++++++-------
>  1 file changed, 23 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/block/zloop.c b/drivers/block/zloop.c
> index 55eeb6aac0ea..42967e7aea58 100644
> --- a/drivers/block/zloop.c
> +++ b/drivers/block/zloop.c
> @@ -798,6 +798,26 @@ static void zloop_cmd_workfn(struct work_struct *work)
>  	current->flags = orig_flags;
>  }
>  
> +static void zloop_fill_zero_rq(struct request *rq, unsigned int zero_from)
> +{
> +	struct req_iterator iter;
> +	struct bio_vec bv;
> +	unsigned int pos = 0;
> +	unsigned int end, off;
> +
> +	rq_for_each_segment(bv, rq, iter) {
> +		end = pos + bv.bv_len;
> +		if (end > zero_from) {
> +			off = 0;
> +			if (pos < zero_from)
> +				off = zero_from - pos;

Maybe write this as:

			if (pos < zero_from)
				off = zero_from - pos;
			else
				off = 0;

> +			memzero_page(bv.bv_page, bv.bv_offset + off,
> +				     bv.bv_len - off);
> +		}
> +		pos = end;
> +	}
> +}
> +
>  static void zloop_complete_rq(struct request *rq)
>  {
>  	struct zloop_cmd *cmd = blk_mq_rq_to_pdu(rq);
> @@ -812,13 +832,9 @@ static void zloop_complete_rq(struct request *rq)
>  			pr_err("Zone %u: failed read sector %llu, %llu sectors\n",
>  			       zone_no, cmd->sector, cmd->nr_sectors);
>  
> -		if (cmd->ret >= 0 && cmd->ret != blk_rq_bytes(rq)) {
> -			/* short read */
> -			struct bio *bio;
> -
> -			__rq_for_each_bio(bio, rq)
> -				zero_fill_bio(bio);
> -		}
> +		/* Short read. Zero out the unread tail beyond cmd->ret. */
> +		if (cmd->ret >= 0 && cmd->ret != blk_rq_bytes(rq))
> +			zloop_fill_zero_rq(rq, cmd->ret);
>  		break;
>  	case REQ_OP_WRITE:
>  	case REQ_OP_ZONE_APPEND:


-- 
Damien Le Moal
Western Digital Research