Re: [RFC PATCH 2/6] nvme: wire-up support for async-passthru on char-device.

[Christoph Hellwig <hch@lst.de>]

On Tue, Sep 07, 2021 at 09:50:27PM +0530, Kanchan Joshi wrote:
> > A few other notes:
> >
> >  - I suspect the ioctl_cmd really should move into the core using_cmd
> >    infrastructure
> 
> Yes, that seems possible by creating that field outside by combining
> "op" and "unused" below.
> +struct io_uring_cmd {
> + struct file *file;
> + __u16 op;
> + __u16 unused;
> + __u32 len;
> + __u64 pdu[5]; /* 40 bytes available inline for free use */
> +};

Two different issues here:

 - the idea of having a two layer indirection with op and a cmd doesn't
   really make much sense
 - if we want to avoid conflicts using 32-bit probably makes sense

So I'd turn op and unused into a single cmd field, use the ioctl encoding
macros for it (but preferably pick different numbers than the existing
ioctls).

> >  - that whole mix of user space interface and internal data in the
> >    ->pdu field is a mess.  What is the problem with deferring the
> >    request freeing into the user context, which would clean up
> >    quite a bit of that, especially if io_uring_cmd grows a private
> >    field.
> 
> That mix isn't great but the attempt was to save the allocation.
> And I was not very sure if it'd be fine to defer freeing the request
> until task-work fires up.

What would be the problem with the delaying?

> Even if we take that route, we would still need a place to store bio
> pointer (hopefully meta pointer can be extracted out of bio).
> Do you see it differently?

We don't need the bio pointer at all.  The old passthrough code needed
it when we still used block layer bonuce buffering for it.  But that
bounce buffering for passthrough commands got removed a while ago,
and even before nvme never used it.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme