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

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

[Caleb Sander Mateos]

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@purestorage.com>
---
 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);
 	rq_list_init(l);
-	io_uring_cmd_complete_in_task(cmd, ublk_cmd_list_tw_cb);
+	io_uring_cmd_complete_in_task(cmd, tw_cb);
 }
 
 static enum blk_eh_timer_return ublk_timeout(struct request *rq)
 {
 	struct ublk_queue *ubq = rq->mq_hctx->driver_data;

base-commit: e6ac1da6a53c5f670bf09e60a2500d15be68d2fc
-- 
2.56.0

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@purestorage.com>
> ---
>  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@gmail.com>


Thanks,
Ming