io-uring.vger.kernel.org archive mirror
 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>,
	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: Wed, 8 Sep 2021 08:15:30 +0200	[thread overview]
Message-ID: <20210908061530.GA28505@lst.de> (raw)
In-Reply-To: <CA+1E3rJAav=4abJXs8fO49aiMNPqjv6dD7HBfhB+JQrNbaX3=A@mail.gmail.com>

On Tue, Sep 07, 2021 at 09:50:27PM +0530, Kanchan Joshi wrote:
> > 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 */
> +};

Two different issues here:

 - the idea of having a two layer indirection with op and a cmd doesn't
   really make much sense
 - if we want to avoid conflicts using 32-bit probably makes sense

So I'd turn op and unused into a single cmd field, use the ioctl encoding
macros for it (but preferably pick different numbers than the existing
ioctls).

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

What would be the problem with the delaying?

> 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?

We don't need the bio pointer at all.  The old passthrough code needed
it when we still used block layer bonuce buffering for it.  But that
bounce buffering for passthrough commands got removed a while ago,
and even before nvme never used it.

  reply	other threads:[~2021-09-08  6:15 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
2021-09-08  6:15           ` Christoph Hellwig [this message]
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=20210908061530.GA28505@lst.de \
    --to=hch@lst.de \
    --cc=anuj20.g@samsung.com \
    --cc=axboe@kernel.dk \
    --cc=hare@suse.de \
    --cc=io-uring@vger.kernel.org \
    --cc=javier.gonz@samsung.com \
    --cc=joshi.k@samsung.com \
    --cc=joshiiitr@gmail.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).