[Qemu-devel] [RFC v9 19/27] virtio-blk: Disable guest->host notifies while processing vring

[Stefan Hajnoczi]

---
 hw/dataplane/vring.h |   28 +++++++++++++++++++++++-----
 hw/virtio-blk.c      |   47 +++++++++++++++++++++++++++++++++++------------
 2 files changed, 58 insertions(+), 17 deletions(-)

diff --git a/hw/dataplane/vring.h b/hw/dataplane/vring.h
index 44ef4a9..cdd4d4a 100644
--- a/hw/dataplane/vring.h
+++ b/hw/dataplane/vring.h
@@ -69,11 +69,29 @@ static void vring_setup(Vring *vring, VirtIODevice *vdev, 
int n)
             vring->vr.desc, vring->vr.avail, vring->vr.used);
 }
 
+/* Are there more descriptors available? */
 static bool vring_more_avail(Vring *vring)
 {
        return vring->vr.avail->idx != vring->last_avail_idx;
 }
 
+/* Hint to disable guest->host notifies */
+static void vring_disable_cb(Vring *vring)
+{
+    vring->vr.used->flags |= VRING_USED_F_NO_NOTIFY;
+}
+
+/* Re-enable guest->host notifies
+ *
+ * Returns false if there are more descriptors in the ring.
+ */
+static bool vring_enable_cb(Vring *vring)
+{
+    vring->vr.used->flags &= ~VRING_USED_F_NO_NOTIFY;
+    __sync_synchronize(); /* mb() */
+    return !vring_more_avail(vring);
+}
+
 /* This is stolen from linux-2.6/drivers/vhost/vhost.c. */
 static bool get_indirect(Vring *vring,
                        struct iovec iov[], struct iovec *iov_end,
@@ -160,7 +178,7 @@ static bool get_indirect(Vring *vring,
  *
  * Stolen from linux-2.6/drivers/vhost/vhost.c.
  */
-static unsigned int vring_pop(Vring *vring,
+static int vring_pop(Vring *vring,
                      struct iovec iov[], struct iovec *iov_end,
                      unsigned int *out_num, unsigned int *in_num)
 {
@@ -178,9 +196,9 @@ static unsigned int vring_pop(Vring *vring,
                exit(1);
        }
 
-       /* If there's nothing new since last we looked, return invalid. */
+       /* If there's nothing new since last we looked. */
        if (avail_idx == last_avail_idx)
-               return num;
+               return -EAGAIN;
 
        /* Only get avail ring entries after they have been exposed by guest. */
        __sync_synchronize(); /* smp_rmb() */
@@ -215,7 +233,7 @@ static unsigned int vring_pop(Vring *vring,
         desc = vring->vr.desc[i];
                if (desc.flags & VRING_DESC_F_INDIRECT) {
                        if (!get_indirect(vring, iov, iov_end, out_num, in_num, 
&desc)) {
-                return num; /* not enough iovecs, stop for now */
+                return -ENOBUFS; /* not enough iovecs, stop for now */
             }
             continue;
                }
@@ -225,7 +243,7 @@ static unsigned int vring_pop(Vring *vring,
          * with the current set.
          */
         if (iov >= iov_end) {
-            return num;
+            return -ENOBUFS;
         }
 
         iov->iov_base = phys_to_host(vring, desc.addr);
diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index efeffa0..f67fdb7 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -202,7 +202,8 @@ static bool handle_notify(EventHandler *handler)
      * accept more I/O.  This is not implemented yet.
      */
     struct iovec iovec[VRING_MAX];
-    struct iovec *iov, *end = &iovec[VRING_MAX];
+    struct iovec *end = &iovec[VRING_MAX];
+    struct iovec *iov = iovec;
 
     /* When a request is read from the vring, the index of the first descriptor
      * (aka head) is returned so that the completed request can be pushed onto
@@ -211,19 +212,41 @@ static bool handle_notify(EventHandler *handler)
      * The number of hypervisor read-only iovecs is out_num.  The number of
      * hypervisor write-only iovecs is in_num.
      */
-    unsigned int head, out_num = 0, in_num = 0;
+    int head;
+    unsigned int out_num = 0, in_num = 0;
 
-    for (iov = iovec; ; iov += out_num + in_num) {
-        head = vring_pop(&s->vring, iov, end, &out_num, &in_num);
-        if (head >= vring_get_num(&s->vring)) {
-            break; /* no more requests */
-        }
+    for (;;) {
+        /* Disable guest->host notifies to avoid unnecessary vmexits */
+        vring_disable_cb(&s->vring);
+
+        for (;;) {
+            head = vring_pop(&s->vring, iov, end, &out_num, &in_num);
+            if (head < 0) {
+                break; /* no more requests */
+            }
 
-        /*
-        fprintf(stderr, "out_num=%u in_num=%u head=%u\n", out_num, in_num, 
head);
-        */
+            /*
+            fprintf(stderr, "out_num=%u in_num=%u head=%d\n", out_num, in_num, 
head);
+            */
 
-        process_request(&s->ioqueue, iov, out_num, in_num, head);
+            process_request(&s->ioqueue, iov, out_num, in_num, head);
+            iov += out_num + in_num;
+        }
+
+        if (likely(head == -EAGAIN)) { /* vring emptied */
+            /* Re-enable guest->host notifies and stop processing the vring.
+             * But if the guest has snuck in more descriptors, keep processing.
+             */
+            if (likely(vring_enable_cb(&s->vring))) {
+                break;
+            }
+        } else { /* head == -ENOBUFS, cannot continue since iovecs[] is 
depleted */
+            /* Since there are no iovecs[] left, stop processing for now.  Do
+             * not re-enable guest->host notifies since the I/O completion
+             * handler knows to check for more vring descriptors anyway.
+             */
+            break;
+        }
     }
 
     /* Submit requests, if any */
@@ -247,7 +270,7 @@ static bool handle_io(EventHandler *handler)
      * so check again.  There should now be enough resources to process more
      * requests.
      */
-    if (vring_more_avail(&s->vring)) {
+    if (unlikely(vring_more_avail(&s->vring))) {
         return handle_notify(&s->notify_handler);
     }
 
-- 
1.7.10.4