Re: [PATCH 06/23] ublk: prepare for not tracking task context for command batch

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

On Sat, Sep 06, 2025 at 11:48:08AM -0700, Caleb Sander Mateos wrote:
> On Mon, Sep 1, 2025 at 3:03 AM Ming Lei <ming.lei@redhat.com> wrote:
> >
> > batch io is designed to be independent of task context, and we will not
> > track task context for batch io feature.
> >
> > So warn on non-batch-io code paths.
> >
> > Signed-off-by: Ming Lei <ming.lei@redhat.com>
> > ---
> >  drivers/block/ublk_drv.c | 16 +++++++++++++++-
> >  1 file changed, 15 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> > index a0dfad8a56f0..46be5b656f22 100644
> > --- a/drivers/block/ublk_drv.c
> > +++ b/drivers/block/ublk_drv.c
> > @@ -261,6 +261,11 @@ static inline bool ublk_dev_support_batch_io(const struct ublk_device *ub)
> >         return false;
> >  }
> >
> > +static inline bool ublk_support_batch_io(const struct ublk_queue *ubq)
> > +{
> > +       return false;
> > +}
> > +
> >  static inline struct ublksrv_io_desc *
> >  ublk_get_iod(const struct ublk_queue *ubq, unsigned tag)
> >  {
> > @@ -1309,6 +1314,8 @@ static void ublk_dispatch_req(struct ublk_queue *ubq,
> >                         __func__, ubq->q_id, req->tag, io->flags,
> >                         ublk_get_iod(ubq, req->tag)->addr);
> >
> > +       WARN_ON_ONCE(ublk_support_batch_io(ubq));
> 
> Hmm, not a huge fan of extra checks in the I/O path. It seems fairly
> easy to verify from the code that these functions won't be called for
> batch commands. Do we really need the assertion?

It is just a safety guard, and can be removed, but ubq->flag is really in
hot cache.

> 
> > +
> >         /*
> >          * Task is exiting if either:
> >          *
> > @@ -1868,6 +1875,8 @@ static void ublk_uring_cmd_cancel_fn(struct io_uring_cmd *cmd,
> >         if (WARN_ON_ONCE(pdu->tag >= ubq->q_depth))
> >                 return;
> >
> > +       WARN_ON_ONCE(ublk_support_batch_io(ubq));
> > +
> >         task = io_uring_cmd_get_task(cmd);
> >         io = &ubq->ios[pdu->tag];
> >         if (WARN_ON_ONCE(task && task != io->task))
> > @@ -2233,7 +2242,10 @@ static int __ublk_fetch(struct io_uring_cmd *cmd, struct ublk_queue *ubq,
> >
> >         ublk_fill_io_cmd(io, cmd);
> >
> > -       WRITE_ONCE(io->task, get_task_struct(current));
> > +       if (ublk_support_batch_io(ubq))
> > +               WRITE_ONCE(io->task, NULL);
> 
> Don't see a need to explicitly write NULL here since the ublk_io
> memory is zero-initialized.

You are right, but ublk_fetch() is in slow path.


Thanks,
Ming