public inbox for [email protected]
 help / color / mirror / Atom feed
From: Kanchan Joshi <[email protected]>
To: Christoph Hellwig <[email protected]>
Cc: "Kanchan Joshi" <[email protected]>,
	"Jens Axboe" <[email protected]>, "Keith Busch" <[email protected]>,
	"Pavel Begunkov" <[email protected]>,
	[email protected], [email protected],
	[email protected], [email protected],
	[email protected], "Pankaj Raghav" <[email protected]>,
	"Javier González" <[email protected]>,
	"Luis Chamberlain" <[email protected]>,
	"Adam Manzanares" <[email protected]>,
	"Anuj Gupta" <[email protected]>
Subject: Re: [PATCH 17/17] nvme: enable non-inline passthru commands
Date: Thu, 10 Mar 2022 17:20:13 +0530	[thread overview]
Message-ID: <CA+1E3rLaQstG8LWUyJrbK5Qz+AnNpOnAyoK-7H5foFm67BJeFA@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>

On Thu, Mar 10, 2022 at 2:06 PM Christoph Hellwig <[email protected]> wrote:
>
> On Tue, Mar 08, 2022 at 08:51:05PM +0530, Kanchan Joshi wrote:
> > From: Anuj Gupta <[email protected]>
> >
> > On submission,just fetch the commmand from userspace pointer and reuse
> > everything else. On completion, update the result field inside the
> > passthru command.
>
> What is that supposed to mean?  What is the reason to do it.  Remember
> to always document the why in commit logs.

I covered some of it in patch 6, but yes I need to expand the
reasoning here. Felt that retrospectively too.
So there are two ways/modes of submitting commands:

Mode 1: inline into sqe. This is the default way when passthru command
is placed inside a big sqe which has 80 bytes of space.
The only problem is - passthru command has this 'result' field
(structure below for quick reference) which is statically embedded and
not a pointer (like addr and metadata field).

struct nvme_passthru_cmd64 {
__u8 opcode;
__u8 flags;
__u16 rsvd1;
__u32 nsid;
__u32 cdw2;
__u32 cdw3;
__u64 metadata;
__u64 addr;
__u32 metadata_len;
__u32 data_len;
__u32 cdw10;
__u32 cdw11;
__u32 cdw12;
__u32 cdw13;
__u32 cdw14;
__u32 cdw15;
__u32 timeout_ms;
__u32   rsvd2;
__u64 result;
};
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.

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).

Mode 2: Non-inline/indirect (pointer of command into sqe) submission.
User-space places a pointer of passthru command, and a flag in
io_uring saying that this is not inline.
For this, in nvme (this patch) we always update the 'result' on
completion and therefore can support all passthru commands.

Hope this makes the reasoning clear?
Plumbing wise, non-inline support does not create churn (almost all
the infra of inline-command handling is reused). Extra is
copy_from_user , and put_user.

> > +static inline bool is_inline_rw(struct io_uring_cmd *ioucmd, struct nvme_command *cmd)
>
> Overly long line.

Under 100, but sure, can fold it under 80.


-- 
Kanchan

  reply	other threads:[~2022-03-10 11:50 UTC|newest]

Thread overview: 122+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20220308152651epcas5p1ebd2dc7fa01db43dd587c228a3695696@epcas5p1.samsung.com>
2022-03-08 15:20 ` [PATCH 00/17] io_uring passthru over nvme Kanchan Joshi
     [not found]   ` <CGME20220308152653epcas5p10c31f58cf6bff125cc0baa176b4d4fac@epcas5p1.samsung.com>
2022-03-08 15:20     ` [PATCH 01/17] io_uring: add support for 128-byte SQEs Kanchan Joshi
     [not found]   ` <CGME20220308152655epcas5p4ae47d715e1c15069e97152dcd283fd40@epcas5p4.samsung.com>
2022-03-08 15:20     ` [PATCH 02/17] fs: add file_operations->async_cmd() Kanchan Joshi
     [not found]   ` <CGME20220308152658epcas5p3929bd1fcf75edc505fec71901158d1b5@epcas5p3.samsung.com>
2022-03-08 15:20     ` [PATCH 03/17] io_uring: add infra and support for IORING_OP_URING_CMD Kanchan Joshi
2022-03-11  1:51       ` Luis Chamberlain
2022-03-11  2:43         ` Jens Axboe
2022-03-11 17:11           ` Luis Chamberlain
2022-03-11 18:47             ` Paul Moore
2022-03-11 20:57               ` Luis Chamberlain
2022-03-11 21:03                 ` Paul Moore
2022-03-14 16:25             ` Casey Schaufler
2022-03-14 16:32               ` Luis Chamberlain
2022-03-14 18:05                 ` Casey Schaufler
2022-03-14 19:40                   ` Luis Chamberlain
     [not found]   ` <CGME20220308152700epcas5p4130d20119a3a250a2515217d6552f668@epcas5p4.samsung.com>
2022-03-08 15:20     ` [PATCH 04/17] nvme: modify nvme_alloc_request to take an additional parameter Kanchan Joshi
2022-03-11  6:38       ` Christoph Hellwig
     [not found]   ` <CGME20220308152702epcas5p1eb1880e024ac8b9531c85a82f31a4e78@epcas5p1.samsung.com>
2022-03-08 15:20     ` [PATCH 05/17] nvme: wire-up support for async-passthru on char-device Kanchan Joshi
2022-03-10  0:02       ` Clay Mayers
2022-03-10  8:32         ` Kanchan Joshi
2022-03-11  7:01       ` Christoph Hellwig
2022-03-14 16:23         ` Kanchan Joshi
2022-03-15  8:54           ` Christoph Hellwig
2022-03-16  7:27             ` Kanchan Joshi
2022-03-24  6:22               ` Christoph Hellwig
2022-03-24 17:45                 ` Kanchan Joshi
2022-03-11 17:56       ` Luis Chamberlain
2022-03-11 18:53         ` Paul Moore
2022-03-11 21:02           ` Luis Chamberlain
2022-03-13 21:53       ` Sagi Grimberg
2022-03-14 17:54         ` Kanchan Joshi
2022-03-15  9:02           ` Sagi Grimberg
2022-03-16  9:21             ` Kanchan Joshi
2022-03-16 10:56               ` Sagi Grimberg
2022-03-16 11:51                 ` Kanchan Joshi
2022-03-16 13:52                   ` Sagi Grimberg
2022-03-16 14:35                     ` Jens Axboe
2022-03-16 14:50                       ` Sagi Grimberg
2022-03-24  6:20                         ` Christoph Hellwig
2022-03-24 10:42                           ` Sagi Grimberg
2022-03-22 15:18       ` Clay Mayers
2022-03-22 16:57         ` Kanchan Joshi
     [not found]   ` <CGME20220308152704epcas5p16610e1f50672b25fa1df5f7c5c261bb5@epcas5p1.samsung.com>
2022-03-08 15:20     ` [PATCH 06/17] io_uring: prep for fixed-buffer enabled uring-cmd Kanchan Joshi
     [not found]   ` <CGME20220308152707epcas5p430127761a7fd4bf90c2501eabe9ee96e@epcas5p4.samsung.com>
2022-03-08 15:20     ` [PATCH 07/17] io_uring: add support for uring_cmd with fixed-buffer Kanchan Joshi
     [not found]   ` <CGME20220308152709epcas5p1f9d274a0214dc462c22c278a72d8697c@epcas5p1.samsung.com>
2022-03-08 15:20     ` [PATCH 08/17] nvme: enable passthrough " Kanchan Joshi
2022-03-10  8:32       ` Christoph Hellwig
2022-03-11  6:43       ` Christoph Hellwig
2022-03-14 13:06         ` Kanchan Joshi
2022-03-15  8:55           ` Christoph Hellwig
2022-03-14 12:18       ` Ming Lei
2022-03-14 13:09         ` Kanchan Joshi
     [not found]   ` <CGME20220308152711epcas5p31de5d63f5de91fae94e61e5c857c0f13@epcas5p3.samsung.com>
2022-03-08 15:20     ` [PATCH 09/17] io_uring: plug for async bypass Kanchan Joshi
2022-03-10  8:33       ` Christoph Hellwig
2022-03-14 14:33         ` Ming Lei
2022-03-15  8:56           ` Christoph Hellwig
2022-03-11 17:15       ` Luis Chamberlain
     [not found]   ` <CGME20220308152714epcas5p4c5a0d16512fd7054c9a713ee28ede492@epcas5p4.samsung.com>
2022-03-08 15:20     ` [PATCH 10/17] block: wire-up support for plugging Kanchan Joshi
2022-03-10  8:34       ` Christoph Hellwig
2022-03-10 12:40         ` Kanchan Joshi
2022-03-14 14:40           ` Ming Lei
2022-03-21  7:02             ` Kanchan Joshi
2022-03-23  1:27               ` Ming Lei
2022-03-23  1:41                 ` Jens Axboe
2022-03-23  1:58                   ` Jens Axboe
2022-03-23  2:10                     ` Ming Lei
2022-03-23  2:17                       ` Jens Axboe
     [not found]   ` <CGME20220308152716epcas5p3d38d2372c184259f1a10c969f7e4396f@epcas5p3.samsung.com>
