From: Stanley Zhang <stazhang@purestorage.com>

Add a function ublk_copy_user_integrity() to copy integrity information
between a request and a user iov_iter. This mirrors the existing
ublk_copy_user_pages() but operates on request integrity data instead of
regular data. Check UBLKSRV_IO_INTEGRITY_FLAG in iocb->ki_pos in
ublk_user_copy() to choose between copying data or integrity data.

Signed-off-by: Stanley Zhang <stazhang@purestorage.com>
[csander: change offset units from data bytes to integrity data bytes,
 test UBLKSRV_IO_INTEGRITY_FLAG after subtracting UBLKSRV_IO_BUF_OFFSET,
 fix CONFIG_BLK_DEV_INTEGRITY=n build,
 rebase on ublk user copy refactor]
Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
---
 drivers/block/ublk_drv.c      | 52 +++++++++++++++++++++++++++++++++--
 include/uapi/linux/ublk_cmd.h |  4 +++
 2 files changed, 53 insertions(+), 3 deletions(-)

diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index e44ab9981ef4..9694a4c1caa7 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -621,10 +621,15 @@ static inline unsigned ublk_pos_to_tag(loff_t pos)
 {
 	return ((pos - UBLKSRV_IO_BUF_OFFSET) >> UBLK_TAG_OFF) &
 		UBLK_TAG_BITS_MASK;
 }
 
+static inline bool ublk_pos_is_integrity(loff_t pos)
+{
+	return !!((pos - UBLKSRV_IO_BUF_OFFSET) & UBLKSRV_IO_INTEGRITY_FLAG);
+}
+
 static void ublk_dev_param_basic_apply(struct ublk_device *ub)
 {
 	const struct ublk_param_basic *p = &ub->params.basic;
 
 	if (p->attrs & UBLK_ATTR_READ_ONLY)
@@ -1047,10 +1052,37 @@ static size_t ublk_copy_user_pages(const struct request *req,
 			break;
 	}
 	return done;
 }
 
+#ifdef CONFIG_BLK_DEV_INTEGRITY
+static size_t ublk_copy_user_integrity(const struct request *req,
+		unsigned offset, struct iov_iter *uiter, int dir)
+{
+	size_t done = 0;
+	struct bio *bio = req->bio;
+	struct bvec_iter iter;
+	struct bio_vec iv;
+
+	if (!blk_integrity_rq(req))
+		return 0;
+
+	bio_for_each_integrity_vec(iv, bio, iter) {
+		if (!ublk_copy_user_bvec(&iv, &offset, uiter, dir, &done))
+			break;
+	}
+
+	return done;
+}
+#else /* #ifdef CONFIG_BLK_DEV_INTEGRITY */
+static size_t ublk_copy_user_integrity(const struct request *req,
+		unsigned offset, struct iov_iter *uiter, int dir)
+{
+	return 0;
+}
+#endif /* #ifdef CONFIG_BLK_DEV_INTEGRITY */
+
 static inline bool ublk_need_map_req(const struct request *req)
 {
 	return ublk_rq_has_data(req) && req_op(req) == REQ_OP_WRITE;
 }
 
