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

[Yang Xiuwei <yangxiuwei@kylinos.cn>]

On 1/12/26 17:44, Bart Van Assche wrote:
> On 1/12/26 1:46 AM, Yang Xiuwei wrote:
> > +struct bsg_uring_cmd {
> > +	__u64 cdb_addr;
> > +	__u8  cdb_len;
> > +	__u8  protocol;		/* [i] protocol type (BSG_PROTOCOL_*) */
> > +	__u8  subprotocol;	/* [i] subprotocol type (BSG_SUB_PROTOCOL_*) */
> > +	__u8  reserved1;
> > +	__u32 din_iovec_count;	/* [i] 0 -> flat din transfer else
> > +				 * din_xferp points to array of iovec
> > +				 */
> > +	__u32 din_xfer_len;	/* [i] bytes to be transferred from device */
> > +	__u64 din_xferp;	/* [i] data in buffer address or iovec array
> > +				 * address
> > +				 */
> > +	__u32 dout_iovec_count;	/* [i] 0 -> flat dout transfer else
> > +				 * dout_xferp points to array of iovec
> > +				 */
> > +	__u32 dout_xfer_len;	/* [i] bytes to be transferred to device */
> > +	__u64 dout_xferp;	/* [i] data out buffer address or iovec array address */
> > +	__u32 sense_len;
> > +	__u64 sense_addr;
> > +	__u32 timeout_ms;
> > +	__u32 flags;		/* [i] bit mask (BSG_FLAG_*) - reserved for future use */
> > +	__u8  reserved[16];	/* reserved for future extension */
> 
> BSG supports much more than only SCSI. The above seems to support SCSI
> commands only.

While the current BSG implementation only supports SCSI (BSG_PROTOCOL_SCSI
is the only defined protocol), I understand that the design should allow
for future protocol extensions. I notice that sg_io_v4 uses generic field
names (request, request_len) rather than SCSI-specific names (cdb), which
aligns with this design philosophy.

I'll change cdb_addr/cdb_len to more generic names (request_addr,
request_len) to match the naming convention used in sg_io_v4, even though
the current implementation is SCSI-specific.

> > +} __packed;
> 
> Applying __packed to a data structure in its entirety is wrong because
> it causes compilers to generate suboptimal code on architectures that do
> not support unaligned 16-/32-/64-bit accesses.

You're correct that using __packed on the entire structure can cause
suboptimal code generation on architectures that don't support unaligned
access. I'll remove __packed and reorganize the fields to ensure proper
alignment while maintaining the 80-byte size requirement to fit within the
128-byte SQE cmd field.

I'll prepare a revised patch addressing these issues in the next version
of the series.

As I'm relatively new to kernel development, I appreciate your detailed
feedback. I'll incorporate these improvements and continue learning from
the community.

Thanks again for your review!

Best regards,
Yang Xiuwei