[PATCH 7/7] nvme: allow send fused commands

[Caleb Sander]

There is prior discussion about exactly this change at
https://github.com/linux-nvme/nvme-cli/issues/318. The concerns raised
there still appear valid 6 years later. The main issue is that this
interface requires 2 separate ioctls/io_uring operations to submit a
Fused Compare and Write. That means the 2 commands could be submitted
to different I/O queues, which wouldn't work. Not to mention that 2
threads are required in order to submit the commands concurrently if
the blocking ioctls are used. Additionally, another application using
the NVMe controller concurrently could submit a command in between the
fused commands, which would also break the Fused Compare and Write.

FYI there was a set of patches sent to the mailing list
https://lore.kernel.org/linux-nvme/20210105224939.1336-1-clay.mayers@kioxia.com/T/#u
that attempted to implement a new passthru ioctl for submitting fused
commands. But it adds memory overhead to each NVMe request and there
are a lot of corner cases to consider around error handling.

As an implementer of Fused Compare and Write on the target side, I can
say the concept of "fused commands" is pretty horrible to work with.
It breaks the model where each NVMe command is an independent
operation and requires a ton of additional error propagation logic.
Not to mention that the NSID, SLBA, and NLB parameters are now
duplicated between the 2 commands... As far as I can tell, the only
reason to make the operation 2 commands is to allow a separate PRP/SGL
for each command. I guess this avoids the memcpy that would be
required if a single concatenated data buffer was used? But at what
cost???

On Wed, Sep 11, 2024 at 11:45 PM Dmitry Bogdanov <d.bogdanov at yadro.com> wrote:
>
> Allow a user to pass fused flags for the commands.
>
> Signed-off-by: Dmitry Bogdanov <d.bogdanov at yadro.com>
> ---
>  drivers/nvme/host/ioctl.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/nvme/host/ioctl.c b/drivers/nvme/host/ioctl.c
> index 1d769c842fbf..affedac90f04 100644
> --- a/drivers/nvme/host/ioctl.c
> +++ b/drivers/nvme/host/ioctl.c
> @@ -220,7 +220,7 @@ static int nvme_submit_io(struct nvme_ns *ns, struct nvme_user_io __user *uio)
>
>         if (copy_from_user(&io, uio, sizeof(io)))
>                 return -EFAULT;
> -       if (io.flags)
> +       if (io.flags & 0xFC)
>                 return -EINVAL;
>
>         switch (io.opcode) {
> @@ -299,7 +299,7 @@ static int nvme_user_cmd(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
>
>         if (copy_from_user(&cmd, ucmd, sizeof(cmd)))
>                 return -EFAULT;
> -       if (cmd.flags)
> +       if (cmd.flags & 0xFC)
>                 return -EINVAL;
>         if (!nvme_validate_passthru_nsid(ctrl, ns, cmd.nsid))
>                 return -EINVAL;
> @@ -346,7 +346,7 @@ static int nvme_user_cmd64(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
>
>         if (copy_from_user(&cmd, ucmd, sizeof(cmd)))
>                 return -EFAULT;
> -       if (cmd.flags)
> +       if (cmd.flags & 0xFC)
>                 return -EINVAL;
>         if (!nvme_validate_passthru_nsid(ctrl, ns, cmd.nsid))
>                 return -EINVAL;
> --
> 2.25.1
>
>