[Qemu-devel] [RFC PATCH v2 0/9] Add support for io_uring

[Qemu-devel] [RFC PATCH v2 0/9] Add support for io_uring

[Aarushi Mehta]

This patch series adds support for the newly developed io_uring Linux AIO interface. Linux io_uring is faster than Linux's AIO asynchronous I/O code, offers efficient buffered asynchronous I/O support, the ability to do I/O without performing a system call via polled I/O, and other efficiency enhancements. It is expected that it will replace the old AIO interface

Testing it requires a host kernel 5.1 or newer and the liburing library. Use the option -drive aio=io_uring to enable it.

v2:
- Fix Patchew errors
- Option now enumerates only for CONFIG_LINUX in qapi
- Removed redudant and broken code in io_uring
- io_uring now aborts on sqe leak

Aarushi Mehta (9):
  qapi/block-core: add option for io_uring
  block/block: add BDRV flag for io_uring
  include/block: declare interfaces for io_uring
  stubs: add aio interface stubs for io_uring
  util/async: add aio interfaces for io_uring
  block/io_uring: implements interfaces for io_uring
  blockdev: accept io_uring as option
  block/file-posix: extends to use with io_uring
  configure: permit use of io_uring

 MAINTAINERS             |  11 ++
 block/Makefile.objs     |   2 +
 block/file-posix.c      |  63 ++++++++-
 block/io_uring.c        | 306 ++++++++++++++++++++++++++++++++++++++++
 blockdev.c              |   4 +-
 configure               |  27 ++++
 include/block/aio.h     |  16 ++-
 include/block/block.h   |   1 +
 include/block/raw-aio.h |  15 ++
 qapi/block-core.json    |   6 +-
 stubs/Makefile.objs     |   1 +
 stubs/io_uring.c        |  32 +++++
 util/async.c            |  36 +++++
 13 files changed, 511 insertions(+), 9 deletions(-)
 create mode 100644 block/io_uring.c
 create mode 100644 stubs/io_uring.c

--
2.17.1

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

[Aarushi Mehta]

Signed-off-by: Aarushi Mehta <mehta.aaru20@gmail.com>
---
 qapi/block-core.json | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 7ccbfff9d0..0e927b247d 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2776,11 +2776,13 @@
 #
 # @threads:     Use qemu's thread pool
 # @native:      Use native AIO backend (only Linux and Windows)
+# @io_uring:    Use linux io_uring (only Linux)
 #
-# Since: 2.9
+# Since: 4.1
 ##
 { 'enum': 'BlockdevAioOptions',
-  'data': [ 'threads', 'native' ] }
+  'data': [ 'threads', 'native',
+            { 'name': 'io_uring', 'if': 'defined(CONFIG_LINUX)' } ] }

 ##
 # @BlockdevCacheOptions:
--
2.17.1

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

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1257 bytes --]

On Fri, May 24, 2019 at 07:33:29PM +0530, Aarushi Mehta wrote:
> Signed-off-by: Aarushi Mehta <mehta.aaru20@gmail.com>
> ---
>  qapi/block-core.json | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 7ccbfff9d0..0e927b247d 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -2776,11 +2776,13 @@
>  #
>  # @threads:     Use qemu's thread pool
>  # @native:      Use native AIO backend (only Linux and Windows)
> +# @io_uring:    Use linux io_uring (only Linux)
>  #
> -# Since: 2.9
> +# Since: 4.1

Please leave "Since: 2.9" unchanged and mark only @io_uring "Since:
4.1".  This way users can see that this specific option was added in
4.1 and the rest has been available since 2.9.

>  ##
>  { 'enum': 'BlockdevAioOptions',
> -  'data': [ 'threads', 'native' ] }
> +  'data': [ 'threads', 'native',
> +            { 'name': 'io_uring', 'if': 'defined(CONFIG_LINUX)' } ] }

What about CONFIG_LINUX_IO_URING?  That way the feature wouldn't be
advertized on Linux systems that don't offer io_uring.

Note that in order to do this you'll need to move the ./configure
patches before this patch (or squash them into this patch).

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Markus Armbruster]

Stefan Hajnoczi <stefanha@redhat.com> writes:

> On Fri, May 24, 2019 at 07:33:29PM +0530, Aarushi Mehta wrote:
>> Signed-off-by: Aarushi Mehta <mehta.aaru20@gmail.com>
>> ---
>>  qapi/block-core.json | 6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>> 
>> diff --git a/qapi/block-core.json b/qapi/block-core.json
>> index 7ccbfff9d0..0e927b247d 100644
>> --- a/qapi/block-core.json
>> +++ b/qapi/block-core.json
>> @@ -2776,11 +2776,13 @@
>>  #
>>  # @threads:     Use qemu's thread pool
>>  # @native:      Use native AIO backend (only Linux and Windows)
>> +# @io_uring:    Use linux io_uring (only Linux)
>>  #
>> -# Since: 2.9
>> +# Since: 4.1
>
> Please leave "Since: 2.9" unchanged and mark only @io_uring "Since:
> 4.1".  This way users can see that this specific option was added in
> 4.1 and the rest has been available since 2.9.
>
>>  ##
>>  { 'enum': 'BlockdevAioOptions',
>> -  'data': [ 'threads', 'native' ] }
>> +  'data': [ 'threads', 'native',
>> +            { 'name': 'io_uring', 'if': 'defined(CONFIG_LINUX)' } ] }
>
> What about CONFIG_LINUX_IO_URING?  That way the feature wouldn't be
> advertized on Linux systems that don't offer io_uring.

Yes, please.

Also, drop (only Linux) from the doc comment.  As is, qemu-qmp-ref.txt
looks like this:

     'io_uring'
          Use linux io_uring (only Linux)
          If: 'defined(CONFIG_LINUX)'

> Note that in order to do this you'll need to move the ./configure
> patches before this patch (or squash them into this patch).

[Qemu-devel] [RFC PATCH v2 2/9] block/block: add BDRV flag for io_uring

[Aarushi Mehta]

Signed-off-by: Aarushi Mehta <mehta.aaru20@gmail.com>
---
 include/block/block.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/block/block.h b/include/block/block.h
index 9b083e2bca..60f7c6c01c 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -121,6 +121,7 @@ typedef struct HDGeometry {
                                       ignoring the format layer */
 #define BDRV_O_NO_IO       0x10000 /* don't initialize for I/O */
 #define BDRV_O_AUTO_RDONLY 0x20000 /* degrade to read-only if opening read-write fails */
+#define BDRV_O_IO_URING    0x40000 /* use io_uring instead of the thread pool */

 #define BDRV_O_CACHE_MASK  (BDRV_O_NOCACHE | BDRV_O_NO_FLUSH)

--
2.17.1

Re: [Qemu-devel] [RFC PATCH v2 2/9] block/block: add BDRV flag for io_uring

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 242 bytes --]

On Fri, May 24, 2019 at 07:33:30PM +0530, Aarushi Mehta wrote:
> Signed-off-by: Aarushi Mehta <mehta.aaru20@gmail.com>
> ---
>  include/block/block.h | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[Qemu-devel] [RFC PATCH v2 3/9] include/block: declare interfaces for io_uring

[Aarushi Mehta]

Signed-off-by: Aarushi Mehta <mehta.aaru20@gmail.com>
---
 include/block/aio.h     | 16 +++++++++++++++-
 include/block/raw-aio.h | 15 +++++++++++++++
 2 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/include/block/aio.h b/include/block/aio.h
