[PATCH v7 0/3] bsg: add io_uring command support for SCSI passthrough

[PATCH v7 0/3] bsg: add io_uring command support for SCSI passthrough

[Yang Xiuwei]

This series adds io_uring command support to the BSG SCSI passthrough path.

The goal is to allow userspace to submit SCSI passthrough commands via
IORING_OP_URING_CMD, in addition to the existing sg_io interface.
The io_uring path mirrors the existing BSG behaviour: it currently only
supports BSG_PROTOCOL_SCSI + BSG_SUB_PROTOCOL_SCSI_CMD and does not
support BIDI transfers.

Patch 1 defines struct bsg_uring_cmd in the UAPI and documents the CQE
res2 layout with extraction/assembly macros for userspace.

Patch 2 extends the generic BSG layer with an .uring_cmd file operation
and a bsg_uring_cmd_fn callback, allowing transport-specific handlers to
be registered.

Patch 3 implements the SCSI BSG io_uring handler. It builds a SCSI
request from struct bsg_uring_cmd, maps user buffers (including fixed
buffers), and completes asynchronously via a request end_io callback and
task_work. Completion returns SCSI device/host/driver status, residual
length and sense length packed into CQE res2; status is read from
scmd->result in task_work.

Changes since v6 [2]:

  [1/3] bsg_uring_cmd UAPI
  - Removed the flags field (Bart).
  - Documented CQE res2 layout and added BSG_SCSI_RES2_* macros for userspace (me).
  - Added BSG_SCSI_RES2_BUILD() in UAPI (me).

  [2/3] generic BSG layer
  - Reordered variable declarations (longest to shortest) (Bart).
  - Use early return when uring_cmd_fn is not set (Bart).

  [3/3] SCSI BSG io_uring handler
  - Read device/host/driver status from scmd->result in task_work (Bart).
  - Use status_byte() and host_byte() instead of open-coding (Bart).
  - Removed superfluous " & 0xff" from u8 expressions (Bart).

Compared to the earlier RFC v4 series [1], this version only includes
minor code cleanups (mainly comments and wording), without changing the
behaviour or logic of the implementation.

[1] https://lore.kernel.org/linux-block/20260122015653.703188-1-yangxiuwei@kylinos.cn/
[2] https://lore.kernel.org/linux-block/20260305012857.2136525-1-yangxiuwei@kylinos.cn/

Testing
-------
Testing was done inside a VM on a disk with:
  /sys/block/sdd/mq/0/nr_tags     = 1024
  /sys/block/sdd/queue/nr_requests = 256

The following SCSI INQUIRY micro-benchmark was run with N=100000:

  sg+SG_IO (v3), /dev/sg4:
    avg = 139.0 us, p50 = 128.9 us, p90 = 149.7 us, p99 = 301.0 us

  bsg+SG_IO (v4), /dev/bsg/2:0:0:0:
    avg = 97.2 us,  p50 = 92.7 us,  p90 = 111.5 us, p99 = 150.9 us

  bsg+io_uring, /dev/bsg/2:0:0:0:
    avg = 105.9 us, p50 = 95.4 us,  p90 = 116.2 us, p99 = 175.0 us

  bsg+io_uring (batch=64), /dev/bsg/2:0:0:0:
    avg = 61.9 us,  p50 = 60.9 us,  p90 = 63.9 us,  p99 = 94.6 us

These results show that the new io_uring path is comparable to the
existing BSG SG_IO interface for single-command workloads, and that
batched io_uring submission can significantly improve latency for
high-concurrency workloads when the device supports sufficient queue
depth.

Yang Xiuwei (3):
  bsg: add bsg_uring_cmd uapi structure
  bsg: add io_uring command support to generic layer
  scsi: bsg: add io_uring passthrough handler

 block/bsg-lib.c          |   2 +-
 block/bsg.c              |  36 +++++++-
 drivers/scsi/scsi_bsg.c  | 181 ++++++++++++++++++++++++++++++++++++++-
 include/linux/bsg.h      |   6 +-
 include/uapi/linux/bsg.h |  50 +++++++++++
 5 files changed, 271 insertions(+), 4 deletions(-)

-- 
2.25.1

[PATCH v7 1/3] bsg: add bsg_uring_cmd uapi structure

[Yang Xiuwei]

Add the bsg_uring_cmd structure to the BSG UAPI header to support
io_uring-based SCSI passthrough operations via IORING_OP_URING_CMD.

Signed-off-by: Yang Xiuwei <yangxiuwei@kylinos.cn>
---
 include/uapi/linux/bsg.h | 49 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/include/uapi/linux/bsg.h b/include/uapi/linux/bsg.h
index cd6302def5ed..757118660d86 100644
--- a/include/uapi/linux/bsg.h
+++ b/include/uapi/linux/bsg.h
@@ -63,5 +63,54 @@ struct sg_io_v4 {
 	__u32 padding;
 };
 