@@ -2654,10 +2686,12 @@ ublk_user_copy(struct kiocb *iocb, struct iov_iter *iter, int dir)
 {
 	struct ublk_device *ub = iocb->ki_filp->private_data;
 	struct ublk_queue *ubq;
 	struct request *req;
 	struct ublk_io *io;
+	unsigned data_len;
+	bool is_integrity;
 	size_t buf_off;
 	u16 tag, q_id;
 	ssize_t ret;
 
 	if (!user_backed_iter(iter))
@@ -2667,10 +2701,11 @@ ublk_user_copy(struct kiocb *iocb, struct iov_iter *iter, int dir)
 		return -EACCES;
 
 	tag = ublk_pos_to_tag(iocb->ki_pos);
 	q_id = ublk_pos_to_hwq(iocb->ki_pos);
 	buf_off = ublk_pos_to_buf_off(iocb->ki_pos);
+	is_integrity = ublk_pos_is_integrity(iocb->ki_pos);
 
 	if (q_id >= ub->dev_info.nr_hw_queues)
 		return -EINVAL;
 
 	ubq = ublk_get_queue(ub, q_id);
@@ -2683,21 +2718,31 @@ ublk_user_copy(struct kiocb *iocb, struct iov_iter *iter, int dir)
 	io = &ubq->ios[tag];
 	req = __ublk_check_and_get_req(ub, q_id, tag, io);
 	if (!req)
 		return -EINVAL;
 
-	if (buf_off > blk_rq_bytes(req)) {
+	if (is_integrity) {
+		struct blk_integrity *bi = &req->q->limits.integrity;
+
+		data_len = bio_integrity_bytes(bi, blk_rq_sectors(req));
+	} else {
+		data_len = blk_rq_bytes(req);
+	}
+	if (buf_off > data_len) {
 		ret = -EINVAL;
 		goto out;
 	}
 
 	if (!ublk_check_ubuf_dir(req, dir)) {
 		ret = -EACCES;
 		goto out;
 	}
 
-	ret = ublk_copy_user_pages(req, buf_off, iter, dir);
+	if (is_integrity)
+		ret = ublk_copy_user_integrity(req, buf_off, iter, dir);
+	else
+		ret = ublk_copy_user_pages(req, buf_off, iter, dir);
 
 out:
 	ublk_put_req_ref(io, req);
 	return ret;
 }
@@ -3931,11 +3976,12 @@ static struct miscdevice ublk_misc = {
 static int __init ublk_init(void)
 {
 	int ret;
 
 	BUILD_BUG_ON((u64)UBLKSRV_IO_BUF_OFFSET +
-			UBLKSRV_IO_BUF_TOTAL_SIZE < UBLKSRV_IO_BUF_OFFSET);
+			UBLKSRV_IO_BUF_TOTAL_SIZE +
+			UBLKSRV_IO_INTEGRITY_FLAG < UBLKSRV_IO_BUF_OFFSET);
 	BUILD_BUG_ON(sizeof(struct ublk_auto_buf_reg) != 8);
 
 	init_waitqueue_head(&ublk_idr_wq);
 
 	ret = misc_register(&ublk_misc);
diff --git a/include/uapi/linux/ublk_cmd.h b/include/uapi/linux/ublk_cmd.h
index c1103ad5925b..3af7e3684834 100644
--- a/include/uapi/linux/ublk_cmd.h
+++ b/include/uapi/linux/ublk_cmd.h
@@ -132,10 +132,14 @@
 #define UBLK_MAX_NR_QUEUES	(1U << UBLK_QID_BITS)
 
 #define UBLKSRV_IO_BUF_TOTAL_BITS	(UBLK_QID_OFF + UBLK_QID_BITS)
 #define UBLKSRV_IO_BUF_TOTAL_SIZE	(1ULL << UBLKSRV_IO_BUF_TOTAL_BITS)
 
+/* Copy to/from request integrity buffer instead of data buffer */
+#define UBLK_INTEGRITY_FLAG_OFF UBLKSRV_IO_BUF_TOTAL_BITS
+#define UBLKSRV_IO_INTEGRITY_FLAG (1ULL << UBLK_INTEGRITY_FLAG_OFF)
+
 /*
  * ublk server can register data buffers for incoming I/O requests with a sparse
  * io_uring buffer table. The request buffer can then be used as the data buffer
  * for io_uring operations via the fixed buffer index.
  * Note that the ublk server can never directly access the request data memory.
-- 
2.45.2

On Mon, Jan 05, 2026 at 05:57:41PM -0700, Caleb Sander Mateos wrote:
> From: Stanley Zhang <stazhang@purestorage.com>
> 
> Add a function ublk_copy_user_integrity() to copy integrity information
> between a request and a user iov_iter. This mirrors the existing
> ublk_copy_user_pages() but operates on request integrity data instead of
> regular data. Check UBLKSRV_IO_INTEGRITY_FLAG in iocb->ki_pos in
> ublk_user_copy() to choose between copying data or integrity data.
> 
> Signed-off-by: Stanley Zhang <stazhang@purestorage.com>
> [csander: change offset units from data bytes to integrity data bytes,
>  test UBLKSRV_IO_INTEGRITY_FLAG after subtracting UBLKSRV_IO_BUF_OFFSET,
>  fix CONFIG_BLK_DEV_INTEGRITY=n build,
>  rebase on ublk user copy refactor]
> Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
> ---
>  drivers/block/ublk_drv.c      | 52 +++++++++++++++++++++++++++++++++--
>  include/uapi/linux/ublk_cmd.h |  4 +++
>  2 files changed, 53 insertions(+), 3 deletions(-)
> 

...

> diff --git a/include/uapi/linux/ublk_cmd.h b/include/uapi/linux/ublk_cmd.h
> index c1103ad5925b..3af7e3684834 100644
> --- a/include/uapi/linux/ublk_cmd.h
> +++ b/include/uapi/linux/ublk_cmd.h
> @@ -132,10 +132,14 @@
>  #define UBLK_MAX_NR_QUEUES	(1U << UBLK_QID_BITS)
>  
>  #define UBLKSRV_IO_BUF_TOTAL_BITS	(UBLK_QID_OFF + UBLK_QID_BITS)
>  #define UBLKSRV_IO_BUF_TOTAL_SIZE	(1ULL << UBLKSRV_IO_BUF_TOTAL_BITS)
>  
> +/* Copy to/from request integrity buffer instead of data buffer */
> +#define UBLK_INTEGRITY_FLAG_OFF UBLKSRV_IO_BUF_TOTAL_BITS
> +#define UBLKSRV_IO_INTEGRITY_FLAG (1ULL << UBLK_INTEGRITY_FLAG_OFF)

UBLKSRV_IO_INTEGRITY_FLAG is actually one flag, not same with other encoded
fields, maybe it is better to define it from top bit(62) and not mix with
others? Then it can be helpful to extend in future.


Thanks,
Ming

On Mon, Jan 05, 2026 at 05:57:41PM -0700, Caleb Sander Mateos wrote:
> From: Stanley Zhang <stazhang@purestorage.com>
> 
> Add a function ublk_copy_user_integrity() to copy integrity information
> between a request and a user iov_iter. This mirrors the existing
> ublk_copy_user_pages() but operates on request integrity data instead of
> regular data. Check UBLKSRV_IO_INTEGRITY_FLAG in iocb->ki_pos in
> ublk_user_copy() to choose between copying data or integrity data.
> 
> Signed-off-by: Stanley Zhang <stazhang@purestorage.com>
> [csander: change offset units from data bytes to integrity data bytes,
>  test UBLKSRV_IO_INTEGRITY_FLAG after subtracting UBLKSRV_IO_BUF_OFFSET,
>  fix CONFIG_BLK_DEV_INTEGRITY=n build,
>  rebase on ublk user copy refactor]
> Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
> ---
>  drivers/block/ublk_drv.c      | 52 +++++++++++++++++++++++++++++++++--
>  include/uapi/linux/ublk_cmd.h |  4 +++
>  2 files changed, 53 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> index e44ab9981ef4..9694a4c1caa7 100644
> --- a/drivers/block/ublk_drv.c
> +++ b/drivers/block/ublk_drv.c
> @@ -621,10 +621,15 @@ static inline unsigned ublk_pos_to_tag(loff_t pos)
>  {
>  	return ((pos - UBLKSRV_IO_BUF_OFFSET) >> UBLK_TAG_OFF) &
>  		UBLK_TAG_BITS_MASK;
>  }
>  
> +static inline bool ublk_pos_is_integrity(loff_t pos)
> +{
> +	return !!((pos - UBLKSRV_IO_BUF_OFFSET) & UBLKSRV_IO_INTEGRITY_FLAG);
> +}
> +

It could be more readable to check UBLKSRV_IO_INTEGRITY_FLAG only.

>  static void ublk_dev_param_basic_apply(struct ublk_device *ub)
>  {
>  	const struct ublk_param_basic *p = &ub->params.basic;
>  
>  	if (p->attrs & UBLK_ATTR_READ_ONLY)
> @@ -1047,10 +1052,37 @@ static size_t ublk_copy_user_pages(const struct request *req,
>  			break;
>  	}
>  	return done;
>  }
>  
> +#ifdef CONFIG_BLK_DEV_INTEGRITY
> +static size_t ublk_copy_user_integrity(const struct request *req,
> +		unsigned offset, struct iov_iter *uiter, int dir)
> +{
> +	size_t done = 0;
> +	struct bio *bio = req->bio;
> +	struct bvec_iter iter;
> +	struct bio_vec iv;
> +
> +	if (!blk_integrity_rq(req))
> +		return 0;
> +
> +	bio_for_each_integrity_vec(iv, bio, iter) {
> +		if (!ublk_copy_user_bvec(&iv, &offset, uiter, dir, &done))
> +			break;
> +	}
> +
> +	return done;
> +}
> +#else /* #ifdef CONFIG_BLK_DEV_INTEGRITY */
> +static size_t ublk_copy_user_integrity(const struct request *req,
> +		unsigned offset, struct iov_iter *uiter, int dir)
> +{
> +	return 0;
> +}
> +#endif /* #ifdef CONFIG_BLK_DEV_INTEGRITY */
> +
>  static inline bool ublk_need_map_req(const struct request *req)
>  {
>  	return ublk_rq_has_data(req) && req_op(req) == REQ_OP_WRITE;
>  }
>  
> @@ -2654,10 +2686,12 @@ ublk_user_copy(struct kiocb *iocb, struct iov_iter *iter, int dir)
>  {
>  	struct ublk_device *ub = iocb->ki_filp->private_data;
>  	struct ublk_queue *ubq;
>  	struct request *req;
>  	struct ublk_io *io;
> +	unsigned data_len;
> +	bool is_integrity;
>  	size_t buf_off;
>  	u16 tag, q_id;
>  	ssize_t ret;
>  
>  	if (!user_backed_iter(iter))
> @@ -2667,10 +2701,11 @@ ublk_user_copy(struct kiocb *iocb, struct iov_iter *iter, int dir)
>  		return -EACCES;
>  
>  	tag = ublk_pos_to_tag(iocb->ki_pos);
>  	q_id = ublk_pos_to_hwq(iocb->ki_pos);
>  	buf_off = ublk_pos_to_buf_off(iocb->ki_pos);
> +	is_integrity = ublk_pos_is_integrity(iocb->ki_pos);

UBLKSRV_IO_INTEGRITY_FLAG can be set for device without UBLK_F_INTEGRITY,
so UBLK_F_INTEGRITY need to be checked in case of `is_integrity`.

>  
>  	if (q_id >= ub->dev_info.nr_hw_queues)
>  		return -EINVAL;
>  
>  	ubq = ublk_get_queue(ub, q_id);
> @@ -2683,21 +2718,31 @@ ublk_user_copy(struct kiocb *iocb, struct iov_iter *iter, int dir)
>  	io = &ubq->ios[tag];
>  	req = __ublk_check_and_get_req(ub, q_id, tag, io);
>  	if (!req)
>  		return -EINVAL;
>  
> -	if (buf_off > blk_rq_bytes(req)) {
> +	if (is_integrity) {
> +		struct blk_integrity *bi = &req->q->limits.integrity;
> +
> +		data_len = bio_integrity_bytes(bi, blk_rq_sectors(req));
> +	} else {
> +		data_len = blk_rq_bytes(req);
> +	}
> +	if (buf_off > data_len) {
>  		ret = -EINVAL;
>  		goto out;
>  	}
>  
>  	if (!ublk_check_ubuf_dir(req, dir)) {
>  		ret = -EACCES;
>  		goto out;
>  	}
>  
> -	ret = ublk_copy_user_pages(req, buf_off, iter, dir);
> +	if (is_integrity)
> +		ret = ublk_copy_user_integrity(req, buf_off, iter, dir);
> +	else
> +		ret = ublk_copy_user_pages(req, buf_off, iter, dir);
>  
>  out:
>  	ublk_put_req_ref(io, req);
>  	return ret;
>  }
> @@ -3931,11 +3976,12 @@ static struct miscdevice ublk_misc = {
>  static int __init ublk_init(void)
>  {
>  	int ret;
>  
>  	BUILD_BUG_ON((u64)UBLKSRV_IO_BUF_OFFSET +
> -			UBLKSRV_IO_BUF_TOTAL_SIZE < UBLKSRV_IO_BUF_OFFSET);
> +			UBLKSRV_IO_BUF_TOTAL_SIZE +
> +			UBLKSRV_IO_INTEGRITY_FLAG < UBLKSRV_IO_BUF_OFFSET);

Maybe it can be simplified as:

BUILD_BUG_ON(UBLK_INTEGRITY_FLAG_OFF >= 63);  /* Must fit in loff_t */

>  	BUILD_BUG_ON(sizeof(struct ublk_auto_buf_reg) != 8);
>  
>  	init_waitqueue_head(&ublk_idr_wq);
>  
>  	ret = misc_register(&ublk_misc);
> diff --git a/include/uapi/linux/ublk_cmd.h b/include/uapi/linux/ublk_cmd.h
> index c1103ad5925b..3af7e3684834 100644
> --- a/include/uapi/linux/ublk_cmd.h
> +++ b/include/uapi/linux/ublk_cmd.h
> @@ -132,10 +132,14 @@
>  #define UBLK_MAX_NR_QUEUES	(1U << UBLK_QID_BITS)
>  
>  #define UBLKSRV_IO_BUF_TOTAL_BITS	(UBLK_QID_OFF + UBLK_QID_BITS)
>  #define UBLKSRV_IO_BUF_TOTAL_SIZE	(1ULL << UBLKSRV_IO_BUF_TOTAL_BITS)
>  
> +/* Copy to/from request integrity buffer instead of data buffer */
> +#define UBLK_INTEGRITY_FLAG_OFF UBLKSRV_IO_BUF_TOTAL_BITS
> +#define UBLKSRV_IO_INTEGRITY_FLAG (1ULL << UBLK_INTEGRITY_FLAG_OFF)
> +
>  /*
>   * ublk server can register data buffers for incoming I/O requests with a sparse
>   * io_uring buffer table. The request buffer can then be used as the data buffer
>   * for io_uring operations via the fixed buffer index.
>   * Note that the ublk server can never directly access the request data memory.
> -- 
> 2.45.2
> 

Thanks,
Ming

On Tue, Jan 6, 2026 at 5:34 AM Ming Lei <ming.lei@redhat.com> wrote:
>
> On Mon, Jan 05, 2026 at 05:57:41PM -0700, Caleb Sander Mateos wrote:
> > From: Stanley Zhang <stazhang@purestorage.com>
> >
> > Add a function ublk_copy_user_integrity() to copy integrity information
> > between a request and a user iov_iter. This mirrors the existing
> > ublk_copy_user_pages() but operates on request integrity data instead of
> > regular data. Check UBLKSRV_IO_INTEGRITY_FLAG in iocb->ki_pos in
> > ublk_user_copy() to choose between copying data or integrity data.
> >
> > Signed-off-by: Stanley Zhang <stazhang@purestorage.com>
> > [csander: change offset units from data bytes to integrity data bytes,
> >  test UBLKSRV_IO_INTEGRITY_FLAG after subtracting UBLKSRV_IO_BUF_OFFSET,
> >  fix CONFIG_BLK_DEV_INTEGRITY=n build,
> >  rebase on ublk user copy refactor]
> > Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
> > ---
> >  drivers/block/ublk_drv.c      | 52 +++++++++++++++++++++++++++++++++--
> >  include/uapi/linux/ublk_cmd.h |  4 +++
> >  2 files changed, 53 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> > index e44ab9981ef4..9694a4c1caa7 100644
> > --- a/drivers/block/ublk_drv.c
> > +++ b/drivers/block/ublk_drv.c
> > @@ -621,10 +621,15 @@ static inline unsigned ublk_pos_to_tag(loff_t pos)
> >  {
> >       return ((pos - UBLKSRV_IO_BUF_OFFSET) >> UBLK_TAG_OFF) &
> >               UBLK_TAG_BITS_MASK;
> >  }
> >
> > +static inline bool ublk_pos_is_integrity(loff_t pos)
> > +{
> > +     return !!((pos - UBLKSRV_IO_BUF_OFFSET) & UBLKSRV_IO_INTEGRITY_FLAG);
> > +}
> > +
>
> It could be more readable to check UBLKSRV_IO_INTEGRITY_FLAG only.

That's assuming that UBLK_TAG_BITS = 16 has more bits than are
strictly required by UBLK_MAX_QUEUE_DEPTH = 4096? Otherwise, adding
UBLKSRV_IO_BUF_OFFSET = 1 << 31 to tag << UBLK_TAG_OFF could overflow
into the QID bits, which could then overflow into
UBLKSRV_IO_INTEGRITY_FLAG. That seems like a very fragile assumption.
And if you want to rely on this assumption, why bother subtracting
UBLKSRV_IO_BUF_OFFSET in ublk_pos_to_hwq() either? The compiler should
easily be able to deduplicate the iocb->ki_pos - UBLKSRV_IO_BUF_OFFSET
computations, so I can't imagine it matters for performance.

>
> >  static void ublk_dev_param_basic_apply(struct ublk_device *ub)
> >  {
> >       const struct ublk_param_basic *p = &ub->params.basic;
> >
> >       if (p->attrs & UBLK_ATTR_READ_ONLY)
> > @@ -1047,10 +1052,37 @@ static size_t ublk_copy_user_pages(const struct request *req,
> >                       break;
> >       }
> >       return done;
> >  }
> >
> > +#ifdef CONFIG_BLK_DEV_INTEGRITY
> > +static size_t ublk_copy_user_integrity(const struct request *req,
> > +             unsigned offset, struct iov_iter *uiter, int dir)
> > +{
> > +     size_t done = 0;
> > +     struct bio *bio = req->bio;
> > +     struct bvec_iter iter;
> > +     struct bio_vec iv;
> > +
> > +     if (!blk_integrity_rq(req))
> > +             return 0;
> > +
> > +     bio_for_each_integrity_vec(iv, bio, iter) {
> > +             if (!ublk_copy_user_bvec(&iv, &offset, uiter, dir, &done))
> > +                     break;
> > +     }
> > +
> > +     return done;
> > +}
> > +#else /* #ifdef CONFIG_BLK_DEV_INTEGRITY */
> > +static size_t ublk_copy_user_integrity(const struct request *req,
> > +             unsigned offset, struct iov_iter *uiter, int dir)
> > +{
> > +     return 0;
> > +}
> > +#endif /* #ifdef CONFIG_BLK_DEV_INTEGRITY */
> > +
> >  static inline bool ublk_need_map_req(const struct request *req)
> >  {
> >       return ublk_rq_has_data(req) && req_op(req) == REQ_OP_WRITE;
> >  }
> >
> > @@ -2654,10 +2686,12 @@ ublk_user_copy(struct kiocb *iocb, struct iov_iter *iter, int dir)
> >  {
> >       struct ublk_device *ub = iocb->ki_filp->private_data;
> >       struct ublk_queue *ubq;
> >       struct request *req;
> >       struct ublk_io *io;
> > +     unsigned data_len;
> > +     bool is_integrity;
> >       size_t buf_off;
> >       u16 tag, q_id;
> >       ssize_t ret;
> >
> >       if (!user_backed_iter(iter))
> > @@ -2667,10 +2701,11 @@ ublk_user_copy(struct kiocb *iocb, struct iov_iter *iter, int dir)
> >               return -EACCES;
> >
> >       tag = ublk_pos_to_tag(iocb->ki_pos);
> >       q_id = ublk_pos_to_hwq(iocb->ki_pos);
> >       buf_off = ublk_pos_to_buf_off(iocb->ki_pos);
> > +     is_integrity = ublk_pos_is_integrity(iocb->ki_pos);
>
> UBLKSRV_IO_INTEGRITY_FLAG can be set for device without UBLK_F_INTEGRITY,
> so UBLK_F_INTEGRITY need to be checked in case of `is_integrity`.

If UBLK_F_INTEGRITY isn't set, then UBLK_PARAM_TYPE_INTEGRITY isn't
allowed, so the ublk device won't support integrity data. Therefore,
blk_integrity_rq() will return false and ublk_copy_user_integrity()
will just return 0. Do you think it's important to return some error
code value instead? I would rather avoid the additional checks in the
hot path.

>
> >
> >       if (q_id >= ub->dev_info.nr_hw_queues)
> >               return -EINVAL;
> >
> >       ubq = ublk_get_queue(ub, q_id);
> > @@ -2683,21 +2718,31 @@ ublk_user_copy(struct kiocb *iocb, struct iov_iter *iter, int dir)
> >       io = &ubq->ios[tag];
> >       req = __ublk_check_and_get_req(ub, q_id, tag, io);
> >       if (!req)
> >               return -EINVAL;
> >
> > -     if (buf_off > blk_rq_bytes(req)) {
> > +     if (is_integrity) {
> > +             struct blk_integrity *bi = &req->q->limits.integrity;
> > +
> > +             data_len = bio_integrity_bytes(bi, blk_rq_sectors(req));
> > +     } else {
> > +             data_len = blk_rq_bytes(req);
> > +     }
> > +     if (buf_off > data_len) {
> >               ret = -EINVAL;
> >               goto out;
> >       }
> >
> >       if (!ublk_check_ubuf_dir(req, dir)) {
> >               ret = -EACCES;
> >               goto out;
> >       }
> >
> > -     ret = ublk_copy_user_pages(req, buf_off, iter, dir);
> > +     if (is_integrity)
> > +             ret = ublk_copy_user_integrity(req, buf_off, iter, dir);
> > +     else
> > +             ret = ublk_copy_user_pages(req, buf_off, iter, dir);
> >
> >  out:
> >       ublk_put_req_ref(io, req);
> >       return ret;
> >  }
> > @@ -3931,11 +3976,12 @@ static struct miscdevice ublk_misc = {
> >  static int __init ublk_init(void)
> >  {
> >       int ret;
> >
> >       BUILD_BUG_ON((u64)UBLKSRV_IO_BUF_OFFSET +
> > -                     UBLKSRV_IO_BUF_TOTAL_SIZE < UBLKSRV_IO_BUF_OFFSET);
> > +                     UBLKSRV_IO_BUF_TOTAL_SIZE +
> > +                     UBLKSRV_IO_INTEGRITY_FLAG < UBLKSRV_IO_BUF_OFFSET);
>
> Maybe it can be simplified as:
>
> BUILD_BUG_ON(UBLK_INTEGRITY_FLAG_OFF >= 63);  /* Must fit in loff_t */

Okay, I think that works. Even if the addition of
UBLKSRV_IO_BUF_OFFSET causes an overflow to the next bit, it should
still fit within a 64-bit integer.

Thanks,
Caleb

>
> >       BUILD_BUG_ON(sizeof(struct ublk_auto_buf_reg) != 8);
> >
> >       init_waitqueue_head(&ublk_idr_wq);
> >
> >       ret = misc_register(&ublk_misc);
> > diff --git a/include/uapi/linux/ublk_cmd.h b/include/uapi/linux/ublk_cmd.h
> > index c1103ad5925b..3af7e3684834 100644
> > --- a/include/uapi/linux/ublk_cmd.h
> > +++ b/include/uapi/linux/ublk_cmd.h
> > @@ -132,10 +132,14 @@
> >  #define UBLK_MAX_NR_QUEUES   (1U << UBLK_QID_BITS)
> >
> >  #define UBLKSRV_IO_BUF_TOTAL_BITS    (UBLK_QID_OFF + UBLK_QID_BITS)
> >  #define UBLKSRV_IO_BUF_TOTAL_SIZE    (1ULL << UBLKSRV_IO_BUF_TOTAL_BITS)
> >
> > +/* Copy to/from request integrity buffer instead of data buffer */
> > +#define UBLK_INTEGRITY_FLAG_OFF UBLKSRV_IO_BUF_TOTAL_BITS
> > +#define UBLKSRV_IO_INTEGRITY_FLAG (1ULL << UBLK_INTEGRITY_FLAG_OFF)
> > +
> >  /*
> >   * ublk server can register data buffers for incoming I/O requests with a sparse
> >   * io_uring buffer table. The request buffer can then be used as the data buffer
> >   * for io_uring operations via the fixed buffer index.
> >   * Note that the ublk server can never directly access the request data memory.
> > --
> > 2.45.2
> >
>
> Thanks,
> Ming
>

On Tue, Jan 06, 2026 at 10:20:14AM -0800, Caleb Sander Mateos wrote:
> On Tue, Jan 6, 2026 at 5:34 AM Ming Lei <ming.lei@redhat.com> wrote:
> >
> > On Mon, Jan 05, 2026 at 05:57:41PM -0700, Caleb Sander Mateos wrote:
> > > From: Stanley Zhang <stazhang@purestorage.com>
> > >
> > > Add a function ublk_copy_user_integrity() to copy integrity information
> > > between a request and a user iov_iter. This mirrors the existing
> > > ublk_copy_user_pages() but operates on request integrity data instead of
> > > regular data. Check UBLKSRV_IO_INTEGRITY_FLAG in iocb->ki_pos in
> > > ublk_user_copy() to choose between copying data or integrity data.
> > >
> > > Signed-off-by: Stanley Zhang <stazhang@purestorage.com>
> > > [csander: change offset units from data bytes to integrity data bytes,
> > >  test UBLKSRV_IO_INTEGRITY_FLAG after subtracting UBLKSRV_IO_BUF_OFFSET,
> > >  fix CONFIG_BLK_DEV_INTEGRITY=n build,
> > >  rebase on ublk user copy refactor]
> > > Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
> > > ---
> > >  drivers/block/ublk_drv.c      | 52 +++++++++++++++++++++++++++++++++--
> > >  include/uapi/linux/ublk_cmd.h |  4 +++
> > >  2 files changed, 53 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> > > index e44ab9981ef4..9694a4c1caa7 100644
> > > --- a/drivers/block/ublk_drv.c
> > > +++ b/drivers/block/ublk_drv.c
> > > @@ -621,10 +621,15 @@ static inline unsigned ublk_pos_to_tag(loff_t pos)
> > >  {
> > >       return ((pos - UBLKSRV_IO_BUF_OFFSET) >> UBLK_TAG_OFF) &
> > >               UBLK_TAG_BITS_MASK;
> > >  }
> > >
> > > +static inline bool ublk_pos_is_integrity(loff_t pos)
> > > +{
> > > +     return !!((pos - UBLKSRV_IO_BUF_OFFSET) & UBLKSRV_IO_INTEGRITY_FLAG);
> > > +}
> > > +
> >
> > It could be more readable to check UBLKSRV_IO_INTEGRITY_FLAG only.
> 
> That's assuming that UBLK_TAG_BITS = 16 has more bits than are
> strictly required by UBLK_MAX_QUEUE_DEPTH = 4096? Otherwise, adding
> UBLKSRV_IO_BUF_OFFSET = 1 << 31 to tag << UBLK_TAG_OFF could overflow
> into the QID bits, which could then overflow into
> UBLKSRV_IO_INTEGRITY_FLAG. That seems like a very fragile assumption.
> And if you want to rely on this assumption, why bother subtracting
> UBLKSRV_IO_BUF_OFFSET in ublk_pos_to_hwq() either? The compiler should
> easily be able to deduplicate the iocb->ki_pos - UBLKSRV_IO_BUF_OFFSET
> computations, so I can't imagine it matters for performance.

UBLKSRV_IO_INTEGRITY_FLAG should be defined as one flag starting from top
bit(bit 62), then you will see it is just fine to check it directly.

But it isn't a big deal to subtract UBLKSRV_IO_BUF_OFFSET or not here, I
will leave it to you.

> 
> >
> > >  static void ublk_dev_param_basic_apply(struct ublk_device *ub)
> > >  {
> > >       const struct ublk_param_basic *p = &ub->params.basic;
> > >
> > >       if (p->attrs & UBLK_ATTR_READ_ONLY)
> > > @@ -1047,10 +1052,37 @@ static size_t ublk_copy_user_pages(const struct request *req,
> > >                       break;
> > >       }
> > >       return done;
> > >  }
> > >
> > > +#ifdef CONFIG_BLK_DEV_INTEGRITY
> > > +static size_t ublk_copy_user_integrity(const struct request *req,
> > > +             unsigned offset, struct iov_iter *uiter, int dir)
> > > +{
> > > +     size_t done = 0;
> > > +     struct bio *bio = req->bio;
> > > +     struct bvec_iter iter;
> > > +     struct bio_vec iv;
> > > +
> > > +     if (!blk_integrity_rq(req))
> > > +             return 0;
> > > +
> > > +     bio_for_each_integrity_vec(iv, bio, iter) {
> > > +             if (!ublk_copy_user_bvec(&iv, &offset, uiter, dir, &done))
> > > +                     break;
> > > +     }
> > > +
> > > +     return done;
> > > +}
> > > +#else /* #ifdef CONFIG_BLK_DEV_INTEGRITY */
> > > +static size_t ublk_copy_user_integrity(const struct request *req,
> > > +             unsigned offset, struct iov_iter *uiter, int dir)
> > > +{
> > > +     return 0;
> > > +}
> > > +#endif /* #ifdef CONFIG_BLK_DEV_INTEGRITY */
> > > +
> > >  static inline bool ublk_need_map_req(const struct request *req)
> > >  {
> > >       return ublk_rq_has_data(req) && req_op(req) == REQ_OP_WRITE;
> > >  }
> > >
> > > @@ -2654,10 +2686,12 @@ ublk_user_copy(struct kiocb *iocb, struct iov_iter *iter, int dir)
> > >  {
> > >       struct ublk_device *ub = iocb->ki_filp->private_data;
> > >       struct ublk_queue *ubq;
> > >       struct request *req;
> > >       struct ublk_io *io;
> > > +     unsigned data_len;
> > > +     bool is_integrity;
> > >       size_t buf_off;
> > >       u16 tag, q_id;
> > >       ssize_t ret;
> > >
> > >       if (!user_backed_iter(iter))
> > > @@ -2667,10 +2701,11 @@ ublk_user_copy(struct kiocb *iocb, struct iov_iter *iter, int dir)
> > >               return -EACCES;
> > >
> > >       tag = ublk_pos_to_tag(iocb->ki_pos);
> > >       q_id = ublk_pos_to_hwq(iocb->ki_pos);
> > >       buf_off = ublk_pos_to_buf_off(iocb->ki_pos);
> > > +     is_integrity = ublk_pos_is_integrity(iocb->ki_pos);
> >
> > UBLKSRV_IO_INTEGRITY_FLAG can be set for device without UBLK_F_INTEGRITY,
> > so UBLK_F_INTEGRITY need to be checked in case of `is_integrity`.
> 
> If UBLK_F_INTEGRITY isn't set, then UBLK_PARAM_TYPE_INTEGRITY isn't
> allowed, so the ublk device won't support integrity data. Therefore,
> blk_integrity_rq() will return false and ublk_copy_user_integrity()
> will just return 0. Do you think it's important to return some error
> code value instead? I would rather avoid the additional checks in the
> hot path.

The check could be zero cost, but better to fail the wrong usage than
returning 0 silently, which may often imply big issue.


Thanks, 
Ming

On Tue, Jan 6, 2026 at 4:28 PM Ming Lei <ming.lei@redhat.com> wrote:
>
> On Tue, Jan 06, 2026 at 10:20:14AM -0800, Caleb Sander Mateos wrote:
> > On Tue, Jan 6, 2026 at 5:34 AM Ming Lei <ming.lei@redhat.com> wrote:
> > >
> > > On Mon, Jan 05, 2026 at 05:57:41PM -0700, Caleb Sander Mateos wrote:
> > > > From: Stanley Zhang <stazhang@purestorage.com>
> > > >
> > > > Add a function ublk_copy_user_integrity() to copy integrity information
> > > > between a request and a user iov_iter. This mirrors the existing
> > > > ublk_copy_user_pages() but operates on request integrity data instead of
> > > > regular data. Check UBLKSRV_IO_INTEGRITY_FLAG in iocb->ki_pos in
> > > > ublk_user_copy() to choose between copying data or integrity data.
> > > >
> > > > Signed-off-by: Stanley Zhang <stazhang@purestorage.com>
> > > > [csander: change offset units from data bytes to integrity data bytes,
> > > >  test UBLKSRV_IO_INTEGRITY_FLAG after subtracting UBLKSRV_IO_BUF_OFFSET,
> > > >  fix CONFIG_BLK_DEV_INTEGRITY=n build,
> > > >  rebase on ublk user copy refactor]
> > > > Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
> > > > ---
> > > >  drivers/block/ublk_drv.c      | 52 +++++++++++++++++++++++++++++++++--
> > > >  include/uapi/linux/ublk_cmd.h |  4 +++
> > > >  2 files changed, 53 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> > > > index e44ab9981ef4..9694a4c1caa7 100644
> > > > --- a/drivers/block/ublk_drv.c
> > > > +++ b/drivers/block/ublk_drv.c
> > > > @@ -621,10 +621,15 @@ static inline unsigned ublk_pos_to_tag(loff_t pos)
> > > >  {
> > > >       return ((pos - UBLKSRV_IO_BUF_OFFSET) >> UBLK_TAG_OFF) &
> > > >               UBLK_TAG_BITS_MASK;
> > > >  }
> > > >
> > > > +static inline bool ublk_pos_is_integrity(loff_t pos)
> > > > +{
> > > > +     return !!((pos - UBLKSRV_IO_BUF_OFFSET) & UBLKSRV_IO_INTEGRITY_FLAG);
> > > > +}
> > > > +
> > >
> > > It could be more readable to check UBLKSRV_IO_INTEGRITY_FLAG only.
> >
> > That's assuming that UBLK_TAG_BITS = 16 has more bits than are
> > strictly required by UBLK_MAX_QUEUE_DEPTH = 4096? Otherwise, adding
> > UBLKSRV_IO_BUF_OFFSET = 1 << 31 to tag << UBLK_TAG_OFF could overflow
> > into the QID bits, which could then overflow into
> > UBLKSRV_IO_INTEGRITY_FLAG. That seems like a very fragile assumption.
> > And if you want to rely on this assumption, why bother subtracting
> > UBLKSRV_IO_BUF_OFFSET in ublk_pos_to_hwq() either? The compiler should
> > easily be able to deduplicate the iocb->ki_pos - UBLKSRV_IO_BUF_OFFSET
> > computations, so I can't imagine it matters for performance.
>
> UBLKSRV_IO_INTEGRITY_FLAG should be defined as one flag starting from top
> bit(bit 62), then you will see it is just fine to check it directly.
>
> But it isn't a big deal to subtract UBLKSRV_IO_BUF_OFFSET or not here, I
> will leave it to you.
>
> >
> > >
> > > >  static void ublk_dev_param_basic_apply(struct ublk_device *ub)
> > > >  {
> > > >       const struct ublk_param_basic *p = &ub->params.basic;
> > > >
> > > >       if (p->attrs & UBLK_ATTR_READ_ONLY)
> > > > @@ -1047,10 +1052,37 @@ static size_t ublk_copy_user_pages(const struct request *req,
> > > >                       break;
> > > >       }
> > > >       return done;
> > > >  }
> > > >
> > > > +#ifdef CONFIG_BLK_DEV_INTEGRITY
> > > > +static size_t ublk_copy_user_integrity(const struct request *req,
> > > > +             unsigned offset, struct iov_iter *uiter, int dir)
> > > > +{
> > > > +     size_t done = 0;
> > > > +     struct bio *bio = req->bio;
> > > > +     struct bvec_iter iter;
> > > > +     struct bio_vec iv;
> > > > +
> > > > +     if (!blk_integrity_rq(req))
> > > > +             return 0;
> > > > +
> > > > +     bio_for_each_integrity_vec(iv, bio, iter) {
> > > > +             if (!ublk_copy_user_bvec(&iv, &offset, uiter, dir, &done))
> > > > +                     break;
> > > > +     }
> > > > +
> > > > +     return done;
> > > > +}
> > > > +#else /* #ifdef CONFIG_BLK_DEV_INTEGRITY */
> > > > +static size_t ublk_copy_user_integrity(const struct request *req,
> > > > +             unsigned offset, struct iov_iter *uiter, int dir)
> > > > +{
> > > > +     return 0;
> > > > +}
> > > > +#endif /* #ifdef CONFIG_BLK_DEV_INTEGRITY */
> > > > +
> > > >  static inline bool ublk_need_map_req(const struct request *req)
> > > >  {
> > > >       return ublk_rq_has_data(req) && req_op(req) == REQ_OP_WRITE;
> > > >  }
> > > >
> > > > @@ -2654,10 +2686,12 @@ ublk_user_copy(struct kiocb *iocb, struct iov_iter *iter, int dir)
> > > >  {
> > > >       struct ublk_device *ub = iocb->ki_filp->private_data;
> > > >       struct ublk_queue *ubq;
> > > >       struct request *req;
> > > >       struct ublk_io *io;
> > > > +     unsigned data_len;
> > > > +     bool is_integrity;
> > > >       size_t buf_off;
> > > >       u16 tag, q_id;
> > > >       ssize_t ret;
> > > >
> > > >       if (!user_backed_iter(iter))
> > > > @@ -2667,10 +2701,11 @@ ublk_user_copy(struct kiocb *iocb, struct iov_iter *iter, int dir)
> > > >               return -EACCES;
> > > >
> > > >       tag = ublk_pos_to_tag(iocb->ki_pos);
> > > >       q_id = ublk_pos_to_hwq(iocb->ki_pos);
> > > >       buf_off = ublk_pos_to_buf_off(iocb->ki_pos);
> > > > +     is_integrity = ublk_pos_is_integrity(iocb->ki_pos);
> > >
> > > UBLKSRV_IO_INTEGRITY_FLAG can be set for device without UBLK_F_INTEGRITY,
> > > so UBLK_F_INTEGRITY need to be checked in case of `is_integrity`.
> >
> > If UBLK_F_INTEGRITY isn't set, then UBLK_PARAM_TYPE_INTEGRITY isn't
> > allowed, so the ublk device won't support integrity data. Therefore,
> > blk_integrity_rq() will return false and ublk_copy_user_integrity()
> > will just return 0. Do you think it's important to return some error
> > code value instead? I would rather avoid the additional checks in the
> > hot path.
>
> The check could be zero cost, but better to fail the wrong usage than
> returning 0 silently, which may often imply big issue.

Not sure what you mean by "the check could be zero cost". It's 2
branches to check for UBLK_F_INTEGRITY in the ublk_device flags and to
check is_integrity. Even if the branches are predictable (and the
is_integrity one might not be), there's still some cost for computing
the conditions and taking up space in the branch history table.
A ublk server should already be checking that the return value from
the user copy syscall matches the passed in length. Otherwise, the
request's data was shorter than expected or a fault occurred while
accessing the userspace buffer. But if you feel strongly, I'll add an
explicit -EINVAL return code.

Best,
Caleb

On Wed, Jan 07, 2026 at 05:50:04PM -0800, Caleb Sander Mateos wrote:
> On Tue, Jan 6, 2026 at 4:28 PM Ming Lei <ming.lei@redhat.com> wrote:
> >
> > On Tue, Jan 06, 2026 at 10:20:14AM -0800, Caleb Sander Mateos wrote:
> > > On Tue, Jan 6, 2026 at 5:34 AM Ming Lei <ming.lei@redhat.com> wrote:
> > > >
> > > > On Mon, Jan 05, 2026 at 05:57:41PM -0700, Caleb Sander Mateos wrote:
> > > > > From: Stanley Zhang <stazhang@purestorage.com>
> > > > >
> > > > > Add a function ublk_copy_user_integrity() to copy integrity information
> > > > > between a request and a user iov_iter. This mirrors the existing
> > > > > ublk_copy_user_pages() but operates on request integrity data instead of
> > > > > regular data. Check UBLKSRV_IO_INTEGRITY_FLAG in iocb->ki_pos in
> > > > > ublk_user_copy() to choose between copying data or integrity data.
> > > > >
> > > > > Signed-off-by: Stanley Zhang <stazhang@purestorage.com>
> > > > > [csander: change offset units from data bytes to integrity data bytes,
> > > > >  test UBLKSRV_IO_INTEGRITY_FLAG after subtracting UBLKSRV_IO_BUF_OFFSET,
> > > > >  fix CONFIG_BLK_DEV_INTEGRITY=n build,
> > > > >  rebase on ublk user copy refactor]
> > > > > Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
> > > > > ---
> > > > >  drivers/block/ublk_drv.c      | 52 +++++++++++++++++++++++++++++++++--
> > > > >  include/uapi/linux/ublk_cmd.h |  4 +++
> > > > >  2 files changed, 53 insertions(+), 3 deletions(-)
> > > > >
> > > > > diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> > > > > index e44ab9981ef4..9694a4c1caa7 100644
> > > > > --- a/drivers/block/ublk_drv.c
> > > > > +++ b/drivers/block/ublk_drv.c
> > > > > @@ -621,10 +621,15 @@ static inline unsigned ublk_pos_to_tag(loff_t pos)
> > > > >  {
> > > > >       return ((pos - UBLKSRV_IO_BUF_OFFSET) >> UBLK_TAG_OFF) &
> > > > >               UBLK_TAG_BITS_MASK;
> > > > >  }
> > > > >
> > > > > +static inline bool ublk_pos_is_integrity(loff_t pos)
> > > > > +{
> > > > > +     return !!((pos - UBLKSRV_IO_BUF_OFFSET) & UBLKSRV_IO_INTEGRITY_FLAG);
> > > > > +}
> > > > > +
> > > >
> > > > It could be more readable to check UBLKSRV_IO_INTEGRITY_FLAG only.
> > >
> > > That's assuming that UBLK_TAG_BITS = 16 has more bits than are
> > > strictly required by UBLK_MAX_QUEUE_DEPTH = 4096? Otherwise, adding
> > > UBLKSRV_IO_BUF_OFFSET = 1 << 31 to tag << UBLK_TAG_OFF could overflow
> > > into the QID bits, which could then overflow into
> > > UBLKSRV_IO_INTEGRITY_FLAG. That seems like a very fragile assumption.
> > > And if you want to rely on this assumption, why bother subtracting
> > > UBLKSRV_IO_BUF_OFFSET in ublk_pos_to_hwq() either? The compiler should
> > > easily be able to deduplicate the iocb->ki_pos - UBLKSRV_IO_BUF_OFFSET
> > > computations, so I can't imagine it matters for performance.
> >
> > UBLKSRV_IO_INTEGRITY_FLAG should be defined as one flag starting from top
> > bit(bit 62), then you will see it is just fine to check it directly.
> >
> > But it isn't a big deal to subtract UBLKSRV_IO_BUF_OFFSET or not here, I
> > will leave it to you.
> >
> > >
> > > >
> > > > >  static void ublk_dev_param_basic_apply(struct ublk_device *ub)
> > > > >  {
> > > > >       const struct ublk_param_basic *p = &ub->params.basic;
> > > > >
> > > > >       if (p->attrs & UBLK_ATTR_READ_ONLY)
> > > > > @@ -1047,10 +1052,37 @@ static size_t ublk_copy_user_pages(const struct request *req,
> > > > >                       break;
> > > > >       }
> > > > >       return done;
> > > > >  }
> > > > >
> > > > > +#ifdef CONFIG_BLK_DEV_INTEGRITY
> > > > > +static size_t ublk_copy_user_integrity(const struct request *req,
> > > > > +             unsigned offset, struct iov_iter *uiter, int dir)
> > > > > +{
> > > > > +     size_t done = 0;
> > > > > +     struct bio *bio = req->bio;
> > > > > +     struct bvec_iter iter;
> > > > > +     struct bio_vec iv;
> > > > > +
> > > > > +     if (!blk_integrity_rq(req))
> > > > > +             return 0;
> > > > > +
> > > > > +     bio_for_each_integrity_vec(iv, bio, iter) {
> > > > > +             if (!ublk_copy_user_bvec(&iv, &offset, uiter, dir, &done))
> > > > > +                     break;
> > > > > +     }
> > > > > +
> > > > > +     return done;
> > > > > +}
> > > > > +#else /* #ifdef CONFIG_BLK_DEV_INTEGRITY */
> > > > > +static size_t ublk_copy_user_integrity(const struct request *req,
> > > > > +             unsigned offset, struct iov_iter *uiter, int dir)
> > > > > +{
> > > > > +     return 0;
> > > > > +}
> > > > > +#endif /* #ifdef CONFIG_BLK_DEV_INTEGRITY */
> > > > > +
> > > > >  static inline bool ublk_need_map_req(const struct request *req)
> > > > >  {
> > > > >       return ublk_rq_has_data(req) && req_op(req) == REQ_OP_WRITE;
> > > > >  }
> > > > >
> > > > > @@ -2654,10 +2686,12 @@ ublk_user_copy(struct kiocb *iocb, struct iov_iter *iter, int dir)
> > > > >  {
> > > > >       struct ublk_device *ub = iocb->ki_filp->private_data;
> > > > >       struct ublk_queue *ubq;
> > > > >       struct request *req;
> > > > >       struct ublk_io *io;
> > > > > +     unsigned data_len;
> > > > > +     bool is_integrity;
> > > > >       size_t buf_off;
> > > > >       u16 tag, q_id;
> > > > >       ssize_t ret;
> > > > >
> > > > >       if (!user_backed_iter(iter))
> > > > > @@ -2667,10 +2701,11 @@ ublk_user_copy(struct kiocb *iocb, struct iov_iter *iter, int dir)
> > > > >               return -EACCES;
> > > > >
> > > > >       tag = ublk_pos_to_tag(iocb->ki_pos);
> > > > >       q_id = ublk_pos_to_hwq(iocb->ki_pos);
> > > > >       buf_off = ublk_pos_to_buf_off(iocb->ki_pos);
> > > > > +     is_integrity = ublk_pos_is_integrity(iocb->ki_pos);
> > > >
> > > > UBLKSRV_IO_INTEGRITY_FLAG can be set for device without UBLK_F_INTEGRITY,
> > > > so UBLK_F_INTEGRITY need to be checked in case of `is_integrity`.
> > >
> > > If UBLK_F_INTEGRITY isn't set, then UBLK_PARAM_TYPE_INTEGRITY isn't
> > > allowed, so the ublk device won't support integrity data. Therefore,
> > > blk_integrity_rq() will return false and ublk_copy_user_integrity()
> > > will just return 0. Do you think it's important to return some error
> > > code value instead? I would rather avoid the additional checks in the
> > > hot path.
> >
> > The check could be zero cost, but better to fail the wrong usage than
> > returning 0 silently, which may often imply big issue.
> 
> Not sure what you mean by "the check could be zero cost". It's 2
> branches to check for UBLK_F_INTEGRITY in the ublk_device flags and to
> check is_integrity. Even if the branches are predictable (and the
> is_integrity one might not be), there's still some cost for computing
> the conditions and taking up space in the branch history table.

ub->dev_info.nr_hw_queues is fetched for validating `q_id`, so
ub->dev_info.flags is always hit from the same cache line.

> A ublk server should already be checking that the return value from
> the user copy syscall matches the passed in length. Otherwise, the
> request's data was shorter than expected or a fault occurred while
> accessing the userspace buffer. But if you feel strongly, I'll add an
> explicit -EINVAL return code.

It is absolutely userspace fault or bug, I think it is better to fast fail.
Otherwise, it has to be documented clearly.


Thanks,
Ming