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@xxxxxxxxxx> 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@xxxxxxxxxx>
> > ---
> > 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
