Re: [Qemu-block] [Qemu-devel] [RFC PATCH 1/9] qapi/block-core: add option for io_uring

[Kevin Wolf]

Am 22.05.2019 um 02:51 hat Aarushi Mehta geschrieben:
> On Tue, 2019-05-21 at 19:39 -0500, Eric Blake wrote:
> > On 5/21/19 6:52 PM, Aarushi Mehta wrote:
> > > Signed-off-by: Aarushi Mehta <address@hidden>
> > 
> > Sparse on the details. The subject line says what, but without a
> > 'why'
> > for how io_uring is different from existing aio options, it's hard to
> > see why I'd want to use it. Do you have any benchmark numbers?
> 
> For peak performance, io_uring helps us get to 1.7M 4k IOPS with
> polling. aio reaches a performance cliff much lower than that, at 608K.
> If we disable polling, io_uring is able to drive about 1.2M IOPS for
> the (otherwise) same test case.
> 
> More details, and the source for the above is at
> http://kernel.dk/io_uring.pdf
> 
> > > ---
> > >  qapi/block-core.json | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/qapi/block-core.json b/qapi/block-core.json
> > > index 7ccbfff9d0..116995810a 100644
> > > --- a/qapi/block-core.json
> > > +++ b/qapi/block-core.json
> > > @@ -2776,11 +2776,12 @@
> > >  #
> > >  # @threads:     Use qemu's thread pool
> > >  # @native:      Use native AIO backend (only Linux and Windows)
> > > +# @io_uring:    Use linux io_uring
> > 
> > Missing a '(since 4.1)' tag.
> > 
> > >  #
> > >  # Since: 2.9
> > >  ##
> > >  { 'enum': 'BlockdevAioOptions',
> > > -  'data': [ 'threads', 'native' ] }
> > > +  'data': [ 'threads', 'native','io_uring' ] }
> > 
> > Missing space after ',' (not essential, but matching style is nice).
> > Should the new element be defined conditionally, so that
> > introspection
> > only sees the new enum member when compiled for Linux?
> > 
> I'm not sure what would be the benefits of that? We already check for
> Linux at configure, and this would reduce readability. We aren't doing
> this for native.

BlockdevAioOptions is used in BlockdevOptionsFile, which contains the
options for two different drivers: file-posix and file-win32. Both of
them support both 'threads' and 'native'. However, I don't think you'll
add the new mode to the Windows driver. So I think making it conditional
on CONFIG_POSIX at least is necessary.

Kevin