[PATCH 08/20] ublk: add ublk_copy_user_bvec() helper

Caleb Sander Mateos posted 20 patches 1 month, 3 weeks ago
There is a newer version of this series
[PATCH 08/20] ublk: add ublk_copy_user_bvec() helper
Posted by Caleb Sander Mateos 1 month, 3 weeks ago
Factor a helper function ublk_copy_user_bvec() out of
ublk_copy_user_pages(). It will be used for copying integrity data too.

Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
---
 drivers/block/ublk_drv.c | 52 +++++++++++++++++++++++-----------------
 1 file changed, 30 insertions(+), 22 deletions(-)

diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index d3652ceba96d..0499add560b5 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -987,10 +987,39 @@ static const struct block_device_operations ub_fops = {
 	.open =		ublk_open,
 	.free_disk =	ublk_free_disk,
 	.report_zones =	ublk_report_zones,
 };
 
+static bool ublk_copy_user_bvec(struct bio_vec bv, unsigned *offset,
+				struct iov_iter *uiter, int dir, size_t *done)
+{
+	unsigned len;
+	void *bv_buf;
+	size_t copied;
+
+	if (*offset >= bv.bv_len) {
+		*offset -= bv.bv_len;
+		return true;
+	}
+
+	len = bv.bv_len - *offset;
+	bv_buf = kmap_local_page(bv.bv_page) + bv.bv_offset + *offset;
+	if (dir == ITER_DEST)
+		copied = copy_to_iter(bv_buf, len, uiter);
+	else
+		copied = copy_from_iter(bv_buf, len, uiter);
+
+	kunmap_local(bv_buf);
+
+	*done += copied;
+	if (copied < len)
+		return false;
+
+	*offset = 0;
+	return true;
+}
+
 /*
  * 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,
@@ -999,33 +1028,12 @@ static size_t ublk_copy_user_pages(const struct request *req,
 	struct req_iterator iter;
 	struct bio_vec bv;
 	size_t done = 0;
 
 	rq_for_each_segment(bv, req, iter) {
-		unsigned len;
-		void *bv_buf;
-		size_t copied;
-
-		if (offset >= bv.bv_len) {
-			offset -= bv.bv_len;
-			continue;
-		}
-
-		len = bv.bv_len - offset;
-		bv_buf = kmap_local_page(bv.bv_page) + bv.bv_offset + offset;
-		if (dir == ITER_DEST)
-			copied = copy_to_iter(bv_buf, len, uiter);
-		else
-			copied = copy_from_iter(bv_buf, len, uiter);
-
-		kunmap_local(bv_buf);
-
-		done += copied;
-		if (copied < len)
+		if (!ublk_copy_user_bvec(bv, &offset, uiter, dir, &done))
 			break;
-
-		offset = 0;
 	}
 	return done;
 }
 
 static inline bool ublk_need_map_req(const struct request *req)
-- 
2.45.2
Re: [PATCH 08/20] ublk: add ublk_copy_user_bvec() helper
Posted by Ming Lei 1 month, 2 weeks ago
On Tue, Dec 16, 2025 at 10:34:42PM -0700, Caleb Sander Mateos wrote:
> Factor a helper function ublk_copy_user_bvec() out of
> ublk_copy_user_pages(). It will be used for copying integrity data too.
> 
> Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
> ---
>  drivers/block/ublk_drv.c | 52 +++++++++++++++++++++++-----------------
>  1 file changed, 30 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> index d3652ceba96d..0499add560b5 100644
> --- a/drivers/block/ublk_drv.c
> +++ b/drivers/block/ublk_drv.c
> @@ -987,10 +987,39 @@ static const struct block_device_operations ub_fops = {
>  	.open =		ublk_open,
>  	.free_disk =	ublk_free_disk,
>  	.report_zones =	ublk_report_zones,
>  };
>  
> +static bool ublk_copy_user_bvec(struct bio_vec bv, unsigned *offset,

bv could be better to define as `const struct bio_vec *` for avoiding copy,
otherwise this patch looks fine.


Thanks, 
Ming
Re: [PATCH 08/20] ublk: add ublk_copy_user_bvec() helper
Posted by Caleb Sander Mateos 1 month, 2 weeks ago
On Mon, Dec 22, 2025 at 9:53 AM Ming Lei <ming.lei@redhat.com> wrote:
>
> On Tue, Dec 16, 2025 at 10:34:42PM -0700, Caleb Sander Mateos wrote:
> > Factor a helper function ublk_copy_user_bvec() out of
> > ublk_copy_user_pages(). It will be used for copying integrity data too.
> >
> > Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
> > ---
> >  drivers/block/ublk_drv.c | 52 +++++++++++++++++++++++-----------------
> >  1 file changed, 30 insertions(+), 22 deletions(-)
> >
> > diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> > index d3652ceba96d..0499add560b5 100644
> > --- a/drivers/block/ublk_drv.c
> > +++ b/drivers/block/ublk_drv.c
> > @@ -987,10 +987,39 @@ static const struct block_device_operations ub_fops = {
> >       .open =         ublk_open,
> >       .free_disk =    ublk_free_disk,
> >       .report_zones = ublk_report_zones,
> >  };
> >
> > +static bool ublk_copy_user_bvec(struct bio_vec bv, unsigned *offset,
>
> bv could be better to define as `const struct bio_vec *` for avoiding copy,
> otherwise this patch looks fine.

I was thinking it probably didn't matter much as the compiler was
likely to inline the function call. But sure, I can pass it by
pointer.

Thanks,
Caleb