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

[Kanchan Joshi <joshiiitr@gmail.com>]

On Mon, Apr 4, 2022 at 12:50 PM Christoph Hellwig <hch@lst.de> wrote:
>
> On Fri, Apr 01, 2022 at 04:33:10PM +0530, Kanchan Joshi wrote:
> > Introduce handler for fops->async_cmd(), implementing async passthru
> > on char device (/dev/ngX). The handler supports NVME_IOCTL_IO64_CMD.
>
> I really don't like how this still mixes up ioctls and uring cmds,
> as mentioned close to half a dozend times we really should not mix
> them up.

Sorry, I too had the plans to use a different opcode eventually (i.e.
for the full series). Just wanted to focus on big-cqe this time.

> Something like this (untested) patch should help to separate
> the much better:

It does, thanks. But the only thing is - it would be good to support
vectored-passthru too (i.e. NVME_IOCTL_IO64_CMD_VEC) for this path.
For the new opcode "NVME_URING_CMD_IO" , either we can change the
cmd-structure or flag-based handling so that vectored-io is supported.
Or we introduce NVME_URING_CMD_IO_VEC also for that.
Which one do you prefer?

> +static int nvme_ioctl_finish_metadata(struct bio *bio, int ret,
> +               void __user *meta_ubuf)
> +{
> +       struct bio_integrity_payload *bip = bio_integrity(bio);
> +
> +       if (bip) {
> +               void *meta = bvec_virt(bip->bip_vec);
> +
> +               if (!ret && bio_op(bio) == REQ_OP_DRV_IN &&
> +                   copy_to_user(meta_ubuf, meta, bip->bip_vec->bv_len))
> +                       ret = -EFAULT;

Maybe it is better to move the check "bio_op(bio) != REQ_OP_DRV_IN" outside.
Because this can be common, and for that we can avoid entering into
the function call itself (i.e. nvme_ioctl_finish_metadata).