index 0ca25dfec6..9da3fd9793 100644
--- a/include/block/aio.h
+++ b/include/block/aio.h
@@ -50,6 +50,7 @@ typedef void IOHandler(void *opaque);
 struct Coroutine;
 struct ThreadPool;
 struct LinuxAioState;
+struct LuringState;

 struct AioContext {
     GSource source;
@@ -118,11 +119,19 @@ struct AioContext {
     struct ThreadPool *thread_pool;

 #ifdef CONFIG_LINUX_AIO
-    /* State for native Linux AIO.  Uses aio_context_acquire/release for
+    /*
+     * State for native Linux AIO.  Uses aio_context_acquire/release for
      * locking.
      */
     struct LinuxAioState *linux_aio;
 #endif
+#ifdef CONFIG_LINUX_IO_URING
+    /*
+     * State for Linux io_uring.  Uses aio_context_acquire/release for
+     * locking.
+     */
+    struct LuringState *linux_io_uring;
+#endif

     /* TimerLists for calling timers - one per clock type.  Has its own
      * locking.
@@ -387,6 +396,11 @@ struct LinuxAioState *aio_setup_linux_aio(AioContext *ctx, Error **errp);
 /* Return the LinuxAioState bound to this AioContext */
 struct LinuxAioState *aio_get_linux_aio(AioContext *ctx);

+/* Setup the LuringState bound to this AioContext */
+struct LuringState *aio_setup_linux_io_uring(AioContext *ctx, Error **errp);
+
+/* Return the LuringState bound to this AioContext */
+struct LuringState *aio_get_linux_io_uring(AioContext *ctx);
 /**
  * aio_timer_new_with_attrs:
  * @ctx: the aio context
diff --git a/include/block/raw-aio.h b/include/block/raw-aio.h
index ba223dd1f1..28a836151e 100644
--- a/include/block/raw-aio.h
+++ b/include/block/raw-aio.h
@@ -58,6 +58,21 @@ void laio_attach_aio_context(LinuxAioState *s, AioContext *new_context);
 void laio_io_plug(BlockDriverState *bs, LinuxAioState *s);
 void laio_io_unplug(BlockDriverState *bs, LinuxAioState *s);
 #endif
+/* io_uring.c - Linux io_uring implementation */
+#ifdef CONFIG_LINUX_IO_URING
+typedef struct LuringState LuringState;
+LuringState *luring_init(Error **errp);
+void luring_cleanup(LuringState *s);
+int coroutine_fn luring_co_submit(BlockDriverState *bs, LuringState *s, int fd,
+                                uint64_t offset, QEMUIOVector *qiov, int type);
+BlockAIOCB *luring_submit(BlockDriverState *bs, LuringState *s, int fd,
+        int64_t sector_num, QEMUIOVector *qiov, BlockCompletionFunc *cb,
+        void *opaque, int type);
+void luring_detach_aio_context(LuringState *s, AioContext *old_context);
+void luring_attach_aio_context(LuringState *s, AioContext *new_context);
+void luring_io_plug(BlockDriverState *bs, LuringState *s);
+void luring_io_unplug(BlockDriverState *bs, LuringState *s);
+#endif

 #ifdef _WIN32
 typedef struct QEMUWin32AIOState QEMUWin32AIOState;
--
2.17.1

Re: [Qemu-devel] [RFC PATCH v2 3/9] include/block: declare interfaces for io_uring

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 618 bytes --]

On Fri, May 24, 2019 at 07:33:31PM +0530, Aarushi Mehta wrote:
> Signed-off-by: Aarushi Mehta <mehta.aaru20@gmail.com>
> ---
>  include/block/aio.h     | 16 +++++++++++++++-
>  include/block/raw-aio.h | 15 +++++++++++++++
>  2 files changed, 30 insertions(+), 1 deletion(-)

There's not much a code reviewer can do with this patch in isolation
since they don't know the behavior of the functions declared here.
Please squash this patch with the patch containing the function
definitions.  That way a full review can be done to decide "is this the
correct function prototype?", "can the interface be simplified?", etc.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[Qemu-devel] [RFC PATCH v2 4/9] stubs: add aio interface stubs for io_uring

[Aarushi Mehta]

Signed-off-by: Aarushi Mehta <mehta.aaru20@gmail.com>
---
 MAINTAINERS         | 10 ++++++++++
 stubs/Makefile.objs |  1 +
 stubs/io_uring.c    | 32 ++++++++++++++++++++++++++++++++
 3 files changed, 43 insertions(+)
 create mode 100644 stubs/io_uring.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 3cacd751bf..b8fc1e3fe3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2504,6 +2504,16 @@ F: block/file-posix.c
 F: block/file-win32.c
 F: block/win32-aio.c

+Linux io_uring
+M: Aarushi Mehta <mehta.aaru20@gmail.com>
+R: Stefan Hajnoczi <stefan@redhat.com>
+L: qemu-block@nongnu.org
+S: Maintained
+F: stubs/io_uring.c
+
+
+L: qemu-block@nongnu.original
+
 qcow2
 M: Kevin Wolf <kwolf@redhat.com>
 M: Max Reitz <mreitz@redhat.com>
diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index 73452ad265..ea158cf0ee 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -13,6 +13,7 @@ stub-obj-y += iothread.o
 stub-obj-y += iothread-lock.o
 stub-obj-y += is-daemonized.o
 stub-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
+stub-obj-$(CONFIG_LINUX_IO_URING) += io_uring.o
 stub-obj-y += machine-init-done.o
 stub-obj-y += migr-blocker.o
 stub-obj-y += change-state-handler.o
diff --git a/stubs/io_uring.c b/stubs/io_uring.c
new file mode 100644
index 0000000000..622d1e4648
--- /dev/null
+++ b/stubs/io_uring.c
@@ -0,0 +1,32 @@
+/*
+ * Linux io_uring support.
+ *
+ * Copyright (C) 2009 IBM, Corp.
+ * Copyright (C) 2009 Red Hat, Inc.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+#include "qemu/osdep.h"
+#include "block/aio.h"
+#include "block/raw-aio.h"
+
+void luring_detach_aio_context(LuringState *s, AioContext *old_context)
+{
+    abort();
+}
+
+void luring_attach_aio_context(LuringState *s, AioContext *new_context)
+{
+    abort();
+}
+
+LuringState *luring_init(Error **errp)
+{
+    abort();
+}
+
+void luring_cleanup(LuringState *s)
+{
+    abort();
+}
--
2.17.1

Re: [Qemu-devel] [RFC PATCH v2 4/9] stubs: add aio interface stubs for io_uring

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1078 bytes --]

On Fri, May 24, 2019 at 07:33:32PM +0530, Aarushi Mehta wrote:
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3cacd751bf..b8fc1e3fe3 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2504,6 +2504,16 @@ F: block/file-posix.c
>  F: block/file-win32.c
>  F: block/win32-aio.c
> 
> +Linux io_uring
> +M: Aarushi Mehta <mehta.aaru20@gmail.com>
> +R: Stefan Hajnoczi <stefan@redhat.com>

Julia: Are you willing to review io_uring.c patches in the future too?
I'd like to have all three of us here so we can share the work of
reviewing patches in this area after Outreachy.

> diff --git a/stubs/io_uring.c b/stubs/io_uring.c
> new file mode 100644
> index 0000000000..622d1e4648
> --- /dev/null
> +++ b/stubs/io_uring.c
> @@ -0,0 +1,32 @@
> +/*
> + * Linux io_uring support.
> + *
> + * Copyright (C) 2009 IBM, Corp.
> + * Copyright (C) 2009 Red Hat, Inc.

You could add yourself here and to other files if you wish:

  Copyright (C) 2019 Aarushi Mehta

Git logs carry fine-grained authorship information already, so people
often don't bother.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[Qemu-devel] [RFC PATCH v2 5/9] util/async: add aio interfaces for io_uring

[Aarushi Mehta]

Signed-off-by: Aarushi Mehta <mehta.aaru20@gmail.com>
---
 util/async.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/util/async.c b/util/async.c
index c10642a385..2709f0edc3 100644
--- a/util/async.c
+++ b/util/async.c
@@ -277,6 +277,14 @@ aio_ctx_finalize(GSource     *source)
     }
 #endif

+#ifdef CONFIG_LINUX_IO_URING
+    if (ctx->linux_io_uring) {
+        luring_detach_aio_context(ctx->linux_io_uring, ctx);
+        luring_cleanup(ctx->linux_io_uring);
+        ctx->linux_io_uring = NULL;
+    }
+#endif
+
     assert(QSLIST_EMPTY(&ctx->scheduled_coroutines));
     qemu_bh_delete(ctx->co_schedule_bh);

@@ -341,6 +349,29 @@ LinuxAioState *aio_get_linux_aio(AioContext *ctx)
 }
 #endif

+#ifdef CONFIG_LINUX_IO_URING
+LuringState *aio_setup_linux_io_uring(AioContext *ctx, Error **errp)
+{
+    if (ctx->linux_io_uring) {
+        return ctx->linux_io_uring;
+    }
+
+    ctx->linux_io_uring = luring_init(errp);
+    if (!ctx->linux_io_uring) {
+        return NULL;
+    }
+
+    luring_attach_aio_context(ctx->linux_io_uring, ctx);
+    return ctx->linux_io_uring;
+}
+
+LuringState *aio_get_linux_io_uring(AioContext *ctx)
+{
+    assert(ctx->linux_io_uring);
+    return ctx->linux_io_uring;
+}
+#endif
+
 void aio_notify(AioContext *ctx)
 {
     /* Write e.g. bh->scheduled before reading ctx->notify_me.  Pairs
@@ -432,6 +463,11 @@ AioContext *aio_context_new(Error **errp)
 #ifdef CONFIG_LINUX_AIO
     ctx->linux_aio = NULL;
 #endif
+
+#ifdef CONFIG_LINUX_IO_URING
+    ctx->linux_io_uring = NULL;
+#endif
+
     ctx->thread_pool = NULL;
     qemu_rec_mutex_init(&ctx->lock);
     timerlistgroup_init(&ctx->tlg, aio_timerlist_notify, ctx);
--
2.17.1

Re: [Qemu-devel] [RFC PATCH v2 5/9] util/async: add aio interfaces for io_uring

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 271 bytes --]

On Fri, May 24, 2019 at 07:33:33PM +0530, Aarushi Mehta wrote:
> Signed-off-by: Aarushi Mehta <mehta.aaru20@gmail.com>
> ---
>  util/async.c | 36 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[Qemu-devel] [RFC PATCH v2 6/9] block/io_uring: implements interfaces for io_uring

[Aarushi Mehta]

Signed-off-by: Aarushi Mehta <mehta.aaru20@gmail.com>
---
 MAINTAINERS         |   1 +
 block/Makefile.objs |   2 +
 block/io_uring.c    | 306 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 309 insertions(+)
 create mode 100644 block/io_uring.c

diff --git a/MAINTAINERS b/MAINTAINERS
index b8fc1e3fe3..770d562c6c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2510,6 +2510,7 @@ R: Stefan Hajnoczi <stefan@redhat.com>
 L: qemu-block@nongnu.org
 S: Maintained
 F: stubs/io_uring.c
+F: block/io_uring.c


 L: qemu-block@nongnu.original
diff --git a/block/Makefile.objs b/block/Makefile.objs
index 7a81892a52..262d413c6d 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -18,6 +18,7 @@ block-obj-y += block-backend.o snapshot.o qapi.o
 block-obj-$(CONFIG_WIN32) += file-win32.o win32-aio.o
 block-obj-$(CONFIG_POSIX) += file-posix.o
 block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
+block-obj-$(CONFIG_LINUX_IO_URING) += io_uring.o
 block-obj-y += null.o mirror.o commit.o io.o create.o
 block-obj-y += throttle-groups.o
 block-obj-$(CONFIG_LINUX) += nvme.o
@@ -61,5 +62,6 @@ block-obj-$(if $(CONFIG_LZFSE),m,n) += dmg-lzfse.o
 dmg-lzfse.o-libs   := $(LZFSE_LIBS)
 qcow.o-libs        := -lz
 linux-aio.o-libs   := -laio
+io_uring.o-libs    := -luring
 parallels.o-cflags := $(LIBXML2_CFLAGS)
 parallels.o-libs   := $(LIBXML2_LIBS)
diff --git a/block/io_uring.c b/block/io_uring.c
new file mode 100644
index 0000000000..817ec055db
--- /dev/null
+++ b/block/io_uring.c
@@ -0,0 +1,306 @@
+/*
+ * Linux io_uring support.
+ *
+ * Copyright (C) 2009 IBM, Corp.
+ * Copyright (C) 2009 Red Hat, Inc.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+#include <liburing.h>
+
+#include "qemu/osdep.h"
+#include "qemu-common.h"
+#include "block/aio.h"
+#include "qemu/queue.h"
+#include "block/block.h"
+#include "block/raw-aio.h"
+#include "qemu/event_notifier.h"
+#include "qemu/coroutine.h"
+#include "qapi/error.h"
+
+#define MAX_EVENTS 128
+
+typedef struct LuringAIOCB {
+    BlockAIOCB common;
+    Coroutine *co;
+    struct io_uring_sqe sqeq;
+    int ret;
+    QSIMPLEQ_ENTRY(LuringAIOCB) next;
+} LuringAIOCB;
+
+typedef struct LuringQueue {
+    int plugged;
+    unsigned int in_queue;
+    unsigned int in_flight;
+    bool blocked;
+    QSIMPLEQ_HEAD(, LuringAIOCB) pending;
+} LuringQueue;
+
+typedef struct LuringState {
+    AioContext *aio_context;
+
+    struct io_uring ring;
+
+    /* io queue for submit at batch.  Protected by AioContext lock. */
+    LuringQueue io_q;
+
+    /* I/O completion processing.  Only runs in I/O thread.  */
+    QEMUBH *completion_bh;
+    int event_idx;
+    int event_max;
+} LuringState;
+
+static void ioq_submit(LuringState *s);
+
+static inline int io_cqe_ret(struct io_uring_cqe *cqe)
+{
+    return cqe->res;
+}
+
+/**
+ * qemu_luring_process_completions:
+ * @s: AIO state
+ *
+ * Fetches completed I/O requests, consumes cqes and invokes their callbacks.
+ *
+ */
+static void qemu_luring_process_completions(LuringState *s)
+{
+    struct io_uring_cqe *cqes;
+
+    qemu_bh_schedule(s->completion_bh);
+
+    while ((s->event_max = s->io_q.in_flight) &&
+           !io_uring_peek_cqe(&s->ring, &cqes)) {
+        for (s->event_idx = 0; s->event_idx < s->event_max;) {
+            io_uring_cqe_seen(&s->ring, cqes);
+
+            LuringAIOCB *luringcb;
+            luringcb = g_malloc0(sizeof(luringcb));
+            luringcb->ret = io_cqe_ret(cqes);
+            /* Change counters one-by-one because we can be nested. */
+            s->io_q.in_flight--;
+            s->event_idx++;
+        }
+    }
+
+    qemu_bh_cancel(s->completion_bh);
+
+    /*
+     *If we are nested we have to notify the level above that we are done
+     * by setting event_max to zero, upper level will then jump out of it's
+     * own `for` loop.  If we are the last all counters dropped to zero.
+     */
+    s->event_max = 0;
+    s->event_idx = 0;
+}
+
+static void qemu_luring_process_completions_and_submit(LuringState *s)
+{
+    aio_context_acquire(s->aio_context);
+    qemu_luring_process_completions(s);
+
+    if (!s->io_q.plugged && !QSIMPLEQ_EMPTY(&s->io_q.pending)) {
+        ioq_submit(s);
+    }
+    aio_context_release(s->aio_context);
+}
+
+static void qemu_luring_completion_bh(void *opaque)
+{
+    LuringState *s = opaque;
+    qemu_luring_process_completions_and_submit(s);
+}
+
+static void qemu_luring_completion_cb(void *opaque)
+{
+    LuringState *s = opaque;
+    qemu_luring_process_completions_and_submit(s);
+}
+
+static const AIOCBInfo luring_aiocb_info = {
+    .aiocb_size         = sizeof(LuringAIOCB),
+};
+
+
+static void ioq_init(LuringQueue *io_q)
+{
+    QSIMPLEQ_INIT(&io_q->pending);
+    io_q->plugged = 0;
+    io_q->in_queue = 0;
+    io_q->in_flight = 0;
+    io_q->blocked = false;
+}
+
+static void ioq_submit(LuringState *s)
+{
+    int ret, len;
+    LuringAIOCB *luringcb;
+    QSIMPLEQ_HEAD(, LuringAIOCB) completed;
+
+    while (s->io_q.in_flight >= MAX_EVENTS && s->io_q.in_queue) {
+        len = 0;
+        QSIMPLEQ_FOREACH(luringcb, &s->io_q.pending, next) {
+            if (s->io_q.in_flight + len++ >= MAX_EVENTS) {
+                break;
+            }
+            struct io_uring_sqe *sqes = io_uring_get_sqe(&s->ring);
+            if (sqes)  { /* Prep sqe for subission */
+                memset(sqes, 0, sizeof(*sqes));
+                *sqes = luringcb->sqeq;
+                QSIMPLEQ_REMOVE_HEAD(&s->io_q.pending, next);
+            } else {
+                break;
+            }
+        }
+
+        ret =  io_uring_submit(&s->ring);
+        if (ret == -EAGAIN) {
+            break;
+        }
+
+        s->io_q.in_flight += ret;
+        s->io_q.in_queue  -= ret;
+        QSIMPLEQ_SPLIT_AFTER(&s->io_q.pending, luringcb, next, &completed);
+    }
+    s->io_q.blocked = (s->io_q.in_queue > 0);
+
+    if (s->io_q.in_flight) {
+        /*
+         * We can try to complete something just right away if there are
+         * still requests in-flight.
+         */
+        qemu_luring_process_completions(s);
+    }
+}
+
+void luring_io_plug(BlockDriverState *bs, LuringState *s)
+{
+    s->io_q.plugged++;
+}
+
+void luring_io_unplug(BlockDriverState *bs, LuringState *s)
+{
+    assert(s->io_q.plugged);
+    if (--s->io_q.plugged == 0 &&
+        !s->io_q.blocked && !QSIMPLEQ_EMPTY(&s->io_q.pending)) {
+        ioq_submit(s);
+    }
+}
+
+static int luring_do_submit(int fd, LuringAIOCB *luringcb, LuringState *s,
+                            uint64_t offset, QEMUIOVector *qiov, int type)
+{
+    struct io_uring_sqe *sqes = io_uring_get_sqe(&s->ring);
+    if (!sqes) {
+        sqes = &luringcb->sqeq;
+        QSIMPLEQ_INSERT_TAIL(&s->io_q.pending, luringcb, next);
+    }
+
+    switch (type) {
+    case QEMU_AIO_WRITE:
+        io_uring_prep_writev(sqes, fd, qiov->iov, qiov->niov, offset);
+        break;
+    case QEMU_AIO_READ:
+        io_uring_prep_readv(sqes, fd, qiov->iov, qiov->niov, offset);
+        break;
+    case QEMU_AIO_FLUSH:
+        io_uring_prep_fsync(sqes, fd, IORING_FSYNC_DATASYNC);
+        break;
+    default:
+        fprintf(stderr, "%s: invalid AIO request type, aborting 0x%x.\n",
+                        __func__, type);
+        abort();
+    }
+
+    s->io_q.in_queue++;
+    if (!s->io_q.blocked &&
+        (!s->io_q.plugged ||
+         s->io_q.in_flight + s->io_q.in_queue >= MAX_EVENTS)) {
+        ioq_submit(s);
+    }
+
+    return 0;
+}
+
+int coroutine_fn luring_co_submit(BlockDriverState *bs, LuringState *s, int fd,
+                                uint64_t offset, QEMUIOVector *qiov, int type)
+{
+    int ret;
+    LuringAIOCB luringcb = {
+        .co         = qemu_coroutine_self(),
+        .ret        = -EINPROGRESS,
+    };
+
+    ret = luring_do_submit(fd, &luringcb, s, offset, qiov, type);
+    if (ret < 0) {
+        return ret;
+    }
+
+    if (luringcb.ret == -EINPROGRESS) {
+        qemu_coroutine_yield();
+    }
+    return luringcb.ret;
+}
+
+BlockAIOCB *luring_submit(BlockDriverState *bs, LuringState *s, int fd,
+        int64_t sector_num, QEMUIOVector *qiov, BlockCompletionFunc *cb,
+        void *opaque, int type)
+{
+    LuringAIOCB *luringcb;
+    off_t offset = sector_num * BDRV_SECTOR_SIZE;
+    int ret;
+
+    luringcb = qemu_aio_get(&luring_aiocb_info, bs, cb, opaque);
+    luringcb->ret = -EINPROGRESS;
+    ret = luring_do_submit(fd, luringcb, s, offset, qiov, type);
+    if (ret < 0) {
+        qemu_aio_unref(luringcb);
+        return NULL;
+    }
+
+    return &luringcb->common;
+}
+
+void luring_detach_aio_context(LuringState *s, AioContext *old_context)
+{
+    aio_set_fd_handler(old_context, s->ring.ring_fd, false, NULL, NULL, NULL,
+                       &s);
+    qemu_bh_delete(s->completion_bh);
+    s->aio_context = NULL;
+}
+
+void luring_attach_aio_context(LuringState *s, AioContext *new_context)
+{
+    s->aio_context = new_context;
+    s->completion_bh = aio_bh_new(new_context, qemu_luring_completion_bh, s);
+    aio_set_fd_handler(s->aio_context, s->ring.ring_fd, false,
+                       (IOHandler *)qemu_luring_completion_cb, NULL, NULL, &s);
+}
+
+LuringState *luring_init(Error **errp)
+{
+    int rc;
+    LuringState *s;
+    s = g_malloc0(sizeof(*s));
+    struct io_uring *ring = &s->ring;
+    rc =  io_uring_queue_init(MAX_EVENTS, ring, 0);
+    if (rc == -1) {
+        error_setg_errno(errp, errno, "failed to init linux io_uring ring");
+        goto out_close_efd;
+    }
+
+    ioq_init(&s->io_q);
+    return s;
+
+out_close_efd:
+    g_free(s);
+    return NULL;
+}
+
+void luring_cleanup(LuringState *s)
+{
+    io_uring_queue_exit(&s->ring);
+    g_free(s);
+}
--
2.17.1

