Re: [PATCH v2 2/2] ublk: use rq_for_each_bvec() for user copy

[Ming Lei <ming.lei@redhat.com>]

On Wed, Nov 05, 2025 at 01:28:23PM -0700, Caleb Sander Mateos wrote:
> ublk_advance_io_iter() and ublk_copy_io_pages() currently open-code the
> iteration over request's bvecs. Switch to the rq_for_each_bvec() macro
> provided by blk-mq to avoid reaching into the bio internals and simplify
> the code. Unlike bio_iter_iovec(), rq_for_each_bvec() can return
> multi-page bvecs. So switch from copy_{to,from}_iter() to
> copy_page_{to,from}_iter() to map and copy each page in the bvec.
> 
> Suggested-by: Ming Lei <ming.lei@redhat.com>
> Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
> ---
>  drivers/block/ublk_drv.c | 78 ++++++++++++----------------------------
>  1 file changed, 23 insertions(+), 55 deletions(-)
> 
> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> index 40eee3e15a4c..929d40fe0250 100644
> --- a/drivers/block/ublk_drv.c
> +++ b/drivers/block/ublk_drv.c
> @@ -911,81 +911,49 @@ static const struct block_device_operations ub_fops = {
>  	.open =		ublk_open,
>  	.free_disk =	ublk_free_disk,
>  	.report_zones =	ublk_report_zones,
>  };
>  
> -struct ublk_io_iter {
> -	struct bio *bio;
> -	struct bvec_iter iter;
> -};
> -
> -/* return how many bytes are copied */
> -static size_t ublk_copy_io_pages(struct ublk_io_iter *data,
> -		struct iov_iter *uiter, int dir)
> +/*
> + * Copy data between request pages and io_iter, and 'offset'
> + * is the start point of linear offset of request.
> + */
> +static size_t ublk_copy_user_pages(const struct request *req,
> +		unsigned offset, struct iov_iter *uiter, int dir)
>  {
> +	struct req_iterator iter;
> +	struct bio_vec bv;
>  	size_t done = 0;
>  
> -	for (;;) {
> -		struct bio_vec bv = bio_iter_iovec(data->bio, data->iter);
> -		void *bv_buf = bvec_kmap_local(&bv);
> +	rq_for_each_bvec(bv, req, iter) {
>  		size_t copied;
>  
> +		if (offset >= bv.bv_len) {
> +			offset -= bv.bv_len;
> +			continue;
> +		}
> +
> +		bv.bv_offset += offset;
> +		bv.bv_len -= offset;
> +		bv.bv_page += bv.bv_offset / PAGE_SIZE;
> +		bv.bv_offset %= PAGE_SIZE;

The above two lines are not needed because copy_page_*_iter() handles
it already.

>  		if (dir == ITER_DEST)
> -			copied = copy_to_iter(bv_buf, bv.bv_len, uiter);
> +			copied = copy_page_to_iter(
> +				bv.bv_page, bv.bv_offset, bv.bv_len, uiter);

WARN in page_copy_sane() is triggered because copy_page_*_iter() requires 
all pages belong to one same compound page.

One quick fix is to replace rq_for_each_bvec() with rq_for_each_segment().

thanks,
Ming