+struct bsg_uring_cmd {
+	__u64 request;		/* [i], [*i] command descriptor address */
+	__u32 request_len;	/* [i] command descriptor length in bytes */
+	__u32 protocol;		/* [i] protocol type (BSG_PROTOCOL_*) */
+	__u32 subprotocol;	/* [i] subprotocol type (BSG_SUB_PROTOCOL_*) */
+	__u32 max_response_len;	/* [i] response buffer size in bytes */
+
+	__u64 response;		/* [i], [*o] response data address */
+	__u64 dout_xferp;	/* [i], [*i] */
+	__u32 dout_xfer_len;	/* [i] bytes to be transferred to device */
+	__u32 dout_iovec_count;	/* [i] 0 -> "flat" dout transfer else
+				 * dout_xferp points to array of iovec
+				 */
+	__u64 din_xferp;	/* [i], [*o] */
+	__u32 din_xfer_len;	/* [i] bytes to be transferred from device */
+	__u32 din_iovec_count;	/* [i] 0 -> "flat" din transfer */
+
+	__u32 timeout_ms;	/* [i] timeout in milliseconds */
+	__u8  reserved[12];	/* reserved for future extension */
+};
+
+/*
+ * SCSI BSG io_uring completion (res2, 64-bit)
+ *
+ * When using BSG_PROTOCOL_SCSI + BSG_SUB_PROTOCOL_SCSI_CMD with
+ * IORING_OP_URING_CMD, the completion queue entry (CQE) contains:
+ *   - result: errno (0 on success)
+ *   - res2: packed SCSI status; see macros below to decode.
+ *
+ * res2 bit layout:
+ *   [0..7]   device_status  (SCSI status byte, e.g. CHECK_CONDITION)
+ *   [8..15]  driver_status  (e.g. DRIVER_SENSE when sense data is valid)
+ *   [16..23] host_status    (e.g. DID_OK, DID_TIME_OUT)
+ *   [24..31] sense_len_wr   (bytes of sense data written to response buffer)
+ *   [32..63] resid_len      (residual transfer length)
+ */
+#define BSG_SCSI_RES2_DEVICE_STATUS(res2)   ((__u8)((__u64)(res2) & 0xff))
+#define BSG_SCSI_RES2_DRIVER_STATUS(res2)   ((__u8)((__u64)(res2) >> 8))
+#define BSG_SCSI_RES2_HOST_STATUS(res2)     ((__u8)((__u64)(res2) >> 16))
+#define BSG_SCSI_RES2_SENSE_LEN(res2)       ((__u8)((__u64)(res2) >> 24))
+#define BSG_SCSI_RES2_RESID_LEN(res2)       ((__u32)((__u64)(res2) >> 32))
+
+#define BSG_SCSI_RES2_BUILD(device_status, driver_status, host_status,   \
+			    sense_len_wr, resid_len)			\
+	(((__u64)(__u32)(resid_len) << 32) |				\
+	 ((__u64)(__u8)(sense_len_wr) << 24) |				\
+	 ((__u64)(__u8)(host_status) << 16) |				\
+	 ((__u64)(__u8)(driver_status) << 8) |				\
+	 ((__u64)(__u8)(device_status)))
 
 #endif /* _UAPIBSG_H */
-- 
2.25.1

Re: [PATCH v7 1/3] bsg: add bsg_uring_cmd uapi structure

[Bart Van Assche]

On 3/12/26 2:22 AM, Yang Xiuwei wrote:
> +struct bsg_uring_cmd {
> +	__u64 request;		/* [i], [*i] command descriptor address */
> +	__u32 request_len;	/* [i] command descriptor length in bytes */
> +	__u32 protocol;		/* [i] protocol type (BSG_PROTOCOL_*) */
> +	__u32 subprotocol;	/* [i] subprotocol type (BSG_SUB_PROTOCOL_*) */
> +	__u32 max_response_len;	/* [i] response buffer size in bytes */
> +
> +	__u64 response;		/* [i], [*o] response data address */
> +	__u64 dout_xferp;	/* [i], [*i] */
> +	__u32 dout_xfer_len;	/* [i] bytes to be transferred to device */
> +	__u32 dout_iovec_count;	/* [i] 0 -> "flat" dout transfer else
> +				 * dout_xferp points to array of iovec
> +				 */
> +	__u64 din_xferp;	/* [i], [*o] */
> +	__u32 din_xfer_len;	/* [i] bytes to be transferred from device */
> +	__u32 din_iovec_count;	/* [i] 0 -> "flat" din transfer */
> +
> +	__u32 timeout_ms;	/* [i] timeout in milliseconds */
> +	__u8  reserved[12];	/* reserved for future extension */
> +};

Please consider adding a static_assert() statement that verifies the
size of this data structure at compile time. Such a statement is useful
to document the size of the data structure, helps with verifying that
the size is the same on all architectures and helps with verifying that
the size doesn't change if a reserved byte is taken in use.

> +#define BSG_SCSI_RES2_DEVICE_STATUS(res2)   ((__u8)((__u64)(res2) & 0xff))
> +#define BSG_SCSI_RES2_DRIVER_STATUS(res2)   ((__u8)((__u64)(res2) >> 8))
> +#define BSG_SCSI_RES2_HOST_STATUS(res2)     ((__u8)((__u64)(res2) >> 16))
> +#define BSG_SCSI_RES2_SENSE_LEN(res2)       ((__u8)((__u64)(res2) >> 24))
> +#define BSG_SCSI_RES2_RESID_LEN(res2)       ((__u32)((__u64)(res2) >> 32))
> +
> +#define BSG_SCSI_RES2_BUILD(device_status, driver_status, host_status,   \
> +			    sense_len_wr, resid_len)			\
> +	(((__u64)(__u32)(resid_len) << 32) |				\
> +	 ((__u64)(__u8)(sense_len_wr) << 24) |				\
> +	 ((__u64)(__u8)(host_status) << 16) |				\
> +	 ((__u64)(__u8)(driver_status) << 8) |				\
> +	 ((__u64)(__u8)(device_status)))

