Re: [PATCH 5/6] ublk: add UBLK_F_IO_DESC_SIZE

[Ming Lei <tom.leiming@gmail.com>]

On Tue, Jul 28, 2026 at 07:29:50PM -0600, Caleb Sander Mateos wrote:
> ublk passes the parameters of incoming I/O in memory shared between the
> kernel ublk driver and userspace ublk server in struct ublksrv_io_desc.
> The size of this struct is currently fixed to 24 bytes, which has been
> an obstacle to extending it with additional fields [1]. Additionally,
> with multiple ublk server threads handling I/Os from the same ublk queue
> (possible with UBLK_F_PER_IO_DAEMON or UBLK_F_BATCH_IO), false sharing
> results from adjacent io_descs sharing the same cache line.
> 
> Add a ublk feature UBLK_F_IO_DESC_SIZE to allow a ublk server to
> override the size of each io_desc. The size must be at least 24 and a
> multiple of 8 to store a properly-aligned struct ublksrv_io_desc. The
> struct ublksrv_io_desc is located at the beginning of each io_desc and
> the remainder is padding. The mmap() performed for each queue must have
> a length of queue_depth * io_desc_size, rounded up to the page size. The
> mmap() offset must be q_id * UBLK_MAX_QUEUE_DEPTH * io_desc_size, also
> rounded up to the page size.
> 
> [1]: https://lore.kernel.org/linux-block/aV8QfvaNO5P6vOs6@fedora/
> 
> Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
> ---
>  drivers/block/ublk_drv.c      | 35 ++++++++++++++++++++++++-----------
>  include/uapi/linux/ublk_cmd.h |  5 ++++-
>  2 files changed, 28 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> index 46f5ab13f87e..1da45e382253 100644
> --- a/drivers/block/ublk_drv.c
> +++ b/drivers/block/ublk_drv.c
> @@ -87,11 +87,12 @@
>  		| UBLK_F_BUF_REG_OFF_DAEMON \
>  		| (IS_ENABLED(CONFIG_BLK_DEV_INTEGRITY) ? UBLK_F_INTEGRITY : 0) \
>  		| UBLK_F_SAFE_STOP_DEV \
>  		| UBLK_F_BATCH_IO \
>  		| UBLK_F_NO_AUTO_PART_SCAN \
> -		| UBLK_F_SHMEM_ZC)
> +		| UBLK_F_SHMEM_ZC \
> +		| UBLK_F_IO_DESC_SIZE)
>  
>  #define UBLK_F_ALL_RECOVERY_FLAGS (UBLK_F_USER_RECOVERY \
>  		| UBLK_F_USER_RECOVERY_REISSUE \
>  		| UBLK_F_USER_RECOVERY_FAIL_IO)
>  
> @@ -237,10 +238,11 @@ struct ublk_io {
>  } ____cacheline_aligned_in_smp;
>  
>  struct ublk_queue {
>  	u16 q_id;
>  	u16 q_depth;
> +	u16 io_desc_size;
>  
>  	unsigned long flags;
>  	struct ublksrv_io_desc *io_cmd_buf;
>  
>  	bool force_abort;
> @@ -403,11 +405,11 @@ static inline void ublk_io_evts_deinit(struct ublk_queue *q)
>  }
>  
>  static inline struct ublksrv_io_desc *
>  ublk_get_iod(const struct ublk_queue *ubq, u16 tag)
>  {
> -	return &ubq->io_cmd_buf[tag];
> +	return (void *)ubq->io_cmd_buf + tag * (size_t)ubq->io_desc_size;
>  }
>  
>  static inline bool ublk_support_zero_copy(const struct ublk_queue *ubq)
>  {
>  	return ubq->flags & UBLK_F_SUPPORT_ZERO_COPY;
> @@ -1246,23 +1248,24 @@ static inline struct ublksrv_io_desc *
>  ublk_queue_cmd_buf(struct ublk_device *ub, u16 q_id)
>  {
>  	return ublk_get_queue(ub, q_id)->io_cmd_buf;
>  }
>  
> -static inline int __ublk_queue_cmd_buf_size(u16 depth)
> +static inline size_t __ublk_queue_cmd_buf_size(const struct ublk_device *ub,
> +					       u16 depth)
>  {
> -	return round_up(depth * sizeof(struct ublksrv_io_desc), PAGE_SIZE);
> +	return round_up(depth * (size_t)ub->dev_info.io_desc_size, PAGE_SIZE);
>  }
>  
> -static inline int ublk_queue_cmd_buf_size(struct ublk_device *ub)
> +static inline size_t ublk_queue_cmd_buf_size(const struct ublk_device *ub)
>  {
> -	return __ublk_queue_cmd_buf_size(ub->dev_info.queue_depth);
> +	return __ublk_queue_cmd_buf_size(ub, ub->dev_info.queue_depth);
>  }
>  
> -static int ublk_max_cmd_buf_size(void)
> +static size_t ublk_max_cmd_buf_size(const struct ublk_device *ub)
>  {
> -	return __ublk_queue_cmd_buf_size(UBLK_MAX_QUEUE_DEPTH);
> +	return __ublk_queue_cmd_buf_size(ub, UBLK_MAX_QUEUE_DEPTH);
>  }
>  
>  /*
>   * Should I/O outstanding to the ublk server when it exits be reissued?
>   * If not, outstanding I/O will get errors.
> @@ -2648,11 +2651,11 @@ static int ublk_ch_release(struct inode *inode, struct file *filp)
>  /* map pre-allocated per-queue cmd buffer to ublksrv daemon */
>  static int ublk_ch_mmap(struct file *filp, struct vm_area_struct *vma)
>  {
>  	struct ublk_device *ub = filp->private_data;
>  	size_t sz = vma->vm_end - vma->vm_start;
> -	unsigned max_sz = ublk_max_cmd_buf_size();
> +	size_t max_sz = ublk_max_cmd_buf_size(ub);
>  	unsigned long pfn, end, phys_off = vma->vm_pgoff << PAGE_SHIFT;
>  	int ret = 0;
>  	u16 q_id;
>  
>  	spin_lock(&ub->lock);
> @@ -4174,11 +4177,11 @@ static const struct file_operations ublk_ch_batch_io_fops = {
>  	.mmap = ublk_ch_mmap,
>  };
>  
>  static void __ublk_deinit_queue(struct ublk_device *ub, struct ublk_queue *ubq)
>  {
> -	int size;
> +	size_t size;
>  	u16 i;
>  
>  	size = ublk_queue_cmd_buf_size(ub);
>  
>  	for (i = 0; i < ubq->q_depth; i++) {
> @@ -4227,11 +4230,12 @@ static int ublk_init_queue(struct ublk_device *ub, u16 q_id)
>  	u16 depth = ub->dev_info.queue_depth;
>  	gfp_t gfp_flags = GFP_KERNEL | __GFP_ZERO;
>  	struct ublk_queue *ubq;
>  	struct page *page;
>  	int numa_node;
> -	int size, ret;
> +	size_t size;
> +	int ret;
>  	u16 i;
>  
>  	/* Determine NUMA node based on queue's CPU affinity */
>  	numa_node = ublk_get_queue_numa_node(ub, q_id);
>  
> @@ -4252,10 +4256,11 @@ static int ublk_init_queue(struct ublk_device *ub, u16 q_id)
>  	if (!page) {
>  		kvfree(ubq);
>  		return -ENOMEM;
>  	}
>  	ubq->io_cmd_buf = page_address(page);
> +	ubq->io_desc_size = ub->dev_info.io_desc_size;
>  
>  	for (i = 0; i < ubq->q_depth; i++)
>  		spin_lock_init(&ubq->ios[i].lock);
>  
>  	if (ublk_dev_support_batch_io(ub)) {
> @@ -4736,10 +4741,18 @@ static int ublk_ctrl_add_dev(const struct ublksrv_ctrl_cmd *header)
>  
>  	/* User copy is required to access integrity buffer */
>  	if (info.flags & UBLK_F_INTEGRITY && !(info.flags & UBLK_F_USER_COPY))
>  		return -EINVAL;
>  
> +	if (info.flags & UBLK_F_IO_DESC_SIZE) {
> +		if (info.io_desc_size < sizeof(struct ublksrv_io_desc) ||
> +		    info.io_desc_size % _Alignof(struct ublksrv_io_desc))
> +			return -EINVAL;
> +	} else {
> +		info.io_desc_size = sizeof(struct ublksrv_io_desc);
> +	}
> +

No upper bound on io_desc_size.

Also Missing selftest update — this causes generic_13 failure.

New feature needs sanity test to show it working at least.

Thanks,
Ming