Re: [Qemu-devel] [RFC PATCH v2 6/9] block/io_uring: implements interfaces for io_uring

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 15954 bytes --]

On Fri, May 24, 2019 at 07:33:34PM +0530, Aarushi Mehta wrote:
> Signed-off-by: Aarushi Mehta <mehta.aaru20@gmail.com>
> ---
>  MAINTAINERS         |   1 +
>  block/Makefile.objs |   2 +
>  block/io_uring.c    | 306 ++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 309 insertions(+)
>  create mode 100644 block/io_uring.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b8fc1e3fe3..770d562c6c 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2510,6 +2510,7 @@ R: Stefan Hajnoczi <stefan@redhat.com>
>  L: qemu-block@nongnu.org
>  S: Maintained
>  F: stubs/io_uring.c
> +F: block/io_uring.c
> 
> 
>  L: qemu-block@nongnu.original
> diff --git a/block/Makefile.objs b/block/Makefile.objs
> index 7a81892a52..262d413c6d 100644
> --- a/block/Makefile.objs
> +++ b/block/Makefile.objs
> @@ -18,6 +18,7 @@ block-obj-y += block-backend.o snapshot.o qapi.o
>  block-obj-$(CONFIG_WIN32) += file-win32.o win32-aio.o
>  block-obj-$(CONFIG_POSIX) += file-posix.o
>  block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
> +block-obj-$(CONFIG_LINUX_IO_URING) += io_uring.o
>  block-obj-y += null.o mirror.o commit.o io.o create.o
>  block-obj-y += throttle-groups.o
>  block-obj-$(CONFIG_LINUX) += nvme.o
> @@ -61,5 +62,6 @@ block-obj-$(if $(CONFIG_LZFSE),m,n) += dmg-lzfse.o
>  dmg-lzfse.o-libs   := $(LZFSE_LIBS)
>  qcow.o-libs        := -lz
>  linux-aio.o-libs   := -laio
> +io_uring.o-libs    := -luring
>  parallels.o-cflags := $(LIBXML2_CFLAGS)
>  parallels.o-libs   := $(LIBXML2_LIBS)
> diff --git a/block/io_uring.c b/block/io_uring.c
> new file mode 100644
> index 0000000000..817ec055db
> --- /dev/null
> +++ b/block/io_uring.c
> @@ -0,0 +1,306 @@
> +/*
> + * Linux io_uring support.
> + *
> + * Copyright (C) 2009 IBM, Corp.
> + * Copyright (C) 2009 Red Hat, Inc.
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +#include <liburing.h>

Please move this include below osdep.h as per ./HACKING "1.2. Include
directives".

> +
> +#include "qemu/osdep.h"
> +#include "qemu-common.h"
> +#include "block/aio.h"
> +#include "qemu/queue.h"
> +#include "block/block.h"
> +#include "block/raw-aio.h"
> +#include "qemu/event_notifier.h"

Unused, please drop.

> +#include "qemu/coroutine.h"
> +#include "qapi/error.h"
> +
> +#define MAX_EVENTS 128
> +
> +typedef struct LuringAIOCB {
> +    BlockAIOCB common;
> +    Coroutine *co;
> +    struct io_uring_sqe sqeq;
> +    int ret;
> +    QSIMPLEQ_ENTRY(LuringAIOCB) next;
> +} LuringAIOCB;
> +
> +typedef struct LuringQueue {
> +    int plugged;
> +    unsigned int in_queue;
> +    unsigned int in_flight;
> +    bool blocked;
> +    QSIMPLEQ_HEAD(, LuringAIOCB) pending;
> +} LuringQueue;
> +
> +typedef struct LuringState {
> +    AioContext *aio_context;
> +
> +    struct io_uring ring;
> +
> +    /* io queue for submit at batch.  Protected by AioContext lock. */
> +    LuringQueue io_q;
> +
> +    /* I/O completion processing.  Only runs in I/O thread.  */
> +    QEMUBH *completion_bh;
> +    int event_idx;
> +    int event_max;
> +} LuringState;
> +
> +static void ioq_submit(LuringState *s);
> +
> +static inline int io_cqe_ret(struct io_uring_cqe *cqe)
> +{
> +    return cqe->res;
> +}
> +
> +/**
> + * qemu_luring_process_completions:
> + * @s: AIO state
> + *
> + * Fetches completed I/O requests, consumes cqes and invokes their callbacks.
> + *
> + */
> +static void qemu_luring_process_completions(LuringState *s)
> +{
> +    struct io_uring_cqe *cqes;
> +
> +    qemu_bh_schedule(s->completion_bh);

Please add a comment explaining why this is necessary:

  /*
   * Request completion callbacks can run the nested event loop.
   * Schedule ourselves so the nested event loop will "see" remaining
   * completed requests and process them.  Without this, completion
   * callbacks that wait for other requests using a nested event loop
   * would hang forever.
   */

> +
> +    while ((s->event_max = s->io_q.in_flight) &&
> +           !io_uring_peek_cqe(&s->ring, &cqes)) {
> +        for (s->event_idx = 0; s->event_idx < s->event_max;) {
> +            io_uring_cqe_seen(&s->ring, cqes);

What is the purpose of event_max/event_idx given that we can consume
cqes via io_uring_peek_cqe() + io_uring_cqe_seen()?

I think just this will do:

  while (!io_uring_peek_cqe(&s->ring, &cqes)) {
      int ret = io_cqe_ret(cqes);

      /* We're done accessing cqes, replenish the cq ring */
      io_uring_cqe_seen(&s->ring, cqes);

> +
> +            LuringAIOCB *luringcb;
> +            luringcb = g_malloc0(sizeof(luringcb));

We need to fetch the original LuringAIOCB associated with this request
instead of allocating a new one.  The original LuringAIOCB contains the
coroutine and async completion callback function pointer that we need.

io_uring allows us to pass user data along with the sqe and it gets
returned in the cqe.

During request submission:

  io_uring_sqe_set_data(sqes, luringcb);

During request completion:

  LuringAIOCB *luringcb = io_uring_cqe_get_data(cqes);

> +            luringcb->ret = io_cqe_ret(cqes);
> +            /* Change counters one-by-one because we can be nested. */
> +            s->io_q.in_flight--;
> +            s->event_idx++;

We must invoke completion and possibly unref luringcb here:

  if (luringcb->co) {
      /* If the coroutine is already entered it must be in ioq_submit() and
       * will notice laio->ret has been filled in when it eventually runs
       * later.  Coroutines cannot be entered recursively so avoid doing
       * that!
       */
      if (!qemu_coroutine_entered(luringcb->co)) {
          aio_wake(luringcb->co);
      }
  } else {
      luringcb->common.cb(luringcb->common.opaque, luringcb->ret);
      qemu_aio_unref(luringcb);
  }

Without this code the request is never completed.  How are you testing
this code (you mentioned the guest boots)?  Is it possible that the
Linux AIO or thread-pool code path is being taken accidentally and we're
actually not running io_uring yet?

> +        }
> +    }
> +
> +    qemu_bh_cancel(s->completion_bh);
> +
> +    /*
> +     *If we are nested we have to notify the level above that we are done
> +     * by setting event_max to zero, upper level will then jump out of it's
> +     * own `for` loop.  If we are the last all counters dropped to zero.
> +     */
> +    s->event_max = 0;
> +    s->event_idx = 0;
> +}
> +
> +static void qemu_luring_process_completions_and_submit(LuringState *s)
> +{
> +    aio_context_acquire(s->aio_context);
> +    qemu_luring_process_completions(s);
> +
> +    if (!s->io_q.plugged && !QSIMPLEQ_EMPTY(&s->io_q.pending)) {

Please use in_queue instead of checking io_q.pending since there might
be requests in the sq ring that also need to be submitted.

> +        ioq_submit(s);
> +    }
> +    aio_context_release(s->aio_context);
> +}
> +
> +static void qemu_luring_completion_bh(void *opaque)
> +{
> +    LuringState *s = opaque;
> +    qemu_luring_process_completions_and_submit(s);
> +}
> +
> +static void qemu_luring_completion_cb(void *opaque)
> +{
> +    LuringState *s = opaque;
> +    qemu_luring_process_completions_and_submit(s);
> +}
> +
> +static const AIOCBInfo luring_aiocb_info = {
> +    .aiocb_size         = sizeof(LuringAIOCB),
> +};
> +
> +
> +static void ioq_init(LuringQueue *io_q)
> +{
> +    QSIMPLEQ_INIT(&io_q->pending);
> +    io_q->plugged = 0;
> +    io_q->in_queue = 0;
> +    io_q->in_flight = 0;
> +    io_q->blocked = false;
> +}
> +
> +static void ioq_submit(LuringState *s)
> +{
> +    int ret, len;
> +    LuringAIOCB *luringcb;
> +    QSIMPLEQ_HEAD(, LuringAIOCB) completed;
> +
> +    while (s->io_q.in_flight >= MAX_EVENTS && s->io_q.in_queue) {

Try to submit requests as long as MAX_EVENTS has not been reached:

s/>=/</

By the way, is a loop necessary?  I don't see a scenario where we need
multiple iterations.  In fact, this is an infinite loop if the kernel
refuses to consume further requests for some reason.

> +        len = 0;
> +        QSIMPLEQ_FOREACH(luringcb, &s->io_q.pending, next) {
> +            if (s->io_q.in_flight + len++ >= MAX_EVENTS) {
> +                break;
> +            }

Can we rely on io_uring_get_sqe() failing when the sq ring is exhausted?
That way there's no need for this if statement.

> +            struct io_uring_sqe *sqes = io_uring_get_sqe(&s->ring);
> +            if (sqes)  { /* Prep sqe for subission */
> +                memset(sqes, 0, sizeof(*sqes));

memset is unnecessary since the next statement overwrites all of sqes.

> +                *sqes = luringcb->sqeq;
> +                QSIMPLEQ_REMOVE_HEAD(&s->io_q.pending, next);

Careful, a special API is needed when you want to modify the queue
during iteration.  Please see QSIMPLEQ_FOREACH_SAFE().

> +            } else {
> +                break;
> +            }

The simpler form of this if statement is:

  if (!sqes) {
      break;
  }

  ...the success case without nesting...

This is just a style suggestion.  I find code easier to read without
nesting.  Feel free to leave it if you prefer your way.

> +        }
> +
> +        ret =  io_uring_submit(&s->ring);
> +        if (ret == -EAGAIN) {
> +            break;
> +        }

Actually in all error cases since we don't want to increment
in_flight/in_queue with -errno:

  if (ret < 0) {
      break;
  }

Please add a TODO comment for error handling, the error is currently
ignored and this could be a problem.  In the EAGAIN case the kernel is
unable to process more requests temporarily and we should try again
later.  But we don't try again later, so this could result in hung I/O.

> +
> +        s->io_q.in_flight += ret;
> +        s->io_q.in_queue  -= ret;
> +        QSIMPLEQ_SPLIT_AFTER(&s->io_q.pending, luringcb, next, &completed);

Hmm...what is this doing?  QSIMPLEQ_REMOVE_HEAD() was already called
earlier on, so why modify the list again?

> +    }
> +    s->io_q.blocked = (s->io_q.in_queue > 0);
> +
> +    if (s->io_q.in_flight) {
> +        /*
> +         * We can try to complete something just right away if there are
> +         * still requests in-flight.
> +         */
> +        qemu_luring_process_completions(s);
> +    }
> +}
> +
> +void luring_io_plug(BlockDriverState *bs, LuringState *s)
> +{
> +    s->io_q.plugged++;
> +}
> +
> +void luring_io_unplug(BlockDriverState *bs, LuringState *s)
> +{
> +    assert(s->io_q.plugged);
> +    if (--s->io_q.plugged == 0 &&
> +        !s->io_q.blocked && !QSIMPLEQ_EMPTY(&s->io_q.pending)) {

Remember to take into account requests in the sq ring, they also need to
be submitted:

s/!QSIMPLEQ_EMPTY(&s->io_q.pending)/s->io_q.in_queue > 0/

> +        ioq_submit(s);
> +    }
> +}
> +
> +static int luring_do_submit(int fd, LuringAIOCB *luringcb, LuringState *s,
> +                            uint64_t offset, QEMUIOVector *qiov, int type)
> +{
> +    struct io_uring_sqe *sqes = io_uring_get_sqe(&s->ring);
> +    if (!sqes) {
> +        sqes = &luringcb->sqeq;
> +        QSIMPLEQ_INSERT_TAIL(&s->io_q.pending, luringcb, next);
> +    }
> +
> +    switch (type) {
> +    case QEMU_AIO_WRITE:
> +        io_uring_prep_writev(sqes, fd, qiov->iov, qiov->niov, offset);
> +        break;
> +    case QEMU_AIO_READ:
> +        io_uring_prep_readv(sqes, fd, qiov->iov, qiov->niov, offset);
> +        break;
> +    case QEMU_AIO_FLUSH:
> +        io_uring_prep_fsync(sqes, fd, IORING_FSYNC_DATASYNC);
> +        break;
> +    default:
> +        fprintf(stderr, "%s: invalid AIO request type, aborting 0x%x.\n",
> +                        __func__, type);
> +        abort();
> +    }
> +
> +    s->io_q.in_queue++;

It's easy to think that "in_queue" is just the length of io_q.pending,
but that's incorrect.  "pending" and "in_queue" have different semantics
and it's a bit subtle:

 * The "pending" queue is only used when the sq ring is full.

 * The "in_queue" counter indicates the total number of requests on the
   io_q.pending list and in the sq_ring but not yet consumed by the
   kernel.

Changing the names would help.  Here is my suggestion, maybe you have a
better idea:

 * Call the list io_q.sq_overflow so it's clear these are requests that
   didn't fit into the sq ring.

 * Call the counter num_unsubmitted.  I don't know about this one, it's
   hard to find a good name :).  Maybe it can be left as in_queue.

> +    if (!s->io_q.blocked &&
> +        (!s->io_q.plugged ||
> +         s->io_q.in_flight + s->io_q.in_queue >= MAX_EVENTS)) {
> +        ioq_submit(s);
> +    }
> +
> +    return 0;
> +}
> +
> +int coroutine_fn luring_co_submit(BlockDriverState *bs, LuringState *s, int fd,
> +                                uint64_t offset, QEMUIOVector *qiov, int type)
> +{
> +    int ret;
> +    LuringAIOCB luringcb = {
> +        .co         = qemu_coroutine_self(),
> +        .ret        = -EINPROGRESS,
> +    };
> +
> +    ret = luring_do_submit(fd, &luringcb, s, offset, qiov, type);
> +    if (ret < 0) {
> +        return ret;
> +    }
> +
> +    if (luringcb.ret == -EINPROGRESS) {
> +        qemu_coroutine_yield();
> +    }
> +    return luringcb.ret;
> +}
> +
> +BlockAIOCB *luring_submit(BlockDriverState *bs, LuringState *s, int fd,
> +        int64_t sector_num, QEMUIOVector *qiov, BlockCompletionFunc *cb,
> +        void *opaque, int type)
> +{
> +    LuringAIOCB *luringcb;
> +    off_t offset = sector_num * BDRV_SECTOR_SIZE;
> +    int ret;
> +
> +    luringcb = qemu_aio_get(&luring_aiocb_info, bs, cb, opaque);
> +    luringcb->ret = -EINPROGRESS;
> +    ret = luring_do_submit(fd, luringcb, s, offset, qiov, type);
> +    if (ret < 0) {
> +        qemu_aio_unref(luringcb);
> +        return NULL;
> +    }
> +
> +    return &luringcb->common;
> +}
> +
> +void luring_detach_aio_context(LuringState *s, AioContext *old_context)
> +{
> +    aio_set_fd_handler(old_context, s->ring.ring_fd, false, NULL, NULL, NULL,
> +                       &s);
> +    qemu_bh_delete(s->completion_bh);
> +    s->aio_context = NULL;
> +}
> +
> +void luring_attach_aio_context(LuringState *s, AioContext *new_context)
> +{
> +    s->aio_context = new_context;
> +    s->completion_bh = aio_bh_new(new_context, qemu_luring_completion_bh, s);
> +    aio_set_fd_handler(s->aio_context, s->ring.ring_fd, false,
> +                       (IOHandler *)qemu_luring_completion_cb, NULL, NULL, &s);

Casting function pointers is suspicious because it often indicates
unportable code (it relies on assumptions about the calling convention
that the compiler is using).

Luckily this type cast seems unnecessary and can be dropped:

  static void qemu_luring_completion_cb(void *opaque)
  ==
  typedef void IOHandler(void *opaque);

> +}
> +
> +LuringState *luring_init(Error **errp)
> +{
> +    int rc;
> +    LuringState *s;
> +    s = g_malloc0(sizeof(*s));
> +    struct io_uring *ring = &s->ring;
> +    rc =  io_uring_queue_init(MAX_EVENTS, ring, 0);
> +    if (rc == -1) {
> +        error_setg_errno(errp, errno, "failed to init linux io_uring ring");
> +        goto out_close_efd;
> +    }
> +
> +    ioq_init(&s->io_q);
> +    return s;
> +
> +out_close_efd:

There is no eventfd so "efd" is outdated.  Given that there are no other
gotos, you could just inline this return code and avoid the goto
altogether.

> +    g_free(s);
> +    return NULL;
> +}
> +
> +void luring_cleanup(LuringState *s)
> +{
> +    io_uring_queue_exit(&s->ring);
> +    g_free(s);
> +}
> --
> 2.17.1

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[Qemu-devel] [RFC PATCH v2 7/9] blockdev: accept io_uring as option