Please convert all the above macros into inline functions because that
will result in removal of most of the typecasts. I think that inline
functions are allowed in uapi headers:

$ git grep 'static.*inline' include/uapi | wc -l
354

Thanks,

Bart.

Re: [PATCH v7 1/3] bsg: add bsg_uring_cmd uapi structure

[Yang Xiuwei]

Hi Bart,

On 3/12/26 7:29 PM, Bart Van Assche wrote:
> On 3/12/26 2:22 AM, Yang Xiuwei wrote:
>> +struct bsg_uring_cmd {
>> +	__u64 request;		/* [i], [*i] command descriptor address */
>> +	__u32 request_len;	/* [i] command descriptor length in bytes */
>> +	__u32 protocol;		/* [i] protocol type (BSG_PROTOCOL_*) */
>> +	__u32 subprotocol;	/* [i] subprotocol type (BSG_SUB_PROTOCOL_*) */
>> +	__u32 max_response_len;	/* [i] response buffer size in bytes */
>> +
>> +	__u64 response;		/* [i], [*o] response data address */
>> +	__u64 dout_xferp;	/* [i], [*i] */
>> +	__u32 dout_xfer_len;	/* [i] bytes to be transferred to device */
>> +	__u32 dout_iovec_count;	/* [i] 0 -> "flat" dout transfer else
>> +				 * dout_xferp points to array of iovec
>> +				 */
>> +	__u64 din_xferp;	/* [i], [*o] */
>> +	__u32 din_xfer_len;	/* [i] bytes to be transferred from device */
>> +	__u32 din_iovec_count;	/* [i] 0 -> "flat" din transfer */
>> +
>> +	__u32 timeout_ms;	/* [i] timeout in milliseconds */
>> +	__u8  reserved[12];	/* reserved for future extension */
>> +};
>
> Please consider adding a static_assert() statement that verifies the
> size of this data structure at compile time. Such a statement is useful
> to document the size of the data structure, helps with verifying that
> the size is the same on all architectures and helps with verifying that
> the size doesn't change if a reserved byte is taken in use.

Good point. In v8 I plan to define BSG_URING_CMD_SIZE in the UAPI header and
add a static_assert in drivers/scsi/scsi_bsg.c to verify at compile time that
sizeof(struct bsg_uring_cmd) matches that macro, e.g.:

	/* include/uapi/linux/bsg.h */
	#define BSG_URING_CMD_SIZE	80

	/* drivers/scsi/scsi_bsg.c */
	static_assert(sizeof(struct bsg_uring_cmd) == BSG_URING_CMD_SIZE);

Does this arrangement look reasonable to you?

>> +#define BSG_SCSI_RES2_DEVICE_STATUS(res2)   ((__u8)((__u64)(res2) & 0xff))
>> +#define BSG_SCSI_RES2_DRIVER_STATUS(res2)   ((__u8)((__u64)(res2) >> 8))
>> +#define BSG_SCSI_RES2_HOST_STATUS(res2)     ((__u8)((__u64)(res2) >> 16))
>> +#define BSG_SCSI_RES2_SENSE_LEN(res2)       ((__u8)((__u64)(res2) >> 24))
>> +#define BSG_SCSI_RES2_RESID_LEN(res2)       ((__u32)((__u64)(res2) >> 32))
>> +
>> +#define BSG_SCSI_RES2_BUILD(device_status, driver_status, host_status,   \
>> +			    sense_len_wr, resid_len)			\
>> +	(((__u64)(__u32)(resid_len) << 32) |				\
>> +	 ((__u64)(__u8)(sense_len_wr) << 24) |				\
>> +	 ((__u64)(__u8)(host_status) << 16) |				\
>> +	 ((__u64)(__u8)(driver_status) << 8) |				\
>> +	 ((__u64)(__u8)(device_status)))
>
> Please convert all the above macros into inline functions because that
> will result in removal of most of the typecasts. I think that inline
> functions are allowed in uapi headers:
>
> $ git grep 'static.*inline' include/uapi | wc -l
> 354

Done. I have replaced the BSG_SCSI_RES2_* macros with static inline
functions (bsg_scsi_res2_device_status(), bsg_scsi_res2_driver_status(),
bsg_scsi_res2_host_status(), bsg_scsi_res2_sense_len(),
bsg_scsi_res2_resid_len(), and bsg_scsi_res2_build()) in the UAPI header
to improve type safety and remove redundant type casts as suggested.

Best regards,
Yang Xiuwei

Re: [PATCH v7 1/3] bsg: add bsg_uring_cmd uapi structure

[Bart Van Assche]

