Re: [PATCH] ublk: optimize ublk_queue_cmd_list() for length-1 lists

[Ming Lei]

On Wed, Nov 04, 2026 at 07:13:49PM -0700, Caleb Sander Mateos wrote:
> For a request list of length 1, ublk_cmd_list_tw_cb() is more expensive
> than ublk_cmd_tw_cb(). It loads rq_next, which is in a separate cache
> line from the other struct request fields accessed in the dispatch path.
> And it dereferences mq_hctx->driver_data to get the struct ublk_queue *,
> whereas ublk_cmd_tw_cb() gets it from struct ublk_uring_cmd_pdu
> alongside the request pointer. On the submission, ublk_queue_cmd_list()
> can cheaply check whether the request list just assembled has length 1.
> So pass ublk_cmd_tw_cb as the io_uring task work callback if the list
> has a single request and ublk_cmd_list_tw_cb if it contains multiple.
> 
> Collapse the req/req_list union in ublk_uring_cmd_pdu into a single
> field so ublk_queue_cmd_list() can store the head request to it in
> either case.
> 
> Signed-off-by: Caleb Sander Mateos <csander@xxxxxxxxxxxxxxx>
> ---
>  drivers/block/ublk_drv.c | 17 ++++++++---------
>  1 file changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> index 3d322fa1bc83..c397e354a7d6 100644
> --- a/drivers/block/ublk_drv.c
> +++ b/drivers/block/ublk_drv.c
> @@ -117,21 +117,18 @@ struct ublk_batch_fetch_cmd {
>  	unsigned short buf_group;
>  };
>  
>  struct ublk_uring_cmd_pdu {
>  	/*
> -	 * Store requests in same batch temporarily for queuing them to
> -	 * daemon context.
> +	 * For ublk_cmd_list_tw_cb(), a list of batched requests queued to
> +	 * daemon context. For ublk_cmd_tw_cb(), a single request.
>  	 *
>  	 * It should have been stored to request payload, but we do want
>  	 * to avoid extra pre-allocation, and uring_cmd payload is always
>  	 * free for us
>  	 */
> -	union {
> -		struct request *req;
> -		struct request *req_list;
> -	};
> +	struct request *req;
>  
>  	/*
>  	 * The following two are valid in this cmd whole lifetime, and
>  	 * setup in ublk uring_cmd handler
>  	 */
> @@ -2104,11 +2101,11 @@ static void ublk_queue_cmd(struct ublk_queue *ubq, struct request *rq)
>  
>  static void ublk_cmd_list_tw_cb(struct io_tw_req tw_req, io_tw_token_t tw)
>  {
>  	struct io_uring_cmd *cmd = io_uring_cmd_from_tw(tw_req);
>  	struct ublk_uring_cmd_pdu *pdu = ublk_get_uring_cmd_pdu(cmd);
> -	struct request *rq = pdu->req_list;
> +	struct request *rq = pdu->req;
>  	struct request *next;
>  
>  	do {
>  		next = rq->rq_next;
>  		rq->rq_next = NULL;
> @@ -2119,14 +2116,16 @@ static void ublk_cmd_list_tw_cb(struct io_tw_req tw_req, io_tw_token_t tw)
>  
>  static void ublk_queue_cmd_list(struct ublk_io *io, struct rq_list *l)
>  {
>  	struct io_uring_cmd *cmd = io->cmd;
>  	struct ublk_uring_cmd_pdu *pdu = ublk_get_uring_cmd_pdu(cmd);
> +	io_req_tw_func_t tw_cb;
>  
> -	pdu->req_list = rq_list_peek(l);
> +	tw_cb = l->head == l->tail ? ublk_cmd_tw_cb : ublk_cmd_list_tw_cb;
> +	pdu->req = rq_list_peek(l);

Both callers guarantee the list is non-empty, so this is fine. It
would be a bit nicer as a rq_list_is_singular() helper in blk-mq.h
rather than open-coding rq_list internals in the driver, but not a
blocker, anyway:

Reviewed-by: Ming Lei <tom.leiming@xxxxxxxxx>


Thanks,
Ming