On Sun, Jun 22, 2025 at 6:19 PM Ming Lei <ming.lei@xxxxxxxxxx> wrote:
>
> ublk_queue_cmd_list() dispatches the whole batch list by scheduling task
> work via the tail request's io_uring_cmd, this way is fine even though
> more than one io_ring_ctx are involved for this batch since it is just
> one running context.
>
> However, the task work handler ublk_cmd_list_tw_cb() takes `issue_flags`
> of tail uring_cmd's io_ring_ctx for completing all commands. This way is
> wrong if any uring_cmd is issued from different io_ring_ctx.
>
> Fixes it by always building per-io-ring-ctx batch list.
>
> For typical per-queue or per-io daemon implementation, this way shouldn't
> make difference from performance viewpoint, because single io_ring_ctx is
> often taken in each daemon.
>
> Fixes: d796cea7b9f3 ("ublk: implement ->queue_rqs()")
> Fixes: ab03a61c6614 ("ublk: have a per-io daemon instead of a per-queue daemon")
> Signed-off-by: Ming Lei <ming.lei@xxxxxxxxxx>
> ---
> drivers/block/ublk_drv.c | 17 +++++++++--------
> 1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> index c637ea010d34..e79b04e61047 100644
> --- a/drivers/block/ublk_drv.c
> +++ b/drivers/block/ublk_drv.c
> @@ -1336,9 +1336,8 @@ static void ublk_cmd_list_tw_cb(struct io_uring_cmd *cmd,
> } while (rq);
> }
>
> -static void ublk_queue_cmd_list(struct ublk_io *io, struct rq_list *l)
> +static void ublk_queue_cmd_list(struct io_uring_cmd *cmd, struct rq_list *l)
> {
> - struct io_uring_cmd *cmd = io->cmd;
> struct ublk_uring_cmd_pdu *pdu = ublk_get_uring_cmd_pdu(cmd);
>
> pdu->req_list = rq_list_peek(l);
> @@ -1420,16 +1419,18 @@ static void ublk_queue_rqs(struct rq_list *rqlist)
> {
> struct rq_list requeue_list = { };
> struct rq_list submit_list = { };
> - struct ublk_io *io = NULL;
> + struct io_uring_cmd *cmd = NULL;
> struct request *req;
>
> while ((req = rq_list_pop(rqlist))) {
> struct ublk_queue *this_q = req->mq_hctx->driver_data;
> - struct ublk_io *this_io = &this_q->ios[req->tag];
> + struct io_uring_cmd *this_cmd = this_q->ios[req->tag].cmd;
>
> - if (io && io->task != this_io->task && !rq_list_empty(&submit_list))
> - ublk_queue_cmd_list(io, &submit_list);
> - io = this_io;
> + if (cmd && io_uring_cmd_ctx_handle(cmd) !=
> + io_uring_cmd_ctx_handle(this_cmd) &&
> + !rq_list_empty(&submit_list))
> + ublk_queue_cmd_list(cmd, &submit_list);
I don't think we can assume that ublk commands submitted to the same
io_uring have the same daemon task. It's possible for multiple tasks
to submit to the same io_uring, even though that's not a common or
performant way to use io_uring. Probably we need to check that both
the task and io_ring_ctx match.
Best,
Caleb
> + cmd = this_cmd;
>
> if (ublk_prep_req(this_q, req, true) == BLK_STS_OK)
> rq_list_add_tail(&submit_list, req);
> @@ -1438,7 +1439,7 @@ static void ublk_queue_rqs(struct rq_list *rqlist)
> }
>
> if (!rq_list_empty(&submit_list))
> - ublk_queue_cmd_list(io, &submit_list);
> + ublk_queue_cmd_list(cmd, &submit_list);
> *rqlist = requeue_list;
> }
>
> --
> 2.47.0
>