On 3/13/26 12:34 AM, Yang Xiuwei wrote:
> 	/* include/uapi/linux/bsg.h */
> 	#define BSG_URING_CMD_SIZE	80
> 
> 	/* drivers/scsi/scsi_bsg.c */
> 	static_assert(sizeof(struct bsg_uring_cmd) == BSG_URING_CMD_SIZE);
> 
> Does this arrangement look reasonable to you?

Wouldn't it be simpler to add the following statement just below the
struct bsg_uring_cmd definition, or in other words, not to introduce a
new #define?

	static_assert(sizeof(struct bsg_uring_cmd) == 80);

There are already static_assert() statements in uapi header files:

$ git grep -nH 'static_assert.*sizeof' include/uapi
include/uapi/cxl/features.h:22:static_assert(sizeof(__uapi_uuid_t) == 
sizeof(uuid_t));
include/uapi/linux/media/amlogic/c3-isp-config.h:516:static_assert((sizeof(struct 
c3_isp_params_cfg) - C3_ISP_PARAMS_MAX_SIZE) ==
include/uapi/linux/rkisp1-config.h:1617:static_assert((sizeof(struct 
rkisp1_ext_params_cfg) -

Thanks,

Bart.

[PATCH v7 2/3] bsg: add io_uring command support to generic layer

[Yang Xiuwei]

Add an io_uring command handler to the generic BSG layer. The new
.uring_cmd file operation validates io_uring features and delegates
handling to a per-queue bsg_uring_cmd_fn callback.

Extend bsg_register_queue() so transport drivers can register both
sg_io and io_uring command handlers.

Signed-off-by: Yang Xiuwei <yangxiuwei@kylinos.cn>
---
 block/bsg-lib.c         |  2 +-
 block/bsg.c             | 36 +++++++++++++++++++++++++++++++++++-
 drivers/scsi/scsi_bsg.c | 10 +++++++++-
 include/linux/bsg.h     |  6 +++++-
 4 files changed, 50 insertions(+), 4 deletions(-)

diff --git a/block/bsg-lib.c b/block/bsg-lib.c
index 20cd0ef3c394..fdb4b290ca68 100644
--- a/block/bsg-lib.c
+++ b/block/bsg-lib.c
@@ -393,7 +393,7 @@ struct request_queue *bsg_setup_queue(struct device *dev, const char *name,
 
 	blk_queue_rq_timeout(q, BLK_DEFAULT_SG_TIMEOUT);
 
-	bset->bd = bsg_register_queue(q, dev, name, bsg_transport_sg_io_fn);
+	bset->bd = bsg_register_queue(q, dev, name, bsg_transport_sg_io_fn, NULL);
 	if (IS_ERR(bset->bd)) {
 		ret = PTR_ERR(bset->bd);
 		goto out_cleanup_queue;
diff --git a/block/bsg.c b/block/bsg.c
index e0af6206ed28..4eaea66ad492 100644
--- a/block/bsg.c
+++ b/block/bsg.c
@@ -12,6 +12,7 @@
 #include <linux/idr.h>
 #include <linux/bsg.h>
 #include <linux/slab.h>
+#include <linux/io_uring/cmd.h>
 
 #include <scsi/scsi.h>
 #include <scsi/scsi_ioctl.h>
@@ -28,6 +29,7 @@ struct bsg_device {
 	unsigned int timeout;
 	unsigned int reserved_size;
 	bsg_sg_io_fn *sg_io_fn;
+	bsg_uring_cmd_fn *uring_cmd_fn;
 };
 
 static inline struct bsg_device *to_bsg_device(struct inode *inode)
@@ -158,11 +160,41 @@ static long bsg_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	}
 }
 
+static int bsg_check_uring_features(unsigned int issue_flags)
+{
+	/* BSG passthrough requires big SQE/CQE support */
+	if ((issue_flags & (IO_URING_F_SQE128|IO_URING_F_CQE32)) !=
+	    (IO_URING_F_SQE128|IO_URING_F_CQE32))
+		return -EOPNOTSUPP;
+	return 0;
+}
+
+static int bsg_uring_cmd(struct io_uring_cmd *ioucmd, unsigned int issue_flags)
+{
+	struct request_queue *q;
+	struct bsg_device *bd;
+	bool open_for_write = ioucmd->file->f_mode & FMODE_WRITE;
+	int ret;
+
+	bd = to_bsg_device(file_inode(ioucmd->file));
+	q = bd->queue;
+
+	ret = bsg_check_uring_features(issue_flags);
+	if (ret)
+		return ret;
+
+	if (!bd->uring_cmd_fn)
+		return -EOPNOTSUPP;
+
+	return bd->uring_cmd_fn(q, ioucmd, issue_flags, open_for_write);
+}
+
 static const struct file_operations bsg_fops = {
 	.open		=	bsg_open,
 	.release	=	bsg_release,
 	.unlocked_ioctl	=	bsg_ioctl,
 	.compat_ioctl	=	compat_ptr_ioctl,
+	.uring_cmd	=	bsg_uring_cmd,
 	.owner		=	THIS_MODULE,
 	.llseek		=	default_llseek,
 };
@@ -187,7 +219,8 @@ void bsg_unregister_queue(struct bsg_device *bd)
 EXPORT_SYMBOL_GPL(bsg_unregister_queue);
 
 struct bsg_device *bsg_register_queue(struct request_queue *q,
-		struct device *parent, const char *name, bsg_sg_io_fn *sg_io_fn)
+		struct device *parent, const char *name, bsg_sg_io_fn *sg_io_fn,
+		bsg_uring_cmd_fn *uring_cmd_fn)
 {
 	struct bsg_device *bd;
 	int ret;
@@ -199,6 +232,7 @@ struct bsg_device *bsg_register_queue(struct request_queue *q,
 	bd->reserved_size = INT_MAX;
 	bd->queue = q;
 	bd->sg_io_fn = sg_io_fn;
+	bd->uring_cmd_fn = uring_cmd_fn;
 
 	ret = ida_alloc_max(&bsg_minor_ida, BSG_MAX_DEVS - 1, GFP_KERNEL);
 	if (ret < 0) {
diff --git a/drivers/scsi/scsi_bsg.c b/drivers/scsi/scsi_bsg.c
index a9a9ec086a7e..4d57e524e141 100644
--- a/drivers/scsi/scsi_bsg.c
+++ b/drivers/scsi/scsi_bsg.c
@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 #include <linux/bsg.h>
+#include <linux/io_uring/cmd.h>
 #include <scsi/scsi.h>
 #include <scsi/scsi_ioctl.h>
 #include <scsi/scsi_cmnd.h>
@@ -9,6 +10,12 @@
 
 #define uptr64(val) ((void __user *)(uintptr_t)(val))
 
+static int scsi_bsg_uring_cmd(struct request_queue *q, struct io_uring_cmd *ioucmd,
+			       unsigned int issue_flags, bool open_for_write)
+{
+	return -EOPNOTSUPP;
+}
+
 static int scsi_bsg_sg_io_fn(struct request_queue *q, struct sg_io_v4 *hdr,
 		bool open_for_write, unsigned int timeout)
 {
@@ -99,5 +106,6 @@ static int scsi_bsg_sg_io_fn(struct request_queue *q, struct sg_io_v4 *hdr,
 struct bsg_device *scsi_bsg_register_queue(struct scsi_device *sdev)
 {
 	return bsg_register_queue(sdev->request_queue, &sdev->sdev_gendev,
-			dev_name(&sdev->sdev_gendev), scsi_bsg_sg_io_fn);
+			dev_name(&sdev->sdev_gendev), scsi_bsg_sg_io_fn,
+			scsi_bsg_uring_cmd);
 }
diff --git a/include/linux/bsg.h b/include/linux/bsg.h
index ee2df73edf83..162730bfc2d8 100644
--- a/include/linux/bsg.h
+++ b/include/linux/bsg.h
@@ -7,13 +7,17 @@
 struct bsg_device;
 struct device;
 struct request_queue;
+struct io_uring_cmd;
 
 typedef int (bsg_sg_io_fn)(struct request_queue *, struct sg_io_v4 *hdr,
 		bool open_for_write, unsigned int timeout);
 
+typedef int (bsg_uring_cmd_fn)(struct request_queue *q, struct io_uring_cmd *ioucmd,
+			       unsigned int issue_flags, bool open_for_write);
+
 struct bsg_device *bsg_register_queue(struct request_queue *q,
 		struct device *parent, const char *name,
-		bsg_sg_io_fn *sg_io_fn);
+		bsg_sg_io_fn *sg_io_fn, bsg_uring_cmd_fn *uring_cmd_fn);
 void bsg_unregister_queue(struct bsg_device *bcd);
 
 #endif /* _LINUX_BSG_H */
-- 
2.25.1

Re: [PATCH v7 2/3] bsg: add io_uring command support to generic layer

[Bart Van Assche]

On 3/12/26 2:22 AM, Yang Xiuwei wrote:
> +static int bsg_uring_cmd(struct io_uring_cmd *ioucmd, unsigned int issue_flags)
> +{
> +	struct request_queue *q;
> +	struct bsg_device *bd;
> +	bool open_for_write = ioucmd->file->f_mode & FMODE_WRITE;
> +	int ret;
> +
> +	bd = to_bsg_device(file_inode(ioucmd->file));
> +	q = bd->queue;

Please combine the above assignments with the declarations of the
modified variables.

Thanks,

Bart.

Re: [PATCH v7 2/3] bsg: add io_uring command support to generic layer

[Yang Xiuwei]

Hi Bart,

On 3/12/26 7:33 PM, Bart Van Assche wrote:
> On 3/12/26 2:22 AM, Yang Xiuwei wrote:
>> +static int bsg_uring_cmd(struct io_uring_cmd *ioucmd, unsigned int issue_flags)
>> +{
>> +	struct request_queue *q;
>> +	struct bsg_device *bd;
>> +	bool open_for_write = ioucmd->file->f_mode & FMODE_WRITE;
>> +	int ret;
>> +
>> +	bd = to_bsg_device(file_inode(ioucmd->file));
>> +	q = bd->queue;
>>
>> Please combine the above assignments with the declarations of the
>> modified variables.
>

To follow your suggestion, I now combine the declarations with the
initializations, but I had to keep the dependency order between `bd`
and `q`, since `q` is initialized from `bd->queue`. The current version
looks like this:

        struct bsg_device *bd = to_bsg_device(file_inode(ioucmd->file));
        bool open_for_write = ioucmd->file->f_mode & FMODE_WRITE;
        struct request_queue *q = bd->queue;
        int ret;
		
This way we respect the declaration+initialization style and keep the
data dependency clear, while following the reverse Christmas tree style
as far as it does not conflict with the dependency.
Does this arrangement look reasonable to you, or would you prefer a
different ordering here?

Thanks,
Yang Xiuwei

Re: [PATCH v7 2/3] bsg: add io_uring command support to generic layer

[Bart Van Assche]

On 3/13/26 12:34 AM, Yang Xiuwei wrote:
> To follow your suggestion, I now combine the declarations with the
> initializations, but I had to keep the dependency order between `bd`
> and `q`, since `q` is initialized from `bd->queue`. The current version
> looks like this:
> 
>          struct bsg_device *bd = to_bsg_device(file_inode(ioucmd->file));
>          bool open_for_write = ioucmd->file->f_mode & FMODE_WRITE;
>          struct request_queue *q = bd->queue;
>          int ret;
> 		
> This way we respect the declaration+initialization style and keep the
> data dependency clear, while following the reverse Christmas tree style
> as far as it does not conflict with the dependency.
> Does this arrangement look reasonable to you, or would you prefer a
> different ordering here?

That sounds good to me and I think this is the style followed elsewhere 
in the Linux kernel.

Thanks,

Bart.

[PATCH v7 3/3] scsi: bsg: add io_uring passthrough handler

[Yang Xiuwei]

Implement the SCSI-specific io_uring command handler for BSG using
struct bsg_uring_cmd.

The handler builds a SCSI request from the io_uring command, maps user
buffers (including fixed buffers), and completes asynchronously via a
request end_io callback and task_work. Completion returns a 32-bit
status and packed residual/sense information via CQE res and res2, and
supports IO_URING_F_NONBLOCK.

Signed-off-by: Yang Xiuwei <yangxiuwei@kylinos.cn>
---
 drivers/scsi/scsi_bsg.c | 173 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 172 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_bsg.c b/drivers/scsi/scsi_bsg.c
index 4d57e524e141..da227fa890e0 100644
--- a/drivers/scsi/scsi_bsg.c
+++ b/drivers/scsi/scsi_bsg.c
@@ -10,10 +10,181 @@
 
 #define uptr64(val) ((void __user *)(uintptr_t)(val))
 
+/*
+ * Per-command BSG SCSI PDU stored in io_uring_cmd.pdu[32].
+ * Holds temporary state between submission, completion and task_work.
+ */
+struct scsi_bsg_uring_cmd_pdu {
+	struct bio *bio;		/* mapped user buffer, unmap in task work */
+	struct request *req;		/* block request, freed in task work */
+	u64 response_addr;		/* user space response buffer address */
+};
+
+static inline struct scsi_bsg_uring_cmd_pdu *scsi_bsg_uring_cmd_pdu(
+	struct io_uring_cmd *ioucmd)
+{
+	return io_uring_cmd_to_pdu(ioucmd, struct scsi_bsg_uring_cmd_pdu);
+}
+
+/* Task work: build res2 (layout in uapi/linux/bsg.h) and copy sense to user. */
+static void scsi_bsg_uring_task_cb(struct io_tw_req tw_req, io_tw_token_t tw)
+{
+	struct scsi_bsg_uring_cmd_pdu *pdu;
+	struct io_uring_cmd *ioucmd = io_uring_cmd_from_tw(tw_req);
+	struct scsi_cmnd *scmd;
+	struct request *rq;
+	u64 res2;
+	int ret = 0;
+	u8 driver_status = 0;
+	u8 sense_len_wr = 0;
+
+	pdu = scsi_bsg_uring_cmd_pdu(ioucmd);
+	rq = pdu->req;
+	scmd = blk_mq_rq_to_pdu(rq);
+
+	if (pdu->bio)
+		blk_rq_unmap_user(pdu->bio);
+
+	if (scsi_status_is_check_condition(scmd->result)) {
+		driver_status = DRIVER_SENSE;
+		if (pdu->response_addr)
+			sense_len_wr = min_t(u8, scmd->sense_len,
+					     SCSI_SENSE_BUFFERSIZE);
+	}
+
+	if (sense_len_wr) {
+		if (copy_to_user(uptr64(pdu->response_addr), scmd->sense_buffer,
+				 sense_len_wr))
+			ret = -EFAULT;
+	}
+
+	res2 = BSG_SCSI_RES2_BUILD(status_byte(scmd->result), driver_status,
+				  host_byte(scmd->result), sense_len_wr,
+				  scmd->resid_len);
+
+	blk_mq_free_request(rq);
+	io_uring_cmd_done32(ioucmd, ret, res2,
+			    IO_URING_CMD_TASK_WORK_ISSUE_FLAGS);
+}
+
+static enum rq_end_io_ret scsi_bsg_uring_cmd_done(struct request *req,
+						  blk_status_t status,
+						  const struct io_comp_batch *iocb)
+{
+	struct io_uring_cmd *ioucmd = req->end_io_data;
+
+	io_uring_cmd_do_in_task_lazy(ioucmd, scsi_bsg_uring_task_cb);
+	return RQ_END_IO_NONE;
+}
+
+static int scsi_bsg_map_user_buffer(struct request *req,
+				    struct io_uring_cmd *ioucmd,
+				    unsigned int issue_flags, gfp_t gfp_mask)
+{
+	const struct bsg_uring_cmd *cmd = io_uring_sqe128_cmd(ioucmd->sqe, struct bsg_uring_cmd);
+	struct iov_iter iter;
+	unsigned long buf_len;
+	bool is_write = cmd->dout_xfer_len > 0;
+	u64 buf_addr = is_write ? cmd->dout_xferp : cmd->din_xferp;
+	int ret;
+
+	buf_len = is_write ? cmd->dout_xfer_len : cmd->din_xfer_len;
+
+	if (ioucmd->flags & IORING_URING_CMD_FIXED) {
+		ret = io_uring_cmd_import_fixed(buf_addr, buf_len,
+						is_write ? WRITE : READ,
+						&iter, ioucmd, issue_flags);
+		if (ret < 0)
+			return ret;
+		ret = blk_rq_map_user_iov(req->q, req, NULL, &iter, gfp_mask);
+	} else {
+		ret = blk_rq_map_user(req->q, req, NULL, uptr64(buf_addr),
+				      buf_len, gfp_mask);
+	}
+
+	return ret;
+}
+
 static int scsi_bsg_uring_cmd(struct request_queue *q, struct io_uring_cmd *ioucmd,
 			       unsigned int issue_flags, bool open_for_write)
 {
-	return -EOPNOTSUPP;
+	struct scsi_bsg_uring_cmd_pdu *pdu = scsi_bsg_uring_cmd_pdu(ioucmd);
+	const struct bsg_uring_cmd *cmd = io_uring_sqe128_cmd(ioucmd->sqe, struct bsg_uring_cmd);
+	struct scsi_cmnd *scmd;
+	blk_mq_req_flags_t blk_flags = 0;
+	struct request *req;
+	gfp_t gfp_mask = GFP_KERNEL;
+	int ret = 0;
+
+	if (cmd->protocol != BSG_PROTOCOL_SCSI ||
+	    cmd->subprotocol != BSG_SUB_PROTOCOL_SCSI_CMD)
+		return -EINVAL;
+
+	if (!cmd->request || cmd->request_len == 0)
+		return -EINVAL;
+
+	if (cmd->dout_xfer_len && cmd->din_xfer_len) {
+		pr_warn_once("BIDI support in bsg has been removed.\n");
+		return -EOPNOTSUPP;
+	}
+
+	if (cmd->dout_iovec_count > 0 || cmd->din_iovec_count > 0)
+		return -EOPNOTSUPP;
+
+	if (issue_flags & IO_URING_F_NONBLOCK) {
+		blk_flags = BLK_MQ_REQ_NOWAIT;
+		gfp_mask = GFP_NOWAIT;
+	}
+
+	req = scsi_alloc_request(q, cmd->dout_xfer_len ?
+				 REQ_OP_DRV_OUT : REQ_OP_DRV_IN, blk_flags);
+	if (IS_ERR(req))
+		return PTR_ERR(req);
+
+	scmd = blk_mq_rq_to_pdu(req);
+	scmd->cmd_len = cmd->request_len;
+	if (scmd->cmd_len > sizeof(scmd->cmnd)) {
+		ret = -EINVAL;
+		goto out_free_req;
+	}
+	scmd->allowed = SG_DEFAULT_RETRIES;
+
+	if (copy_from_user(scmd->cmnd, uptr64(cmd->request), cmd->request_len)) {
+		ret = -EFAULT;
+		goto out_free_req;
+	}
+
+	if (!scsi_cmd_allowed(scmd->cmnd, open_for_write)) {
+		ret = -EPERM;
+		goto out_free_req;
+	}
+
+	pdu->response_addr = cmd->response;
+	scmd->sense_len = cmd->max_response_len ?
+		min(cmd->max_response_len, SCSI_SENSE_BUFFERSIZE) : SCSI_SENSE_BUFFERSIZE;
+
+	if (cmd->dout_xfer_len || cmd->din_xfer_len) {
+		ret = scsi_bsg_map_user_buffer(req, ioucmd, issue_flags, gfp_mask);
+		if (ret)
+			goto out_free_req;
+		pdu->bio = req->bio;
+	} else {
+		pdu->bio = NULL;
+	}
+
+	req->timeout = cmd->timeout_ms ?
+		msecs_to_jiffies(cmd->timeout_ms) : BLK_DEFAULT_SG_TIMEOUT;
+
+	req->end_io = scsi_bsg_uring_cmd_done;
+	req->end_io_data = ioucmd;
+	pdu->req = req;
+
+	blk_execute_rq_nowait(req, false);
+	return -EIOCBQUEUED;
+
+out_free_req:
+	blk_mq_free_request(req);
+	return ret;
 }
 
 static int scsi_bsg_sg_io_fn(struct request_queue *q, struct sg_io_v4 *hdr,
-- 
2.25.1

Re: [PATCH v7 3/3] scsi: bsg: add io_uring passthrough handler

[Bart Van Assche]

On 3/12/26 2:22 AM, Yang Xiuwei wrote:
> +/*
> + * Per-command BSG SCSI PDU stored in io_uring_cmd.pdu[32].
> + * Holds temporary state between submission, completion and task_work.
> + */
> +struct scsi_bsg_uring_cmd_pdu {
> +	struct bio *bio;		/* mapped user buffer, unmap in task work */
> +	struct request *req;		/* block request, freed in task work */
> +	u64 response_addr;		/* user space response buffer address */
> +};

A static_assert() that verifies that sizeof(struct
scsi_bsg_uring_cmd_pdu) is less than or equal to the size of
((struct io_uring_cmd *)NULL)->pdu seems appropriate here.

> +/* Task work: build res2 (layout in uapi/linux/bsg.h) and copy sense to user. */
> +static void scsi_bsg_uring_task_cb(struct io_tw_req tw_req, io_tw_token_t tw)
> +{
> +	struct scsi_bsg_uring_cmd_pdu *pdu;
> +	struct io_uring_cmd *ioucmd = io_uring_cmd_from_tw(tw_req);
> +	struct scsi_cmnd *scmd;
> +	struct request *rq;
> +	u64 res2;
> +	int ret = 0;
> +	u8 driver_status = 0;
> +	u8 sense_len_wr = 0;
> +
> +	pdu = scsi_bsg_uring_cmd_pdu(ioucmd);
> +	rq = pdu->req;
> +	scmd = blk_mq_rq_to_pdu(rq);

Please combine the above three assignments with the above declarations
since that is the style followed by most kernel code.

Otherwise this patch series looks good to me.

Thanks,

Bart.

Re: [PATCH v7 3/3] scsi: bsg: add io_uring passthrough handler

[Yang Xiuwei]

Hi Bart,

On 3/12/26 7:41 PM, Bart Van Assche wrote:
> On 3/12/26 2:22 AM, Yang Xiuwei wrote:
>> +/*
>> + * Per-command BSG SCSI PDU stored in io_uring_cmd.pdu[32].
>> + * Holds temporary state between submission, completion and task_work.
>> + */
>> +struct scsi_bsg_uring_cmd_pdu {
>> +	struct bio *bio;		/* mapped user buffer, unmap in task work */
>> +	struct request *req;		/* block request, freed in task work */
>> +	u64 response_addr;		/* user space response buffer address */
>> +};
>
> A static_assert() that verifies that sizeof(struct
> scsi_bsg_uring_cmd_pdu) is less than or equal to the size of
> ((struct io_uring_cmd *)NULL)->pdu seems appropriate here.
>

Will add a static_assert() in scsi_bsg.c to ensure the PDU fits in
io_uring_cmd->pdu, e.g.:

        static_assert(sizeof(struct scsi_bsg_uring_cmd_pdu) <=
                      sizeof_field(struct io_uring_cmd, pdu));

>> +struct scsi_bsg_uring_cmd_pdu *pdu;
>> +struct io_uring_cmd *ioucmd = io_uring_cmd_from_tw(tw_req);
>> +struct scsi_cmnd *scmd;
>> +struct request *rq;
>> +u64 res2;
>> +int ret = 0;
>> +u8 driver_status = 0;
>> +u8 sense_len_wr = 0;
>> +
>> +pdu = scsi_bsg_uring_cmd_pdu(ioucmd);
>> +rq = pdu->req;
>> +scmd = blk_mq_rq_to_pdu(rq);
>
> Please combine the above three assignments with the above declarations
> since that is the style followed by most kernel code.
>

Sorry for the noise here — in v6 I did not change this part, but in v7
I took the liberty to tweak the ordering myself. Done. I will combine
the declarations with initializers in scsi_bsg_uring_task_cb() in v8
while keeping the data dependency and reverse Christmas tree style, e.g.:

	struct io_uring_cmd *ioucmd = io_uring_cmd_from_tw(tw_req);
	struct scsi_bsg_uring_cmd_pdu *pdu = scsi_bsg_uring_cmd_pdu(ioucmd);
	struct request *rq = pdu->req;
	struct scsi_cmnd *scmd = blk_mq_rq_to_pdu(rq);

Additionally, for the `scsi_bsg_map_user_buffer()` helper in v7 I tried
to follow the same principle (dependency first, then length) when
ordering the initializations around `cmd`, `is_write`, `buf_addr` and
`buf_len`. If you think the current ordering there should be adjusted as
well, I'm happy to rework that in v8. In particular, when there is a
tension between keeping the reverse Christmas tree order and making
data/control dependencies explicit in the declaration order, I would
very much appreciate your advice on how to handle such cases, so that I
can do better here and in future patches.

Thanks for the review.

Best regards,
Yang Xiuwei

Re: [PATCH v7 3/3] scsi: bsg: add io_uring passthrough handler

[Bart Van Assche]

On 3/13/26 12:34 AM, Yang Xiuwei wrote:
> In particular, when there is a
> tension between keeping the reverse Christmas tree order and making
> data/control dependencies explicit in the declaration order, I would
> very much appreciate your advice on how to handle such cases, so that I
> can do better here and in future patches.

I think that combining declarations and initializations is considered 
more important than strictly ordering declarations from longest to
shortest.

Thanks,

Bart.