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

[Caleb Sander Mateos]

On Thu, Jul 30, 2026 at 8:14 AM Ming Lei <tom.leiming@xxxxxxxxx> wrote:
>
> 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@xxxxxxxxxxxxxxx>
> > ---
> >  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.

What upper bound would you suggest?

>
> Also Missing selftest update — this causes generic_13 failure.
>
> New feature needs sanity test to show it working at least.

Okay, will add a test.

Thanks,
Caleb