All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: qemu-devel@nongnu.org
Cc: cornelia.huck@de.ibm.com,
	Vincenzo Maffione <v.maffione@gmail.com>,
	mst@redhat.com
Subject: [Qemu-devel] [PATCH 09/10] virtio: read avail_idx from VQ only when necessary
Date: Sun, 31 Jan 2016 11:29:05 +0100	[thread overview]
Message-ID: <1454236146-23293-10-git-send-email-pbonzini@redhat.com> (raw)
In-Reply-To: <1454236146-23293-1-git-send-email-pbonzini@redhat.com>

From: Vincenzo Maffione <v.maffione@gmail.com>

The virtqueue_pop() implementation needs to check if the avail ring
contains some pending buffers. To perform this check, it is not
always necessary to fetch the avail_idx in the VQ memory, which is
expensive. This patch introduces a shadow variable tracking avail_idx
and modifies virtio_queue_empty() to access avail_idx in physical
memory only when necessary.

Signed-off-by: Vincenzo Maffione <v.maffione@gmail.com>
Message-Id: <b617d6459902773d9f4ab843bfaca764f5af8eda.1450218353.git.v.maffione@gmail.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
        v1->v2: update shadow avail_idx in virtio_queue_set_last_avail_idx
                [Michael]

 hw/virtio/virtio.c | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 5116a2e..6842938 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -70,8 +70,13 @@ typedef struct VRing
 struct VirtQueue
 {
     VRing vring;
+
+    /* Next head to pop */
     uint16_t last_avail_idx;
 
+    /* Last avail_idx read from VQ. */
+    uint16_t shadow_avail_idx;
+
     uint16_t used_idx;
 
     /* Last used index value we have signalled on */
@@ -132,7 +137,8 @@ static inline uint16_t vring_avail_idx(VirtQueue *vq)
 {
     hwaddr pa;
     pa = vq->vring.avail + offsetof(VRingAvail, idx);
-    return virtio_lduw_phys(vq->vdev, pa);
+    vq->shadow_avail_idx = virtio_lduw_phys(vq->vdev, pa);
+    return vq->shadow_avail_idx;
 }
 
 static inline uint16_t vring_avail_ring(VirtQueue *vq, int i)
@@ -223,8 +229,14 @@ int virtio_queue_ready(VirtQueue *vq)
     return vq->vring.avail != 0;
 }
 
+/* Fetch avail_idx from VQ memory only when we really need to know if
+ * guest has added some buffers. */
 int virtio_queue_empty(VirtQueue *vq)
 {
+    if (vq->shadow_avail_idx != vq->last_avail_idx) {
+        return 0;
+    }
+
     return vring_avail_idx(vq) == vq->last_avail_idx;
 }
 
@@ -300,7 +312,7 @@ static int virtqueue_num_heads(VirtQueue *vq, unsigned int idx)
     /* Check it isn't doing very strange things with descriptor numbers. */
     if (num_heads > vq->vring.num) {
         error_report("Guest moved used index from %u to %u",
-                     idx, vring_avail_idx(vq));
+                     idx, vq->shadow_avail_idx);
         exit(1);
     }
     /* On success, callers read a descriptor at vq->last_avail_idx.
@@ -535,9 +547,12 @@ void *virtqueue_pop(VirtQueue *vq, size_t sz)
     struct iovec iov[VIRTQUEUE_MAX_SIZE];
     VRingDesc desc;
 
-    if (!virtqueue_num_heads(vq, vq->last_avail_idx)) {
+    if (virtio_queue_empty(vq)) {
         return NULL;
     }
+    /* Needed after virtio_queue_empty(), see comment in
+     * virtqueue_num_heads(). */
+    smp_rmb();
 
     /* When we start there are none of either input nor output. */
     out_num = in_num = 0;
@@ -786,6 +801,7 @@ void virtio_reset(void *opaque)
         vdev->vq[i].vring.avail = 0;
         vdev->vq[i].vring.used = 0;
         vdev->vq[i].last_avail_idx = 0;
+        vdev->vq[i].shadow_avail_idx = 0;
         vdev->vq[i].used_idx = 0;
         virtio_queue_set_vector(vdev, i, VIRTIO_NO_VECTOR);
         vdev->vq[i].signalled_used = 0;
