All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@lst.de>
To: Kanchan Joshi <joshiiitr@gmail.com>
Cc: "Christoph Hellwig" <hch@lst.de>,
	"Kanchan Joshi" <joshi.k@samsung.com>,
	"Jens Axboe" <axboe@kernel.dk>, "Keith Busch" <kbusch@kernel.org>,
	"Pavel Begunkov" <asml.silence@gmail.com>,
	io-uring@vger.kernel.org, linux-nvme@lists.infradead.org,
	linux-block@vger.kernel.org, sbates@raithlin.com,
	logang@deltatee.com, "Pankaj Raghav" <pankydev8@gmail.com>,
	"Javier González" <javier@javigon.com>,
	"Luis Chamberlain" <mcgrof@kernel.org>,
	"Adam Manzanares" <a.manzanares@samsung.com>,
	"Anuj Gupta" <anuj20.g@samsung.com>
Subject: Re: [PATCH 17/17] nvme: enable non-inline passthru commands
Date: Thu, 10 Mar 2022 15:19:45 +0100	[thread overview]
Message-ID: <20220310141945.GA890@lst.de> (raw)
In-Reply-To: <CA+1E3rLaQstG8LWUyJrbK5Qz+AnNpOnAyoK-7H5foFm67BJeFA@mail.gmail.com>

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.

  reply	other threads:[~2022-03-10 14:20 UTC|newest]

Thread overview: 125+ 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
2022-03-08 15:20   ` [PATCH 01/17] io_uring: add support for 128-byte SQEs Kanchan Joshi
2022-03-08 15:20   ` [PATCH 02/17] fs: add file_operations->async_cmd() Kanchan Joshi
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 18:47             ` Paul Moore
2022-03-11 20:57             ` Luis Chamberlain
2022-03-11 20:57               ` Luis Chamberlain
2022-03-11 21:03               ` Paul Moore
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
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
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
2022-03-08 15:20   ` [PATCH 06/17] io_uring: prep for fixed-buffer enabled uring-cmd Kanchan Joshi
2022-03-08 15:20   ` [PATCH 07/17] io_uring: add support for uring_cmd with fixed-buffer Kanchan Joshi
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
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
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
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
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
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
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
2022-03-08 15:21   ` [PATCH 15/17] nvme: wire-up polling for uring-passthru Kanchan Joshi
2022-03-08 15:21   ` [PATCH 16/17] io_uring: add support for non-inline uring-cmd Kanchan Joshi
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
2022-03-10 14:19         ` Christoph Hellwig [this message]
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
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=20220310141945.GA890@lst.de \
    --to=hch@lst.de \
    --cc=a.manzanares@samsung.com \
    --cc=anuj20.g@samsung.com \
    --cc=asml.silence@gmail.com \
    --cc=axboe@kernel.dk \
    --cc=io-uring@vger.kernel.org \
    --cc=javier@javigon.com \
    --cc=joshi.k@samsung.com \
    --cc=joshiiitr@gmail.com \
    --cc=kbusch@kernel.org \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-nvme@lists.infradead.org \
    --cc=logang@deltatee.com \
    --cc=mcgrof@kernel.org \
    --cc=pankydev8@gmail.com \
    --cc=sbates@raithlin.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.