Re: [PATCH] ublk: remove io_cmds list in ublk_queue

[Jens Axboe <axboe@kernel.dk>]

On 3/18/25 12:14 PM, Uday Shankar wrote:
> The current I/O dispatch mechanism - queueing I/O by adding it to the
> io_cmds list (and poking task_work as needed), then dispatching it in
> ublk server task context by reversing io_cmds and completing the
> io_uring command associated to each one - was introduced by commit
> 7d4a93176e014 ("ublk_drv: don't forward io commands in reserve order")
> to ensure that the ublk server received I/O in the same order that the
> block layer submitted it to ublk_drv. This mechanism was only needed for
> the "raw" task_work submission mechanism, since the io_uring task work
> wrapper maintains FIFO ordering (using quite a similar mechanism in
> fact). The "raw" task_work submission mechanism is no longer supported
> in ublk_drv as of commit 29dc5d06613f2 ("ublk: kill queuing request by
> task_work_add"), so the explicit llist/reversal is no longer needed - it
> just duplicates logic already present in the underlying io_uring APIs.
> Remove it.

Patch looks good to me, just one followup:

> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> index 2955900ee713c5d8f3cbc2a69f6f6058348e5253..82c9d3d22f0ea5a0fad3f33837fa16146b5af7a9 100644
> --- a/drivers/block/ublk_drv.c
> +++ b/drivers/block/ublk_drv.c
> @@ -77,8 +77,6 @@
>  	 UBLK_PARAM_TYPE_DMA_ALIGN)
>  
>  struct ublk_rq_data {
> -	struct llist_node node;
> -
>  	struct kref ref;
>  };

Can we get rid of ublk_rq_data then? If it's just a ref thing, I'm sure
we can find an atomic_t of space in struct request and avoid it. Not a
pressing thing, just tossing it out there...

-- 
Jens Axboe