Re: [PATCH 17/17] nvme: enable non-inline passthru commands

Christoph Hellwig
2022-03-10T14:19:59Z

On Thu, Mar 10, 2022 at 05:20:13PM +0530, Kanchan Joshi wrote:
> In sync ioctl, we always update this result field by doing put_user on
> completion.
> For async ioctl, since command is inside the the sqe, its lifetime is
> only upto submission. SQE may get reused post submission, leaving no
> way to update the "result" field on completion. Had this field been a
> pointer, we could have saved this on submission and updated on
> completion. But that would require redesigning this structure and
> adding newer ioctl in nvme.

Why would it required adding an ioctl to nvme? The whole io_uring
async_cmd infrastructure is completely independent from ioctls.

> Coming back, even though sync-ioctl alway updates this result to
> user-space, only a few nvme io commands (e.g. zone-append, copy,
> zone-mgmt-send) can return this additional result (spec-wise).
> Therefore in nvme, when we are dealing with inline-sqe commands from
> io_uring, we never attempt to update the result. And since we don't
> update the result, we limit support to only read/write passthru
> commands. And fail any other command during submission itself (Patch
> 2).

Yikes. That is outright horrible. passthrough needs to be command
agnostic and future proof to any newly added nvme command.

> > Overly long line.
>
> Under 100, but sure, can fold it under 80.

You can only use 100 sparingly if it makes the code more readable. Which
I know is fuzzy, and in practice never does. Certainly not in nvme and
block code.