From: "Michael S. Tsirkin" <mst@redhat.com>
To: qemu-devel@nongnu.org, stefanha@redhat.com,
edivaldoapereira@yahoo.com.br
Cc: Amit Shah <amit.shah@redhat.com>, Anthony Liguori <aliguori@us.ibm.com>
Subject: [Qemu-devel] [PATCHv2] virtio: limit avail bytes lookahead
Date: Fri, 30 Nov 2012 00:02:56 +0200 [thread overview]
Message-ID: <20121129220256.GA14799@redhat.com> (raw)
Commit 0d8d7690850eb0cf2b2b60933cf47669a6b6f18f introduced
a regression in virtio-net performance because it looks
into the ring aggressively while we really only care
about a single packet worth of buffers.
Reported as bugzilla 1066055 in launchpad.
To fix, add parameters limiting lookahead, and
use in virtqueue_avail_bytes.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reported-by: Edivaldo de Araujo Pereira <edivaldoapereira@yahoo.com.br>
Tested-by: Edivaldo de Araujo Pereira <edivaldoapereira@yahoo.com.br>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
Anthony says he wants to test this, I only compiled this patch.
hw/virtio-rng.c | 12 +++++++++---
hw/virtio-serial-bus.c | 2 +-
hw/virtio.c | 15 ++++++++-------
hw/virtio.h | 3 ++-
4 files changed, 20 insertions(+), 12 deletions(-)
diff --git a/hw/virtio-rng.c b/hw/virtio-rng.c
index df329f2..a73ef8e 100644
--- a/hw/virtio-rng.c
+++ b/hw/virtio-rng.c
@@ -43,11 +43,11 @@ static bool is_guest_ready(VirtIORNG *vrng)
return false;
}
-static size_t get_request_size(VirtQueue *vq)
+static size_t get_request_size(VirtQueue *vq, unsigned quota)
{
unsigned int in, out;
- virtqueue_get_avail_bytes(vq, &in, &out);
+ virtqueue_get_avail_bytes(vq, &in, &out, quota, 0);
return in;
}
@@ -84,12 +84,18 @@ static void chr_read(void *opaque, const void *buf, size_t size)
static void virtio_rng_process(VirtIORNG *vrng)
{
size_t size;
+ unsigned quota;
if (!is_guest_ready(vrng)) {
return;
}
- size = get_request_size(vrng->vq);
+ if (vrng->quota_remaining < 0) {
+ quota = 0;
+ } else {
+ quota = MIN((uint64_t)vrng->quota_remaining, (uint64_t)UINT32_MAX);
+ }
+ size = get_request_size(vrng->vq, quota);
size = MIN(vrng->quota_remaining, size);
if (size) {
rng_backend_request_entropy(vrng->rng, size, chr_read, vrng);
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index efa8a81..155da58 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -306,7 +306,7 @@ size_t virtio_serial_guest_ready(VirtIOSerialPort *port)
if (use_multiport(port->vser) && !port->guest_connected) {
return 0;
}
- virtqueue_get_avail_bytes(vq, &bytes, NULL);
+ virtqueue_get_avail_bytes(vq, &bytes, NULL, 4096, 0);
return bytes;
}
diff --git a/hw/virtio.c b/hw/virtio.c
index ec8b7d8..f40a8c5 100644
--- a/hw/virtio.c
+++ b/hw/virtio.c
@@ -336,7 +336,8 @@ static unsigned virtqueue_next_desc(hwaddr desc_pa,
}
void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned int *in_bytes,
- unsigned int *out_bytes)
+ unsigned int *out_bytes,
+ unsigned max_in_bytes, unsigned max_out_bytes)
{
unsigned int idx;
unsigned int total_bufs, in_total, out_total;
@@ -385,6 +386,9 @@ void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned int *in_bytes,
} else {
out_total += vring_desc_len(desc_pa, i);
}
+ if (in_total >= max_in_bytes && out_total >= max_out_bytes) {
+ goto done;
+ }
} while ((i = virtqueue_next_desc(desc_pa, i, max)) != max);
if (!indirect)
@@ -392,6 +396,7 @@ void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned int *in_bytes,
else
total_bufs++;
}
+done:
if (in_bytes) {
*in_bytes = in_total;
}
@@ -405,12 +410,8 @@ int virtqueue_avail_bytes(VirtQueue *vq, unsigned int in_bytes,
{
unsigned int in_total, out_total;
- virtqueue_get_avail_bytes(vq, &in_total, &out_total);
- if ((in_bytes && in_bytes < in_total)
- || (out_bytes && out_bytes < out_total)) {
- return 1;
- }
- return 0;
+ virtqueue_get_avail_bytes(vq, &in_total, &out_total, in_bytes, out_bytes);
+ return in_bytes <= in_total && out_bytes <= out_total;
}
void virtqueue_map_sg(struct iovec *sg, hwaddr *addr,
diff --git a/hw/virtio.h b/hw/virtio.h
index df8d0f7..7c17f7b 100644
--- a/hw/virtio.h
+++ b/hw/virtio.h
@@ -150,7 +150,8 @@ int virtqueue_pop(VirtQueue *vq, VirtQueueElement *elem);
int virtqueue_avail_bytes(VirtQueue *vq, unsigned int in_bytes,
unsigned int out_bytes);
void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned int *in_bytes,
- unsigned int *out_bytes);
+ unsigned int *out_bytes,
+ unsigned max_in_bytes, unsigned max_out_bytes);
void virtio_notify(VirtIODevice *vdev, VirtQueue *vq);
--
MST
next reply other threads:[~2012-11-29 22:01 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-11-29 22:02 Michael S. Tsirkin [this message]
2012-11-30 16:14 ` [Qemu-devel] [PATCHv2] virtio: limit avail bytes lookahead Anthony Liguori
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=20121129220256.GA14799@redhat.com \
--to=mst@redhat.com \
--cc=aliguori@us.ibm.com \
--cc=amit.shah@redhat.com \
--cc=edivaldoapereira@yahoo.com.br \
--cc=qemu-devel@nongnu.org \
--cc=stefanha@redhat.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.