io-uring.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Kanchan Joshi <joshiiitr@gmail.com>
To: Christoph Hellwig <hch@lst.de>
Cc: Kanchan Joshi <joshi.k@samsung.com>, Jens Axboe <axboe@kernel.dk>,
	Keith Busch <kbusch@kernel.org>,
	io-uring@vger.kernel.org, linux-nvme@lists.infradead.org,
	anuj20.g@samsung.com, Javier Gonzalez <javier.gonz@samsung.com>,
	hare@suse.de
Subject: Re: [RFC PATCH 2/6] nvme: wire-up support for async-passthru on char-device.
Date: Tue, 7 Sep 2021 21:50:27 +0530	[thread overview]
Message-ID: <CA+1E3rJAav=4abJXs8fO49aiMNPqjv6dD7HBfhB+JQrNbaX3=A@mail.gmail.com> (raw)
In-Reply-To: <20210907074650.GB29874@lst.de>

On Tue, Sep 7, 2021 at 1:17 PM Christoph Hellwig <hch@lst.de> wrote:
>
> Looking at this in isolation:
>
>  - no need to also implement the legacy non-64 passthrough interface
>  - no need to overlay the block_uring_cmd structure as that makes a
>    complete mess
>
> Below is an untested patch to fix that up a bit.

Thanks for taking a look and cleaning that up. Looks a lot better.

> A few other notes:
>
>  - I suspect the ioctl_cmd really should move into the core using_cmd
>    infrastructure

Yes, that seems possible by creating that field outside by combining
"op" and "unused" below.
+struct io_uring_cmd {
+ struct file *file;
+ __u16 op;
+ __u16 unused;
+ __u32 len;
+ __u64 pdu[5]; /* 40 bytes available inline for free use */
+};

>  - please stick to the naming of the file operation instead of using
>    something different.  That being said async_ioctl seems better
>    fitting than uring_cmd

Got it.

>  - that whole mix of user space interface and internal data in the
>    ->pdu field is a mess.  What is the problem with deferring the
>    request freeing into the user context, which would clean up
>    quite a bit of that, especially if io_uring_cmd grows a private
>    field.

That mix isn't great but the attempt was to save the allocation.
And I was not very sure if it'd be fine to defer freeing the request
until task-work fires up.
Even if we take that route, we would still need a place to store bio
pointer (hopefully meta pointer can be extracted out of bio).
Do you see it differently?


Thanks,
-- 
Kanchan

  reply	other threads:[~2021-09-07 16:21 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20210805125910epcas5p1100e7093dd2b1ac5bbb751331e2ded23@epcas5p1.samsung.com>
2021-08-05 12:55 ` [RFC PATCH 0/6] Fixed-buffers io_uring passthrough over nvme-char Kanchan Joshi
     [not found]   ` <CGME20210805125917epcas5p4f75c9423a7b886dc79500901cc8f55ab@epcas5p4.samsung.com>
2021-08-05 12:55     ` [RFC PATCH 1/6] io_uring: add infra for uring_cmd completion in submitter-task Kanchan Joshi
     [not found]   ` <CGME20210805125923epcas5p10e6c1b95475440be68f58244d5a3cb9a@epcas5p1.samsung.com>
2021-08-05 12:55     ` [RFC PATCH 2/6] nvme: wire-up support for async-passthru on char-device Kanchan Joshi
2021-09-07  7:46       ` Christoph Hellwig
2021-09-07 16:20         ` Kanchan Joshi [this message]
2021-09-08  6:15           ` Christoph Hellwig
2021-09-22  7:19             ` Kanchan Joshi
     [not found]   ` <CGME20210805125927epcas5p28f3413fe3d0a2baed37a05453df0d482@epcas5p2.samsung.com>
2021-08-05 12:55     ` [RFC PATCH 3/6] io_uring: mark iopoll not supported for uring-cmd Kanchan Joshi
     [not found]   ` <CGME20210805125931epcas5p259fec172085ea34fdbf5a1c1f8da5e90@epcas5p2.samsung.com>
2021-08-05 12:55     ` [RFC PATCH 4/6] io_uring: add helper for fixed-buffer uring-cmd Kanchan Joshi
2021-09-07  7:47       ` Christoph Hellwig
     [not found]   ` <CGME20210805125934epcas5p4ff88e95d558ad9f65d77a888a4211b18@epcas5p4.samsung.com>
2021-08-05 12:55     ` [RFC PATCH 5/6] io_uring: add support for uring_cmd with fixed-buffer Kanchan Joshi
2021-09-07  7:48       ` Christoph Hellwig
2021-09-07 16:29         ` Kanchan Joshi
     [not found]   ` <CGME20210805125937epcas5p15667b460e28d87bd40400f69005aafe3@epcas5p1.samsung.com>
2021-08-05 12:55     ` [RFC PATCH 6/6] nvme: enable passthrough " Kanchan Joshi
2021-09-07  7:50       ` Christoph Hellwig
2021-09-07 16:47         ` Kanchan Joshi
2021-09-08  6:16           ` Christoph Hellwig
2021-09-07  7:10   ` [RFC PATCH 0/6] Fixed-buffers io_uring passthrough over nvme-char Christoph Hellwig
2021-09-07 12:38     ` Jens Axboe

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+1E3rJAav=4abJXs8fO49aiMNPqjv6dD7HBfhB+JQrNbaX3=A@mail.gmail.com' \
    --to=joshiiitr@gmail.com \
    --cc=anuj20.g@samsung.com \
    --cc=axboe@kernel.dk \
    --cc=hare@suse.de \
    --cc=hch@lst.de \
    --cc=io-uring@vger.kernel.org \
    --cc=javier.gonz@samsung.com \
    --cc=joshi.k@samsung.com \
    --cc=kbusch@kernel.org \
    --cc=linux-nvme@lists.infradead.org \
    --subject='Re: [RFC PATCH 2/6] nvme: wire-up support for async-passthru on char-device.' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).