Series comparison

-[Qemu-devel] [PULL 0/1] Block patches
+[PULL for-6.1 0/3] Block patches
-The following changes since commit 4100a344eb3d50d88f9da85cae334afc47aee134:
+The following changes since commit 3521ade3510eb5cefb2e27a101667f25dad89935:
-  Merge remote-tracking branch 'remotes/sstabellini/tags/xen-20170202' into staging (2017-02-03 12:31:40 +0000)
+  Merge remote-tracking branch 'remotes/thuth-gitlab/tags/pull-request-2021-07-29' into staging (2021-07-29 13:17:20 +0100)
-are available in the git repository at:
+are available in the Git repository at:
-  git://github.com/stefanha/qemu.git tags/block-pull-request
+  https://gitlab.com/stefanha/qemu.git tags/block-pull-request
-for you to fetch changes up to cdd7abfdba9287a289c404dfdcb02316f9ffee7d:
+for you to fetch changes up to cc8eecd7f105a1dff5876adeb238a14696061a4a:
-  iothread: enable AioContext polling by default (2017-02-03 14:23:38 +0000)
+  MAINTAINERS: Added myself as a reviewer for the NVMe Block Driver (2021-07-29 17:17:34 +0100)
 ----------------------------------------------------------------
 Pull request
 The main fix here is for io_uring. Spurious -EAGAIN errors can happen and the
 request needs to be resubmitted.
 The MAINTAINERS changes carry no risk and we might as well include them in QEMU
 .1.
 ----------------------------------------------------------------
-----------------------------------------------------------------
+Fabian Ebner (1):
   block/io_uring: resubmit when result is -EAGAIN
-Stefan Hajnoczi (1):
+Philippe Mathieu-Daudé (1):
-  iothread: enable AioContext polling by default
+  MAINTAINERS: Added myself as a reviewer for the NVMe Block Driver
- iothread.c | 14 ++++++++++++++
+Stefano Garzarella (1):
-file changed, 14 insertions(+)
+  MAINTAINERS: add Stefano Garzarella as io_uring reviewer
  MAINTAINERS      |  2 ++
  block/io_uring.c | 16 +++++++++++++++-
 files changed, 17 insertions(+), 1 deletion(-)
 --
-.9.3
+.31.1

-New patch
+[PULL for-6.1 1/3] MAINTAINERS: add Stefano Garzarella as io_uring reviewer
+From: Stefano Garzarella <sgarzare@redhat.com>
+I've been working with io_uring for a while so I'd like to help
+with reviews.
+Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
+Message-Id: <20210728131515.131045-1-sgarzare@redhat.com>
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
+---
+ MAINTAINERS | 1 +
+file changed, 1 insertion(+)
+diff --git a/MAINTAINERS b/MAINTAINERS
+index XXXXXXX..XXXXXXX 100644
+--- a/MAINTAINERS
++++ b/MAINTAINERS
+@@ -XXX,XX +XXX,XX @@ Linux io_uring
+ M: Aarushi Mehta <mehta.aaru20@gmail.com>
+ M: Julia Suvorova <jusual@redhat.com>
+ M: Stefan Hajnoczi <stefanha@redhat.com>
++R: Stefano Garzarella <sgarzare@redhat.com>
+ L: qemu-block@nongnu.org
+ S: Maintained
+ F: block/io_uring.c
+--
+.31.1

-New patch
+[PULL for-6.1 2/3] block/io_uring: resubmit when result is -EAGAIN
+From: Fabian Ebner <f.ebner@proxmox.com>
+Linux SCSI can throw spurious -EAGAIN in some corner cases in its
+completion path, which will end up being the result in the completed
+io_uring request.
+Resubmitting such requests should allow block jobs to complete, even
+if such spurious errors are encountered.
+Co-authored-by: Stefan Hajnoczi <stefanha@gmail.com>
+Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
+Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
+Message-id: 20210729091029.65369-1-f.ebner@proxmox.com
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
+---
+ block/io_uring.c | 16 +++++++++++++++-
+file changed, 15 insertions(+), 1 deletion(-)
+diff --git a/block/io_uring.c b/block/io_uring.c
+index XXXXXXX..XXXXXXX 100644
+--- a/block/io_uring.c
++++ b/block/io_uring.c
+@@ -XXX,XX +XXX,XX @@ static void luring_process_completions(LuringState *s)
+         total_bytes = ret + luringcb->total_read;
+         if (ret < 0) {
+-            if (ret == -EINTR) {
++            /*
++             * Only writev/readv/fsync requests on regular files or host block
++             * devices are submitted. Therefore -EAGAIN is not expected but it's
++             * known to happen sometimes with Linux SCSI. Submit again and hope
++             * the request completes successfully.
++             *
++             * For more information, see:
++             * https://lore.kernel.org/io-uring/20210727165811.284510-3-axboe@kernel.dk/T/#u
++             *
++             * If the code is changed to submit other types of requests in the
++             * future, then this workaround may need to be extended to deal with
++             * genuine -EAGAIN results that should not be resubmitted
++             * immediately.
++             */
++            if (ret == -EINTR || ret == -EAGAIN) {
+                 luring_resubmit(s, luringcb);
+                 continue;
+             }
+--
+.31.1

-[Qemu-devel] [PULL 1/1] iothread: enable AioContext polling by default
+[PULL for-6.1 3/3] MAINTAINERS: Added myself as a reviewer for the NVMe Block Driver
-IOThread AioContexts are likely to consist only of event sources like
+From: Philippe Mathieu-Daudé <philmd@redhat.com>
 virtqueue ioeventfds and LinuxAIO completion eventfds that are pollable
 from userspace (without system calls).
-We recently merged the AioContext polling feature but didn't enable it
+I'm interested in following the activity around the NVMe bdrv.
 by default yet.  I have gone back over the performance data on the
 mailing list and picked a default polling value that gave good results.
-Let's enable AioContext polling by default so users don't have another
+Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
-switch they need to set manually.  If performance regressions are found
+Message-id: 20210728183340.2018313-1-philmd@redhat.com
 we can still disable this for the QEMU 2.9 release.
 Cc: Paolo Bonzini <pbonzini@redhat.com>
 Cc: Christian Borntraeger <borntraeger@de.ibm.com>
 Cc: Karl Rister <krister@redhat.com>
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 Message-id: 20170126170119.27876-1-stefanha@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- iothread.c | 14 ++++++++++++++
+ MAINTAINERS | 1 +
-file changed, 14 insertions(+)
+file changed, 1 insertion(+)
-diff --git a/iothread.c b/iothread.c
+diff --git a/MAINTAINERS b/MAINTAINERS
 index XXXXXXX..XXXXXXX 100644
---- a/iothread.c
+--- a/MAINTAINERS
-+++ b/iothread.c
++++ b/MAINTAINERS
-@@ -XXX,XX +XXX,XX @@ typedef ObjectClass IOThreadClass;
+@@ -XXX,XX +XXX,XX @@ F: block/null.c
- #define IOTHREAD_CLASS(klass) \
+ NVMe Block Driver
-    OBJECT_CLASS_CHECK(IOThreadClass, klass, TYPE_IOTHREAD)
+ M: Stefan Hajnoczi <stefanha@redhat.com>
+ R: Fam Zheng <fam@euphon.net>
-+/* Benchmark results from 2016 on NVMe SSD drives show max polling times around
++R: Philippe Mathieu-Daudé <philmd@redhat.com>
-+ * 16-32 microseconds yield IOPS improvements for both iodepth=1 and iodepth=32
+ L: qemu-block@nongnu.org
-+ * workloads.
+ S: Supported
-+ */
+ F: block/nvme*
 +#define IOTHREAD_POLL_MAX_NS_DEFAULT 32768ULL
 +
  static __thread IOThread *my_iothread;
  AioContext *qemu_get_current_aio_context(void)
@@ -XXX,XX +XXX,XX @@ static int iothread_stop(Object *object, void *opaque)
      return 0;
  }
 +static void iothread_instance_init(Object *obj)
 +{
 +    IOThread *iothread = IOTHREAD(obj);
 +
 +    iothread->poll_max_ns = IOTHREAD_POLL_MAX_NS_DEFAULT;
 +}
 +
  static void iothread_instance_finalize(Object *obj)
  {
      IOThread *iothread = IOTHREAD(obj);
@@ -XXX,XX +XXX,XX @@ static const TypeInfo iothread_info = {
      .parent = TYPE_OBJECT,
      .class_init = iothread_class_init,
      .instance_size = sizeof(IOThread),
 +    .instance_init = iothread_instance_init,
      .instance_finalize = iothread_instance_finalize,
      .interfaces = (InterfaceInfo[]) {
          {TYPE_USER_CREATABLE},
 --
-.9.3
+.31.1

IOThread AioContexts are likely to consist only of event sources like
virtqueue ioeventfds and LinuxAIO completion eventfds that are pollable
from userspace (without system calls).

We recently merged the AioContext polling feature but didn't enable it
by default yet.  I have gone back over the performance data on the
mailing list and picked a default polling value that gave good results.

Let's enable AioContext polling by default so users don't have another
switch they need to set manually.  If performance regressions are found
we can still disable this for the QEMU 2.9 release.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: Karl Rister <krister@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 20170126170119.27876-1-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 iothread.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/iothread.c b/iothread.c
index XXXXXXX..XXXXXXX 100644
--- a/iothread.c
+++ b/iothread.c
@@ -XXX,XX +XXX,XX @@ typedef ObjectClass IOThreadClass;
 #define IOTHREAD_CLASS(klass) \
    OBJECT_CLASS_CHECK(IOThreadClass, klass, TYPE_IOTHREAD)
 
+/* Benchmark results from 2016 on NVMe SSD drives show max polling times around
+ * 16-32 microseconds yield IOPS improvements for both iodepth=1 and iodepth=32
+ * workloads.
+ */
+#define IOTHREAD_POLL_MAX_NS_DEFAULT 32768ULL
+
 static __thread IOThread *my_iothread;
 
 AioContext *qemu_get_current_aio_context(void)
@@ -XXX,XX +XXX,XX @@ static int iothread_stop(Object *object, void *opaque)
     return 0;
 }
 
+static void iothread_instance_init(Object *obj)
+{
+    IOThread *iothread = IOTHREAD(obj);
+
+    iothread->poll_max_ns = IOTHREAD_POLL_MAX_NS_DEFAULT;
+}
+
 static void iothread_instance_finalize(Object *obj)
 {
     IOThread *iothread = IOTHREAD(obj);
@@ -XXX,XX +XXX,XX @@ static const TypeInfo iothread_info = {
     .parent = TYPE_OBJECT,
     .class_init = iothread_class_init,
     .instance_size = sizeof(IOThread),
+    .instance_init = iothread_instance_init,
     .instance_finalize = iothread_instance_finalize,
     .interfaces = (InterfaceInfo[]) {
         {TYPE_USER_CREATABLE},
-- 
2.9.3

The following changes since commit 3521ade3510eb5cefb2e27a101667f25dad89935:

Merge remote-tracking branch 'remotes/thuth-gitlab/tags/pull-request-2021-07-29' into staging (2021-07-29 13:17:20 +0100)

are available in the Git repository at:

https://gitlab.com/stefanha/qemu.git tags/block-pull-request

for you to fetch changes up to cc8eecd7f105a1dff5876adeb238a14696061a4a:

MAINTAINERS: Added myself as a reviewer for the NVMe Block Driver (2021-07-29 17:17:34 +0100)

----------------------------------------------------------------
Pull request

The main fix here is for io_uring. Spurious -EAGAIN errors can happen and the
request needs to be resubmitted.

The MAINTAINERS changes carry no risk and we might as well include them in QEMU
6.1.

----------------------------------------------------------------

Fabian Ebner (1):
  block/io_uring: resubmit when result is -EAGAIN

Philippe Mathieu-Daudé (1):
  MAINTAINERS: Added myself as a reviewer for the NVMe Block Driver

Stefano Garzarella (1):
  MAINTAINERS: add Stefano Garzarella as io_uring reviewer

MAINTAINERS      |  2 ++
 block/io_uring.c | 16 +++++++++++++++-
 2 files changed, 17 insertions(+), 1 deletion(-)

-- 
2.31.1

From: Fabian Ebner <f.ebner@proxmox.com>

Linux SCSI can throw spurious -EAGAIN in some corner cases in its
completion path, which will end up being the result in the completed
io_uring request.

Resubmitting such requests should allow block jobs to complete, even
if such spurious errors are encountered.

Co-authored-by: Stefan Hajnoczi <stefanha@gmail.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
Message-id: 20210729091029.65369-1-f.ebner@proxmox.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/io_uring.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/block/io_uring.c b/block/io_uring.c
index XXXXXXX..XXXXXXX 100644
--- a/block/io_uring.c
+++ b/block/io_uring.c
@@ -XXX,XX +XXX,XX @@ static void luring_process_completions(LuringState *s)
         total_bytes = ret + luringcb->total_read;
 
         if (ret < 0) {
-            if (ret == -EINTR) {
+            /*
+             * Only writev/readv/fsync requests on regular files or host block
+             * devices are submitted. Therefore -EAGAIN is not expected but it's
+             * known to happen sometimes with Linux SCSI. Submit again and hope
+             * the request completes successfully.
+             *
+             * For more information, see:
+             * https://lore.kernel.org/io-uring/20210727165811.284510-3-axboe@kernel.dk/T/#u
+             *
+             * If the code is changed to submit other types of requests in the
+             * future, then this workaround may need to be extended to deal with
+             * genuine -EAGAIN results that should not be resubmitted
+             * immediately.
+             */
+            if (ret == -EINTR || ret == -EAGAIN) {
                 luring_resubmit(s, luringcb);
                 continue;
             }
-- 
2.31.1