[Aarushi Mehta]

Signed-off-by: Aarushi Mehta <mehta.aaru20@gmail.com>
---
 blockdev.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/blockdev.c b/blockdev.c
index 79fbac8450..b44b9d660d 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -386,6 +386,8 @@ static void extract_common_blockdev_options(QemuOpts *opts, int *bdrv_flags,
         if ((aio = qemu_opt_get(opts, "aio")) != NULL) {
             if (!strcmp(aio, "native")) {
                 *bdrv_flags |= BDRV_O_NATIVE_AIO;
+            } else if (!strcmp(aio, "io_uring")) {
+                *bdrv_flags |= BDRV_O_IO_URING;
             } else if (!strcmp(aio, "threads")) {
                 /* this is the default */
             } else {
@@ -4547,7 +4549,7 @@ QemuOptsList qemu_common_drive_opts = {
         },{
             .name = "aio",
             .type = QEMU_OPT_STRING,
-            .help = "host AIO implementation (threads, native)",
+            .help = "host AIO implementation (threads, native, io_uring)",
         },{
             .name = BDRV_OPT_CACHE_WB,
             .type = QEMU_OPT_BOOL,
--
2.17.1

Re: [Qemu-devel] [RFC PATCH v2 7/9] blockdev: accept io_uring as option

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 250 bytes --]

On Fri, May 24, 2019 at 07:33:35PM +0530, Aarushi Mehta wrote:
> Signed-off-by: Aarushi Mehta <mehta.aaru20@gmail.com>
> ---
>  blockdev.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[Qemu-devel] [RFC PATCH v2 8/9] block/file-posix: extends to use with io_uring

[Aarushi Mehta]

Signed-off-by: Aarushi Mehta <mehta.aaru20@gmail.com>
---
 block/file-posix.c | 63 ++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 58 insertions(+), 5 deletions(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index d018429672..1f24618cfc 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -154,6 +154,7 @@ typedef struct BDRVRawState {
     bool has_write_zeroes:1;
     bool discard_zeroes:1;
     bool use_linux_aio:1;
+    bool use_linux_io_uring:1;
     bool page_cache_inconsistent:1;
     bool has_fallocate;
     bool needs_alignment;
@@ -423,7 +424,7 @@ static QemuOptsList raw_runtime_opts = {
         {
             .name = "aio",
             .type = QEMU_OPT_STRING,
-            .help = "host AIO implementation (threads, native)",
+            .help = "host AIO implementation (threads, native, io_uring)",
         },
         {
             .name = "locking",
@@ -494,6 +495,7 @@ static int raw_open_common(BlockDriverState *bs, QDict *options,
         goto fail;
     }
     s->use_linux_aio = (aio == BLOCKDEV_AIO_OPTIONS_NATIVE);
+    s->use_linux_io_uring = (aio == BLOCKDEV_AIO_OPTIONS_IO_URING);

     locking = qapi_enum_parse(&OnOffAuto_lookup,
                               qemu_opt_get(opts, "locking"),
@@ -557,7 +559,9 @@ static int raw_open_common(BlockDriverState *bs, QDict *options,
     s->shared_perm = BLK_PERM_ALL;

 #ifdef CONFIG_LINUX_AIO
-     /* Currently Linux does AIO only for files opened with O_DIRECT */
+     /*
+      * Currently Linux does AIO only for files opened with O_DIRECT
+      */
     if (s->use_linux_aio) {
         if (!(s->open_flags & O_DIRECT)) {
             error_setg(errp, "aio=native was specified, but it requires "
@@ -578,6 +582,21 @@ static int raw_open_common(BlockDriverState *bs, QDict *options,
         goto fail;
     }
 #endif /* !defined(CONFIG_LINUX_AIO) */
+#ifdef CONFIG_LINUX_IO_URING
+    if (s->use_linux_io_uring) {
+        if (!aio_setup_linux_io_uring(bdrv_get_aio_context(bs), errp)) {
+            error_prepend(errp, "Unable to use io_uring: ");
+            goto fail;
+        }
+    }
+#else
+    if (s->use_linux_io_uring) {
+        error_setg(errp, "aio=io_uring was specified, but is not supported "
+                         "in this build.");
+        ret = -EINVAL;
+        goto fail;
+    }
+#endif /* !defined(CONFIG_LINUX_IO_URING) */

     s->has_discard = true;
     s->has_write_zeroes = true;
@@ -1883,6 +1902,12 @@ static int coroutine_fn raw_co_prw(BlockDriverState *bs, uint64_t offset,
             LinuxAioState *aio = aio_get_linux_aio(bdrv_get_aio_context(bs));
             assert(qiov->size == bytes);
             return laio_co_submit(bs, aio, s->fd, offset, qiov, type);
+#endif
+#ifdef CONFIG_LINUX_IO_URING
+        } else if (s->use_linux_io_uring) {
+            LuringState *aio = aio_get_linux_io_uring(bdrv_get_aio_context(bs));
+            assert(qiov->size == bytes);
+            return luring_co_submit(bs, aio, s->fd, offset, qiov, type);
 #endif
         }
     }
@@ -1920,24 +1945,40 @@ static int coroutine_fn raw_co_pwritev(BlockDriverState *bs, uint64_t offset,

 static void raw_aio_plug(BlockDriverState *bs)
 {
-#ifdef CONFIG_LINUX_AIO
+#if defined CONFIG_LINUX_AIO || defined CONFIG_LINUX_IO_URING
     BDRVRawState *s = bs->opaque;
+#endif
+#ifdef CONFIG_LINUX_AIO
     if (s->use_linux_aio) {
         LinuxAioState *aio = aio_get_linux_aio(bdrv_get_aio_context(bs));
         laio_io_plug(bs, aio);
     }
 #endif
+#ifdef CONFIG_LINUX_IO_URING
+    if (s->use_linux_io_uring) {
+        LuringState *aio = aio_get_linux_io_uring(bdrv_get_aio_context(bs));
+        luring_io_plug(bs, aio);
+    }
+#endif
 }

 static void raw_aio_unplug(BlockDriverState *bs)
 {
-#ifdef CONFIG_LINUX_AIO
+#if defined CONFIG_LINUX_AIO || defined CONFIG_LINUX_IO_URING
     BDRVRawState *s = bs->opaque;
+#endif
+#ifdef CONFIG_LINUX_AIO
     if (s->use_linux_aio) {
         LinuxAioState *aio = aio_get_linux_aio(bdrv_get_aio_context(bs));
         laio_io_unplug(bs, aio);
     }
 #endif
+#ifdef CONFIG_LINUX_IO_URING
+    if (s->use_linux_aio) {
+        LuringState *aio = aio_get_linux_io_uring(bdrv_get_aio_context(bs));
+        luring_io_unplug(bs, aio);
+    }
+#endif
 }

 static int raw_co_flush_to_disk(BlockDriverState *bs)
@@ -1963,8 +2004,10 @@ static int raw_co_flush_to_disk(BlockDriverState *bs)
 static void raw_aio_attach_aio_context(BlockDriverState *bs,
                                        AioContext *new_context)
 {
+#if defined CONFIG_LINUX_AIO || defined CONFIG_LINUX_IO_URING
+        BDRVRawState *s = bs->opaque;
+#endif
 #ifdef CONFIG_LINUX_AIO
-    BDRVRawState *s = bs->opaque;
     if (s->use_linux_aio) {
         Error *local_err;
         if (!aio_setup_linux_aio(new_context, &local_err)) {
@@ -1974,6 +2017,16 @@ static void raw_aio_attach_aio_context(BlockDriverState *bs,
         }
     }
 #endif
+#ifdef CONFIG_LINUX_IO_URING
+    if (s->use_linux_io_uring) {
+        Error *local_err;
+        if (!aio_setup_linux_io_uring(new_context, &local_err)) {
+            error_reportf_err(local_err, "Unable to use linux io_uring, "
+                                         "falling back to thread pool: ");
+            s->use_linux_io_uring = false;
+        }
+    }
+#endif
 }

 static void raw_close(BlockDriverState *bs)
--
2.17.1

Re: [Qemu-devel] [RFC PATCH v2 8/9] block/file-posix: extends to use with io_uring

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 486 bytes --]

On Fri, May 24, 2019 at 07:33:36PM +0530, Aarushi Mehta wrote:
> @@ -557,7 +559,9 @@ static int raw_open_common(BlockDriverState *bs, QDict *options,
>      s->shared_perm = BLK_PERM_ALL;
> 
>  #ifdef CONFIG_LINUX_AIO
> -     /* Currently Linux does AIO only for files opened with O_DIRECT */
> +     /*
> +      * Currently Linux does AIO only for files opened with O_DIRECT
> +      */
>      if (s->use_linux_aio) {

The comment's indentation looks off.  Is it 4 spaces?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[Qemu-devel] [RFC PATCH v2 9/9] configure: permit use of io_uring

[Aarushi Mehta]

Signed-off-by: Aarushi Mehta <mehta.aaru20@gmail.com>
---
 configure | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/configure b/configure
index 528b9ff705..86383dc0b3 100755
--- a/configure
+++ b/configure
@@ -365,6 +365,7 @@ xen=""
 xen_ctrl_version=""
 xen_pci_passthrough=""
 linux_aio=""
+linux_io_uring=""
 cap_ng=""
 attr=""
 libattr=""
@@ -1255,6 +1256,10 @@ for opt do
   ;;
   --enable-linux-aio) linux_aio="yes"
   ;;
+  --disable-linux-io-uring) linux_io_uring="no"
+  ;;
+  --enable-linux-io-uring) linux_io_uring="yes"
+  ;;
   --disable-attr) attr="no"
   ;;
   --enable-attr) attr="yes"
@@ -1773,6 +1778,7 @@ disabled with --disable-FEATURE, default is enabled if available:
   vde             support for vde network
   netmap          support for netmap network
   linux-aio       Linux AIO support
+  linux-io-uring  Linux io_uring support
   cap-ng          libcap-ng support
   attr            attr and xattr support
   vhost-net       vhost-net kernel acceleration support
@@ -3962,6 +3968,23 @@ EOF
     linux_aio=no
   fi
 fi
+##########################################
+# linux-io-uring probe
+
+if test "$linux_io_uring" != "no" ; then
+  cat > $TMPC <<EOF
+#include <liburing.h>
+int main(void) { io_uring_queue_init(0, NULL, 0); io_uring_submit(NULL); return 0; }
+EOF
+  if compile_prog "" "-luring" ; then
+    linux_io_uring=yes
+  else
+    if test "$linux_io_uring" = "yes" ; then
+      feature_not_found "linux io_uring" "Install liburing"
+    fi
+    linux_io_uring=no
+  fi
+fi

 ##########################################
 # TPM emulation is only on POSIX
@@ -6378,6 +6401,7 @@ echo "PIE               $pie"
 echo "vde support       $vde"
 echo "netmap support    $netmap"
 echo "Linux AIO support $linux_aio"
+echo "Linux io_uring support $linux_io_uring"
 echo "ATTR/XATTR support $attr"
 echo "Install blobs     $blobs"
 echo "KVM support       $kvm"
@@ -6858,6 +6882,9 @@ fi
 if test "$linux_aio" = "yes" ; then
   echo "CONFIG_LINUX_AIO=y" >> $config_host_mak
 fi
+if test "$linux_io_uring" = "yes" ; then
+  echo "CONFIG_LINUX_IO_URING=y" >> $config_host_mak
+fi
 if test "$attr" = "yes" ; then
   echo "CONFIG_ATTR=y" >> $config_host_mak
 fi
--
2.17.1

Re: [Qemu-devel] [RFC PATCH v2 9/9] configure: permit use of io_uring

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 437 bytes --]

On Fri, May 24, 2019 at 07:33:37PM +0530, Aarushi Mehta wrote:
> Signed-off-by: Aarushi Mehta <mehta.aaru20@gmail.com>
> ---
>  configure | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)

I will submit pkg-config support for liburing upstream.  Once this is
available this patch will need to be changed to use $pkg_config liburing
to get cflags and libs.

For now:

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]