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

[Ming Lei <ming.lei@redhat.com>]

On Tue, Mar 18, 2025 at 12:48:44PM -0600, Jens Axboe wrote:
> On 3/18/25 12:43 PM, Uday Shankar wrote:
> > On Tue, Mar 18, 2025 at 12:22:57PM -0600, Jens Axboe wrote:
> >>>  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...
> > 
> > Yeah probably - we do need a ref since one could complete a request
> > concurrently with another code path which references it (user copy and
> > zero copy). I see that struct request has a refcount in it already,
> 
> Right, at least with the current usage, we still do need that kref, or
> something similar. I would've probably made it just use refcount_t
> though, rather than rely on the indirect calls. kref doesn't really
> bring us anything here in terms of API.
> 
> > though I don't see any examples of drivers using it. Would it be a bad
> > idea to try and reuse that?
> 
> We can't reuse that one, and it's not for driver use - purely internal.
> But I _think_ you could easily grab space in the union that has the hash
> and ipi_list for it. And then you could dump needing this extra data per
> request.

It should be fine to reuse request->ref, since the payload shares same
lifetime with request.

But if it is exported, the interface is likely to be misused...


thanks,
Ming