Signed-off-by: Aarushi Mehta <mehta.aaru20@gmail.com>
---
 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
 #
 # Since: 2.9
 ##
 { 'enum': 'BlockdevAioOptions',
-  'data': [ 'threads', 'native' ] }
+  'data': [ 'threads', 'native','io_uring' ] }

 ##
 # @BlockdevCacheOptions:
--
2.17.1

On 5/21/19 6:52 PM, Aarushi Mehta wrote:
> Signed-off-by: Aarushi Mehta <mehta.aaru20@gmail.com>

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?

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



-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

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 <mehta.aaru20@gmail.com>
> 
> 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.
> 

On 5/21/19 7:51 PM, Aarushi Mehta wrote:

>>> +# @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.

Look at BlockdevOptionsFile in qapi/block-core.qapi. Telling the QAPI
generator that something is only available on Linux means that it will
be obvious to introspection (the QMP command query-qmp-schema) whether
the feature is present in a particular binary.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

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 <mehta.aaru20@gmail.com>
> > 
> > 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

On Wed, May 22, 2019 at 06:21:51AM +0530, Aarushi Mehta wrote:
> 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 <mehta.aaru20@gmail.com>
> > 
> > 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

So that Aarushi's email isn't accidentally misquoted later on:

These numbers are not via QEMU.  QEMU is likely to show different
performance results and they are expected to be lower due to
virtualization overhead.

Stefan