On Fri, May 9, 2025 at 8:06 AM Ming Lei <ming.lei@xxxxxxxxxx> wrote: > > `ublk_queue` is read only for io buffer register/unregister command. Both > `ublk_io` and block layer request are read-only for IO buffer register/ > unregister command. > > So the two command can be issued from other task contexts. I mentioned this before in https://lore.kernel.org/linux-block/CADUfDZqZ_9O7vUAYtxrrujWqPBuP05nBhCbzNuNsc9kJTmX2sA@xxxxxxxxxxxxxx/ But UBLK_IO_(UN)REGISTER_IO_BUF still reads io->flags. So it would be a race condition to handle it on a thread other than ubq_daemon, as ubq_daemon may concurrently modify io->flags. If you do want to support UBLK_IO_(UN)REGISTER_IO_BUF on other threads, the writes to io->flags should use WRITE_ONCE() and the reads on other threads should use READ_ONCE(). With those modifications, it should be safe because __ublk_check_and_get_req() atomically checks the state of the request and increments its reference count. Best, Caleb > > Not same with other three ublk commands, these two are for handling target > IO only, we shouldn't limit their issue task context, otherwise it becomes > hard for ublk server(backend) to use zero copy feature. > > Reported-by: Uday Shankar <ushankar@xxxxxxxxxxxxxxx> > Closes: https://lore.kernel.org/linux-block/20250410-ublk_task_per_io-v3-2-b811e8f4554a@xxxxxxxxxxxxxxx/ > Signed-off-by: Ming Lei <ming.lei@xxxxxxxxxx> > --- > drivers/block/ublk_drv.c | 17 +++++++++++++++-- > 1 file changed, 15 insertions(+), 2 deletions(-) > > diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c > index cb612151e9a1..31f06e734250 100644 > --- a/drivers/block/ublk_drv.c > +++ b/drivers/block/ublk_drv.c > @@ -2057,6 +2057,12 @@ static bool ublk_get_data(const struct ublk_queue *ubq, struct ublk_io *io) > return ublk_start_io(ubq, req, io); > } > > +static bool is_io_buf_reg_unreg_cmd(unsigned int cmd_op) > +{ > + return _IOC_NR(cmd_op) == UBLK_IO_REGISTER_IO_BUF || > + _IOC_NR(cmd_op) == UBLK_IO_UNREGISTER_IO_BUF; > +} > + > static int __ublk_ch_uring_cmd(struct io_uring_cmd *cmd, > unsigned int issue_flags, > const struct ublksrv_io_cmd *ub_cmd) > @@ -2076,8 +2082,15 @@ static int __ublk_ch_uring_cmd(struct io_uring_cmd *cmd, > goto out; > > ubq = ublk_get_queue(ub, ub_cmd->q_id); > - if (ubq->ubq_daemon && ubq->ubq_daemon != current) > - goto out; > + /* > + * Both `ublk_io` and block layer request are read-only for IO > + * buffer register/unregister command, so the two are allowed to be > + * issued from other task contexts > + */ > + if (!is_io_buf_reg_unreg_cmd(cmd_op)) { > + if (ubq->ubq_daemon && ubq->ubq_daemon != current) > + goto out; > + } > > if (tag >= ubq->q_depth) > goto out; > -- > 2.47.0 >
