Re: [PATCH V3 06/27] ublk: add helper of __ublk_fetch()

[Ming Lei <ming.lei@redhat.com>]

On Fri, Nov 14, 2025 at 09:21:53PM -0800, Caleb Sander Mateos wrote:
> On Wed, Nov 12, 2025 at 1:39 AM Ming Lei <ming.lei@redhat.com> wrote:
> >
> > Add helper __ublk_fetch() for refactoring ublk_fetch().
> >
> > Meantime move ublk_config_io_buf() out of __ublk_fetch() to make
> > the code structure cleaner.
> >
> > Signed-off-by: Ming Lei <ming.lei@redhat.com>
> > ---
> >  drivers/block/ublk_drv.c | 46 +++++++++++++++++++++-------------------
> >  1 file changed, 24 insertions(+), 22 deletions(-)
> >
> > diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> > index 5e83c1b2a69e..dd9c35758a46 100644
> > --- a/drivers/block/ublk_drv.c
> > +++ b/drivers/block/ublk_drv.c
> > @@ -2234,39 +2234,41 @@ static int ublk_check_fetch_buf(const struct ublk_device *ub, __u64 buf_addr)
> >         return 0;
> >  }
> >
> > -static int ublk_fetch(struct io_uring_cmd *cmd, struct ublk_device *ub,
> > -                     struct ublk_io *io, __u64 buf_addr)
> > +static int __ublk_fetch(struct io_uring_cmd *cmd, struct ublk_device *ub,
> > +                       struct ublk_io *io)
> >  {
> > -       int ret = 0;
> > -
> > -       /*
> > -        * When handling FETCH command for setting up ublk uring queue,
> > -        * ub->mutex is the innermost lock, and we won't block for handling
> > -        * FETCH, so it is fine even for IO_URING_F_NONBLOCK.
> > -        */
> > -       mutex_lock(&ub->mutex);
> >         /* UBLK_IO_FETCH_REQ is only allowed before dev is setup */
> > -       if (ublk_dev_ready(ub)) {
> > -               ret = -EBUSY;
> > -               goto out;
> > -       }
> > +       if (ublk_dev_ready(ub))
> > +               return -EBUSY;
> >
> >         /* allow each command to be FETCHed at most once */
> > -       if (io->flags & UBLK_IO_FLAG_ACTIVE) {
> > -               ret = -EINVAL;
> > -               goto out;
> > -       }
> > +       if (io->flags & UBLK_IO_FLAG_ACTIVE)
> > +               return -EINVAL;
> >
> >         WARN_ON_ONCE(io->flags & UBLK_IO_FLAG_OWNED_BY_SRV);
> >
> >         ublk_fill_io_cmd(io, cmd);
> > -       ret = ublk_config_io_buf(ub, io, cmd, buf_addr, NULL);
> > -       if (ret)
> > -               goto out;
> >
> >         WRITE_ONCE(io->task, get_task_struct(current));
> >         ublk_mark_io_ready(ub);
> > -out:
> > +
> > +       return 0;
> > +}
> > +
> > +static int ublk_fetch(struct io_uring_cmd *cmd, struct ublk_device *ub,
> > +                     struct ublk_io *io, __u64 buf_addr)
> > +{
> > +       int ret;
> > +
> > +       /*
> > +        * When handling FETCH command for setting up ublk uring queue,
> > +        * ub->mutex is the innermost lock, and we won't block for handling
> > +        * FETCH, so it is fine even for IO_URING_F_NONBLOCK.
> > +        */
> > +       mutex_lock(&ub->mutex);
> > +       ret = __ublk_fetch(cmd, ub, io);
> > +       if (!ret)
> > +               ret = ublk_config_io_buf(ub, io, cmd, buf_addr, NULL);
> 
> This changes ublk_config_io_buf() to be called *after*
> ublk_mark_io_ready(). Is that safe? It seems like io->addr could be
> read in ublk_setup_iod() as soon as the ublk device is marked as ready
> for I/O.

disk can't be added unless acquiring ub->mutex, so it is safe.


Thanks,
Ming