On Mon, Apr 25, 2022 at 11:08:03PM +0530, Kanchan Joshi wrote:
>On Sat, Apr 23, 2022 at 07:53:09PM +0200, Christoph Hellwig wrote:
>>On Wed, Apr 06, 2022 at 10:50:14AM +0530, Kanchan Joshi wrote:
>>>> In that case we will base the newer version on its top.
>>>But if it saves some cycles for you, and also the travel from nvme to
>>>linux-block tree - I can carry that refactoring as a prep patch in
>>>this series. Your call.
>>
>>FYI, this is what I have so far:
>>
>>http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/nvme-passthrough-refactor
>>
>>the idea would be to use these lower level helpers for uring, and
>>not really share the higher level function at all.  This does create
>>a little extra code, but I think it'll be more modular and better
>>maintainable.  Feel free to pull this in if it helps you, otherwise
>>I'll try to find some time to do more than just light testing and
>>will post it.
>
>Thanks for sharing.
>So I had picked your previous version, and this one streamlines meta
>handling further. But the problem is bip gets freed before we reach to
>this point -
>
>+static int nvme_free_user_metadata(struct bio *bio, void __user *ubuf, int ret)
>+{
>+       struct bio_integrity_payload *bip = bio_integrity(bio);
>+       void *buf = bvec_virt(bip->bip_vec);
>+
>+       if (!ret && bio_op(bio) == REQ_OP_DRV_IN &&
>+           copy_to_user(ubuf, buf, bip->bip_vec->bv_len))
>
>Without bip, we cannot kill current meta/meta_len fields.

And by this I mean we cannot keep io_uring_cmd this way -

+struct io_uring_cmd {
+       struct file     *file;
+       void            *cmd;
+       /* for irq-completion - if driver requires doing stuff in task-context*/
+       void (*driver_cb)(struct io_uring_cmd *cmd);
+       u32             flags;
+       u32             cmd_op;
+
+       void            *private;
+
+       /*
+        * Out of band data can be used for data that is not the main data.
+        * E.g. block device PI/metadata or additional information.
+        */
+       void __user     *oob_user;
+};

Rather we need to backtrack to pdu[28], since nvme would need all that
space.

+struct io_uring_cmd {
+       struct file     *file;
+       void            *cmd;
+       /* for irq-completion - if driver requires doing stuff in task-context*/
+       void (*driver_cb)(struct io_uring_cmd *cmd);
+       u32             flags;
+       u32             cmd_op;
+       u32             unused;
+       u8              pdu[28]; /* available inline for free use */
+};

+struct nvme_uring_cmd_pdu {
+       union {
+               struct bio *bio;
+               struct request *req;
+       };
+       void *meta; /* kernel-resident buffer */
+       void __user *meta_buffer;
+       u32 meta_len;
+} __packed;