linux-block.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jens Axboe <axboe@kernel.dk>
To: Christoph Hellwig <hch@infradead.org>
Cc: linux-fsdevel@vger.kernel.org, linux-aio@kvack.org,
	linux-block@vger.kernel.org, hch@lst.de, viro@zeniv.linux.org.uk
Subject: Re: [PATCH 07/22] aio: add io_setup2() system call
Date: Thu, 27 Dec 2018 13:27:18 -0700	[thread overview]
Message-ID: <a642ec29-4894-d75e-5040-47f3eda8f759@kernel.dk> (raw)
In-Reply-To: <20181227135509.GA25525@infradead.org>

On 12/27/18 6:55 AM, Christoph Hellwig wrote:
> On Fri, Dec 21, 2018 at 12:22:21PM -0700, Jens Axboe wrote:
>> This is just like io_setup(), except add a flags argument to let the
>> caller control/define some of the io_context behavior.
>>
>> Outside of the flags, we add two user pointers for future use.
>>
>> Reviewed-by: Christoph Hellwig <hch@lst.de>
> 
> Hmm, I don't think think I was fine with the current version..

https://marc.info/?l=linux-fsdevel&m=154539128525404&w=2

>> +/* sys_io_setup2:
>> + *	Like sys_io_setup(), except that it takes a set of flags
>> + *	(IOCTX_FLAG_*), and some pointers to user structures:
>> + *
>> + *	*user1 - reserved for future use
>> + *
>> + *	*user2 - reserved for future use.
>> + */
>> +SYSCALL_DEFINE5(io_setup2, u32, nr_events, u32, flags, void __user *, user1,
>> +		void __user *, user2, aio_context_t __user *, ctxp)
> 
> Most importantly I'm really worried about these new user1 and user2
> fields, which aren't really used for aio poll.  They are used for
> the I/O ring, which while I think is a really great concept, I am also
> worried about overlaying into the aio interface just creating confusion..

As I said before, I don't want to create something entirely new just for
the sake of it. We end up reusing basically all aio data structures and
code paths, of course with some deviations here and there.

I don't see why it would cause confusion. Existing aio users are going
to be using io_setup(2) and nothing changes on that front. If you want
polled aio without using the ring, you can do so with io_setup2(2).
The fact that the ring addition ends up being setup in the same
fashion doesn't seem problematic to me. For the polled part, 99%
of the code ends up being the same.

-- 
Jens Axboe


  reply	other threads:[~2018-12-27 20:27 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-21 19:22 [PATCHSET v10] Support for polled and buffered aio (and more) Jens Axboe
2018-12-21 19:22 ` [PATCH 01/22] fs: add an iopoll method to struct file_operations Jens Axboe
2018-12-21 19:22 ` [PATCH 02/22] block: add bio_set_polled() helper Jens Axboe
2018-12-21 19:22 ` [PATCH 03/22] block: wire up block device iopoll method Jens Axboe
2018-12-21 19:22 ` [PATCH 04/22] block: use REQ_HIPRI_ASYNC for non-sync polled IO Jens Axboe
2018-12-21 19:25   ` Jens Axboe
2018-12-27 13:15     ` Christoph Hellwig
2018-12-27 20:22       ` Jens Axboe
2018-12-21 19:22 ` [PATCH 05/22] block: use bio_set_polled() helper for O_DIRECT Jens Axboe
2018-12-21 19:22 ` [PATCH 06/22] iomap: wire up the iopoll method Jens Axboe
2018-12-21 19:22 ` [PATCH 07/22] aio: add io_setup2() system call Jens Axboe
2018-12-27 13:55   ` Christoph Hellwig
2018-12-27 20:27     ` Jens Axboe [this message]
2018-12-21 19:22 ` [PATCH 08/22] aio: support for IO polling Jens Axboe
2018-12-27 13:55   ` Christoph Hellwig
2018-12-27 20:29     ` Jens Axboe
2018-12-21 19:22 ` [PATCH 09/22] aio: add submission side request cache Jens Axboe
2018-12-27 13:56   ` Christoph Hellwig
2018-12-27 20:31     ` Jens Axboe
2018-12-21 19:22 ` [PATCH 10/22] fs: add fget_many() and fput_many() Jens Axboe
2018-12-21 19:22 ` [PATCH 11/22] aio: use fget/fput_many() for file references Jens Axboe
2018-12-21 19:22 ` [PATCH 12/22] aio: split iocb init from allocation Jens Axboe
2018-12-21 19:22 ` [PATCH 13/22] aio: batch aio_kiocb allocation Jens Axboe
2018-12-21 19:22 ` [PATCH 14/22] aio: split old ring complete out from aio_complete() Jens Axboe
2018-12-21 19:22 ` [PATCH 15/22] aio: pass in user index to __io_submit_one() Jens Axboe
2018-12-21 19:22 ` [PATCH 16/22] aio: add support for submission/completion rings Jens Axboe
2018-12-27 13:47   ` Christoph Hellwig
2019-01-02 16:28     ` Jens Axboe
2019-01-02 20:32       ` Jens Axboe
2019-01-02 19:11   ` Jeff Moyer
2018-12-21 19:22 ` [PATCH 17/22] block: implement bio helper to add iter bvec pages to bio Jens Axboe
2018-12-21 19:22 ` [PATCH 18/22] aio: add support for pre-mapped user IO buffers Jens Axboe
2018-12-27 13:57   ` Christoph Hellwig
2018-12-21 19:22 ` [PATCH 19/22] aio: support kernel side submission for aio with SCQRING Jens Axboe
2018-12-27 13:57   ` Christoph Hellwig
2018-12-27 20:34     ` Jens Axboe
2018-12-21 19:22 ` [PATCH 20/22] aio: enable polling for IOCTX_FLAG_SQTHREAD Jens Axboe
2018-12-21 19:22 ` [PATCH 21/22] aio: utilize io_event->res2 for CQ ring Jens Axboe
2018-12-21 19:22 ` [PATCH 22/22] aio: add my copyright 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=a642ec29-4894-d75e-5040-47f3eda8f759@kernel.dk \
    --to=axboe@kernel.dk \
    --cc=hch@infradead.org \
    --cc=hch@lst.de \
    --cc=linux-aio@kvack.org \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    /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;
as well as URLs for NNTP newsgroup(s).