On Mon, Apr 4, 2022 at 12:50 PM Christoph Hellwig <hch@xxxxxx> 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).