@@ -1155,7 +1171,7 @@ static bool vring_notify(VirtIODevice *vdev, VirtQueue *vq)
     smp_mb();
     /* Always notify when queue is empty (when feature acknowledge) */
     if (virtio_vdev_has_feature(vdev, VIRTIO_F_NOTIFY_ON_EMPTY) &&
-        !vq->inuse && vring_avail_idx(vq) == vq->last_avail_idx) {
+        !vq->inuse && virtio_queue_empty(vq)) {
         return true;
     }
 
@@ -1579,6 +1595,7 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id)
                 return -1;
             }
             vdev->vq[i].used_idx = vring_used_idx(&vdev->vq[i]);
+            vdev->vq[i].shadow_avail_idx = vring_avail_idx(&vdev->vq[i]);
         }
     }
 
@@ -1714,6 +1731,7 @@ uint16_t virtio_queue_get_last_avail_idx(VirtIODevice *vdev, int n)
 void virtio_queue_set_last_avail_idx(VirtIODevice *vdev, int n, uint16_t idx)
 {
     vdev->vq[n].last_avail_idx = idx;
+    vdev->vq[n].shadow_avail_idx = idx;
 }
 
 void virtio_queue_invalidate_signalled_used(VirtIODevice *vdev, int n)
-- 
2.5.0

  parent reply	other threads:[~2016-01-31 10:29 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-31 10:28 [Qemu-devel] [PATCH v2 00/10] virtio/vring: optimization patches Paolo Bonzini
2016-01-31 10:28 ` [Qemu-devel] [PATCH 01/10] virtio: move VirtQueueElement at the beginning of the structs Paolo Bonzini
2016-02-01 11:17   ` Cornelia Huck
2016-01-31 10:28 ` [Qemu-devel] [PATCH 02/10] virtio: move allocation to virtqueue_pop/vring_pop Paolo Bonzini
2016-02-01 11:20   ` Cornelia Huck
2016-01-31 10:28 ` [Qemu-devel] [PATCH 03/10] virtio: introduce qemu_get/put_virtqueue_element Paolo Bonzini
2016-01-31 10:29 ` [Qemu-devel] [PATCH 04/10] virtio: introduce virtqueue_alloc_element Paolo Bonzini
2016-01-31 10:29 ` [Qemu-devel] [PATCH 05/10] virtio: slim down allocation of VirtQueueElements Paolo Bonzini
2016-01-31 10:29 ` [Qemu-devel] [PATCH 06/10] vring: " Paolo Bonzini
2016-01-31 10:29 ` [Qemu-devel] [PATCH 07/10] virtio: combine the read of a descriptor Paolo Bonzini
2016-02-03 12:34   ` Gonglei (Arei)
2016-02-03 13:40     ` Paolo Bonzini
2016-02-04  7:48       ` Gonglei (Arei)
2016-02-04 10:18         ` Paolo Bonzini
2016-02-05  6:16           ` Gonglei (Arei)
2016-01-31 10:29 ` [Qemu-devel] [PATCH 08/10] virtio: cache used_idx in a VirtQueue field Paolo Bonzini
2016-01-31 10:29 ` Paolo Bonzini [this message]
2016-01-31 10:29 ` [Qemu-devel] [PATCH 10/10] virtio: combine write of an entry into used ring Paolo Bonzini
2016-02-03 12:08 ` [Qemu-devel] [PATCH v2 00/10] virtio/vring: optimization patches Gonglei (Arei)
2016-02-04 10:19   ` Paolo Bonzini
2016-02-05  7:17     ` Gonglei (Arei)
2016-02-03 12:38 ` Gonglei (Arei)
  -- strict thread matches above, loose matches on Subject: below --
2016-01-15 12:41 [Qemu-devel] [PATCH " Paolo Bonzini
2016-01-15 12:41 ` [Qemu-devel] [PATCH 09/10] virtio: read avail_idx from VQ only when necessary Paolo Bonzini
2016-01-19 16:20   ` Cornelia Huck
2016-01-19 16:54   ` Michael S. Tsirkin
2016-01-19 18:48     ` Paolo Bonzini
2016-01-20 17:32       ` Cornelia Huck
2016-01-21 21:40       ` Vincenzo Maffione

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1454236146-23293-10-git-send-email-pbonzini@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=cornelia.huck@de.ibm.com \
    --cc=mst@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=v.maffione@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.