2022-03-08 15:20     ` [PATCH 11/17] block: factor out helper for bio allocation from cache Kanchan Joshi
2022-03-10  8:35       ` Christoph Hellwig
2022-03-10 12:25         ` Kanchan Joshi
2022-03-24  6:30           ` Christoph Hellwig
2022-03-24 17:45             ` Kanchan Joshi
2022-03-25  5:38               ` Christoph Hellwig
     [not found]   ` <CGME20220308152718epcas5p3afd2c8a628f4e9733572cbb39270989d@epcas5p3.samsung.com>
2022-03-08 15:21     ` [PATCH 12/17] nvme: enable bio-cache for fixed-buffer passthru Kanchan Joshi
2022-03-11  6:48       ` Christoph Hellwig
2022-03-14 18:18         ` Kanchan Joshi
2022-03-15  8:57           ` Christoph Hellwig
     [not found]   ` <CGME20220308152720epcas5p19653942458e160714444942ddb8b8579@epcas5p1.samsung.com>
2022-03-08 15:21     ` [PATCH 13/17] nvme: allow user passthrough commands to poll Kanchan Joshi
2022-03-08 17:08       ` Keith Busch
2022-03-09  7:03         ` Kanchan Joshi
2022-03-11  6:49           ` Christoph Hellwig
     [not found]   ` <CGME20220308152723epcas5p34460b4af720e515317f88dbb78295f06@epcas5p3.samsung.com>
2022-03-08 15:21     ` [PATCH 14/17] io_uring: add polling support for uring-cmd Kanchan Joshi
2022-03-11  6:50       ` Christoph Hellwig
2022-03-14 10:16         ` Kanchan Joshi
2022-03-15  8:57           ` Christoph Hellwig
2022-03-16  5:09             ` Kanchan Joshi
2022-03-24  6:30               ` Christoph Hellwig
     [not found]   ` <CGME20220308152725epcas5p36d1ce3269a47c1c22cc0d66bdc2b9eb3@epcas5p3.samsung.com>
2022-03-08 15:21     ` [PATCH 15/17] nvme: wire-up polling for uring-passthru Kanchan Joshi
     [not found]   ` <CGME20220308152727epcas5p20e605718dd99e97c94f9232d40d04d95@epcas5p2.samsung.com>
2022-03-08 15:21     ` [PATCH 16/17] io_uring: add support for non-inline uring-cmd Kanchan Joshi
     [not found]   ` <CGME20220308152729epcas5p17e82d59c68076eb46b5ef658619d65e3@epcas5p1.samsung.com>
2022-03-08 15:21     ` [PATCH 17/17] nvme: enable non-inline passthru commands Kanchan Joshi
2022-03-10  8:36       ` Christoph Hellwig
2022-03-10 11:50         ` Kanchan Joshi [this message]
2022-03-10 14:19           ` Christoph Hellwig
2022-03-10 18:43             ` Kanchan Joshi
2022-03-11  6:27               ` Christoph Hellwig
2022-03-22 17:10                 ` Kanchan Joshi
2022-03-24  6:32                   ` Christoph Hellwig
2022-03-25 13:39                     ` Kanchan Joshi
2022-03-28  4:44                       ` Kanchan Joshi
2022-03-30 12:59                         ` Christoph Hellwig
2022-03-30 13:02                       ` Christoph Hellwig
2022-03-30 13:14                         ` Kanchan Joshi
2022-04-01  1:25                           ` Jens Axboe
2022-04-01  2:33                             ` Kanchan Joshi
2022-04-01  2:44                               ` Jens Axboe
2022-04-01  3:05                                 ` Jens Axboe
2022-04-01  6:32                                 ` Kanchan Joshi
2022-04-19 17:31                                 ` Kanchan Joshi
2022-04-19 18:19                                   ` Jens Axboe
     [not found]                                     ` <CGME20220420152003epcas5p3991e6941773690bcb425fd9d817105c3@epcas5p3.samsung.com>
2022-04-20 15:14                                       ` Kanchan Joshi
2022-04-20 15:28                                         ` Kanchan Joshi
2022-04-01  1:23                         ` Jens Axboe
2022-04-01  1:22                   ` Jens Axboe
2022-04-01  6:29                     ` Kanchan Joshi
2022-03-24 21:09       ` Clay Mayers
2022-03-24 23:36         ` Jens Axboe
2022-03-10  8:29   ` [PATCH 00/17] io_uring passthru over nvme Christoph Hellwig
2022-03-10 10:05     ` Kanchan Joshi
2022-03-11 16:43       ` Luis Chamberlain
2022-03-11 23:35         ` Adam Manzanares
2022-03-12  2:27           ` Adam Manzanares
2022-03-13  5:07             ` Kanchan Joshi
2022-03-14 20:30               ` Adam Manzanares
2022-03-13  5:10         ` Kanchan Joshi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CA+1E3rLaQstG8LWUyJrbK5Qz+AnNpOnAyoK-7H5foFm67BJeFA@mail.gmail.com \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox