[PULL,v2,10/45] vring: slim down allocation of VirtQueueElements

Commit Message

Michael S. Tsirkin Feb. 6, 2016, 7:12 p.m. UTC

From: Paolo Bonzini <[e-mail]>

Build the addresses and s/g lists on the stack, and then copy them
to a VirtQueueElement that is just as big as required to contain this
particular s/g list.  The cost of the copy is minimal compared to that
of a large malloc.

Reviewed-by: Cornelia Huck <[e-mail]>
Signed-off-by: Paolo Bonzini <[e-mail]>
Reviewed-by: Michael S. Tsirkin <[e-mail]>
Signed-off-by: Michael S. Tsirkin <[e-mail]>
---
 hw/virtio/dataplane/vring.c | 53 ++++++++++++++++++++++++++++++---------------
 1 file changed, 36 insertions(+), 17 deletions(-)

Patch

diff --git a/hw/virtio/dataplane/vring.c b/hw/virtio/dataplane/vring.c
index 57ada3b..4308d9f 100644
--- a/hw/virtio/dataplane/vring.c
+++ b/hw/virtio/dataplane/vring.c
@@ -218,8 +218,14 @@  bool vring_should_notify(VirtIODevice *vdev, Vring *vring)
                             new, old);
 }
 
-
-static int get_desc(Vring *vring, VirtQueueElement *elem,
+typedef struct VirtQueueCurrentElement {
+    unsigned in_num;
+    unsigned out_num;
+    hwaddr addr[VIRTQUEUE_MAX_SIZE];
+    struct iovec iov[VIRTQUEUE_MAX_SIZE];
+} VirtQueueCurrentElement;
+
+static int get_desc(Vring *vring, VirtQueueCurrentElement *elem,
                     struct vring_desc *desc)
 {
     unsigned *num;
@@ -230,12 +236,12 @@  static int get_desc(Vring *vring, VirtQueueElement *elem,
 
     if (desc->flags & VRING_DESC_F_WRITE) {
         num = &elem->in_num;
-        iov = &elem->in_sg[*num];
-        addr = &elem->in_addr[*num];
+        iov = &elem->iov[elem->out_num + *num];
+        addr = &elem->addr[elem->out_num + *num];
     } else {
         num = &elem->out_num;
-        iov = &elem->out_sg[*num];
-        addr = &elem->out_addr[*num];
+        iov = &elem->iov[*num];
+        addr = &elem->addr[*num];
 
         /* If it's an output descriptor, they're all supposed
          * to come before any input descriptors. */
@@ -299,7 +305,8 @@  static bool read_vring_desc(VirtIODevice *vdev,
 
 /* This is stolen from linux/drivers/vhost/vhost.c. */
 static int get_indirect(VirtIODevice *vdev, Vring *vring,
-                        VirtQueueElement *elem, struct vring_desc *indirect)
+                        VirtQueueCurrentElement *cur_elem,
+                        struct vring_desc *indirect)
 {
     struct vring_desc desc;
     unsigned int i = 0, count, found = 0;
@@ -351,7 +358,7 @@  static int get_indirect(VirtIODevice *vdev, Vring *vring,
             return -EFAULT;
         }
 
-        ret = get_desc(vring, elem, &desc);
+        ret = get_desc(vring, cur_elem, &desc);
         if (ret < 0) {
             vring->broken |= (ret == -EFAULT);
             return ret;
@@ -394,6 +401,7 @@  void *vring_pop(VirtIODevice *vdev, Vring *vring, size_t sz)
     struct vring_desc desc;
     unsigned int i, head, found = 0, num = vring->vr.num;
     uint16_t avail_idx, last_avail_idx;
+    VirtQueueCurrentElement cur_elem;
     VirtQueueElement *elem = NULL;
     int ret;
 
@@ -403,10 +411,7 @@  void *vring_pop(VirtIODevice *vdev, Vring *vring, size_t sz)
         goto out;
     }
 
-    elem = virtqueue_alloc_element(sz, VIRTQUEUE_MAX_SIZE, VIRTQUEUE_MAX_SIZE);
-
-    /* Initialize elem so it can be safely unmapped */
-    elem->in_num = elem->out_num = 0;
+    cur_elem.in_num = cur_elem.out_num = 0;
 
     /* Check it isn't doing very strange things with descriptor numbers. */
     last_avail_idx = vring->last_avail_idx;
@@ -433,8 +438,6 @@  void *vring_pop(VirtIODevice *vdev, Vring *vring, size_t sz)
      * the index we've seen. */
     head = vring_get_avail_ring(vdev, vring, last_avail_idx % num);
 
-    elem->index = head;
-
     /* If their number is silly, that's an error. */
     if (unlikely(head >= num)) {
         error_report("Guest says index %u > %u is available", head, num);
@@ -461,14 +464,14 @@  void *vring_pop(VirtIODevice *vdev, Vring *vring, size_t sz)
         barrier();
 
         if (desc.flags & VRING_DESC_F_INDIRECT) {
-            ret = get_indirect(vdev, vring, elem, &desc);
+            ret = get_indirect(vdev, vring, &cur_elem, &desc);
             if (ret < 0) {
                 goto out;
             }
             continue;
         }
 
-        ret = get_desc(vring, elem, &desc);
+        ret = get_desc(vring, &cur_elem, &desc);
         if (ret < 0) {
             goto out;
         }
@@ -483,6 +486,18 @@  void *vring_pop(VirtIODevice *vdev, Vring *vring, size_t sz)
             virtio_tswap16(vdev, vring->last_avail_idx);
     }
 
+    /* Now copy what we have collected and mapped */
+    elem = virtqueue_alloc_element(sz, cur_elem.out_num, cur_elem.in_num);
+    elem->index = head;
+    for (i = 0; i < cur_elem.out_num; i++) {
+        elem->out_addr[i] = cur_elem.addr[i];
+        elem->out_sg[i] = cur_elem.iov[i];
+    }
+    for (i = 0; i < cur_elem.in_num; i++) {
+        elem->in_addr[i] = cur_elem.addr[cur_elem.out_num + i];
+        elem->in_sg[i] = cur_elem.iov[cur_elem.out_num + i];
+    }
+
     return elem;
 
 out:
@@ -490,7 +505,11 @@  out:
     if (ret == -EFAULT) {
         vring->broken = true;
     }
-    vring_unmap_element(elem);
+
+    for (i = 0; i < cur_elem.out_num + cur_elem.in_num; i++) {
+        vring_unmap(cur_elem.iov[i].iov_base, false);
+    }
+
     g_free(elem);
     return NULL;
 }