From: Rusty Russell <rusty@rustcorp.com.au>
To: Sasha Levin <levinsasha928@gmail.com>,
"Michael S. Tsirkin" <mst@redhat.com>
Cc: avi@redhat.com, linux-kernel@vger.kernel.org,
kvm@vger.kernel.org, virtualization@lists.linux-foundation.org
Subject: Re: [PATCH v2 2/2] virtio-ring: Allocate indirect buffers from cache when possible
Date: Thu, 06 Sep 2012 09:32:14 +0930 [thread overview]
Message-ID: <874nncj9x5.fsf__10496.4864826345$1346900525$gmane$org@rustcorp.com.au> (raw)
In-Reply-To: <503CC904.3050207@gmail.com>
Sasha Levin <levinsasha928@gmail.com> writes:
> On 08/28/2012 03:20 PM, Michael S. Tsirkin wrote:
>> On Tue, Aug 28, 2012 at 03:04:03PM +0200, Sasha Levin wrote:
>>> Currently if VIRTIO_RING_F_INDIRECT_DESC is enabled we will
>>> use indirect descriptors and allocate them using a simple
>>> kmalloc().
>>>
>>> This patch adds a cache which will allow indirect buffers under
>>> a configurable size to be allocated from that cache instead.
>>>
>>> Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
>>
>> I imagine this helps performance? Any numbers?
>
> I ran benchmarks on the original RFC, I've re-tested it now and got similar
> numbers to the original ones (virtio-net using vhost-net, thresh=16):
>
> Before:
> Recv Send Send
> Socket Socket Message Elapsed
> Size Size Size Time Throughput
> bytes bytes bytes secs. 10^6bits/sec
>
> 87380 16384 16384 10.00 4512.12
>
> After:
> Recv Send Send
> Socket Socket Message Elapsed
> Size Size Size Time Throughput
> bytes bytes bytes secs. 10^6bits/sec
>
> 87380 16384 16384 10.00 5399.18
I have an older patch which adjusts the threshold dynamically, can you
compare? User-adjustable thresholds are statistically never adjusted :(
virtio: use indirect buffers based on demand (heuristic)
virtio_ring uses a ring buffer of descriptors: indirect support allows
a single descriptor to refer to a table of descriptors. This saves
space in the ring, but requires a kmalloc/kfree.
Rather than try to figure out what the right threshold at which to use
indirect buffers, we drop the threshold dynamically when the ring is
under stress.
Note: to stress this, I reduced the ring size to 32 in lguest, and a
1G send reduced the threshold to 9.
Note2: I moved the BUG_ON()s above the indirect test, where they belong
(indirect falls thru on OOM, so the constraints still apply).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
---
drivers/virtio/virtio_ring.c | 61 ++++++++++++++++++++++++++++++++++++-------
1 file changed, 52 insertions(+), 9 deletions(-)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -89,6 +89,8 @@ struct vring_virtqueue
/* Host supports indirect buffers */
bool indirect;
+ /* Threshold before we go indirect. */
+ unsigned int indirect_threshold;
/* Host publishes avail event idx */
bool event;
@@ -174,6 +176,34 @@ static int vring_add_indirect(struct vri
return head;
}
+static void adjust_threshold(struct vring_virtqueue *vq,
+ unsigned int out, unsigned int in)
+{
+ /* There are really two species of virtqueue, and it matters here.
+ * If there are no output parts, it's a "normally full" receive queue,
+ * otherwise it's a "normally empty" send queue. */
+ if (out) {
+ /* Leave threshold unless we're full. */
+ if (out + in < vq->num_free)
+ return;
+ } else {
+ /* Leave threshold unless we're empty. */
+ if (vq->num_free != vq->vring.num)
+ return;
+ }
+
+ /* Never drop threshold below 1 */
+ vq->indirect_threshold /= 2;
+ vq->indirect_threshold |= 1;
+
+#if 0
+ printk("%s %s: indirect threshold %u (%u+%u vs %u)\n",
+ dev_name(&vq->vq.vdev->dev),
+ vq->vq.name, vq->indirect_threshold,
+ out, in, vq->num_free);
+#endif
+}
+
int virtqueue_get_queue_index(struct virtqueue *_vq)
{
struct vring_virtqueue *vq = to_vvq(_vq);
@@ -226,17 +256,32 @@ int virtqueue_add_buf(struct virtqueue *
}
#endif
- /* If the host supports indirect descriptor tables, and we have multiple
- * buffers, then go indirect. FIXME: tune this threshold */
- if (vq->indirect && (out + in) > 1 && vq->num_free) {
- head = vring_add_indirect(vq, sg, out, in, gfp);
- if (likely(head >= 0))
- goto add_head;
- }
-
BUG_ON(out + in > vq->vring.num);
BUG_ON(out + in == 0);
+
+ /* If the host supports indirect descriptor tables, consider it. */
+ if (vq->indirect) {
+ bool try_indirect;
+
+ /* We tweak the threshold automatically. */
+ adjust_threshold(vq, out, in);
+
+ /* If we can't fit any at all, fall through. */
+ if (vq->num_free == 0)
+ try_indirect = false;
+ else if (out + in > vq->num_free)
+ try_indirect = true;
+ else
+ try_indirect = (out + in > vq->indirect_threshold);
+
+ if (try_indirect) {
+ head = vring_add_indirect(vq, sg, out, in);
+ if (head != vq->vring.num)
+ goto add_head;
+ }
+ }
+
if (vq->num_free < out + in) {
pr_debug("Can't add buf len %i - avail = %i\n",
out + in, vq->num_free);
@@ -666,6 +711,7 @@ struct virtqueue *vring_new_virtqueue(un
#endif
vq->indirect = virtio_has_feature(vdev, VIRTIO_RING_F_INDIRECT_DESC);
+ vq->indirect_threshold = num;
vq->event = virtio_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX);
/* No callback? Tell other side not to bother us. */
next prev parent reply other threads:[~2012-09-06 0:02 UTC|newest]
Thread overview: 59+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-08-28 13:04 [PATCH v2 1/2] virtio-ring: Use threshold for switching to indirect descriptors Sasha Levin
2012-08-28 13:04 ` Sasha Levin
2012-08-28 13:04 ` [PATCH v2 2/2] virtio-ring: Allocate indirect buffers from cache when possible Sasha Levin
2012-08-28 13:04 ` Sasha Levin
2012-08-28 13:20 ` Michael S. Tsirkin
2012-08-28 13:20 ` Michael S. Tsirkin
2012-08-28 13:35 ` Sasha Levin
2012-08-28 13:35 ` Sasha Levin
2012-08-29 11:07 ` Michael S. Tsirkin
2012-08-29 11:07 ` Michael S. Tsirkin
2012-08-29 15:03 ` Sasha Levin
2012-08-29 15:03 ` Sasha Levin
2012-08-29 15:14 ` Michael S. Tsirkin
2012-08-29 15:14 ` Michael S. Tsirkin
2012-08-30 10:34 ` Sasha Levin
2012-08-30 10:34 ` Sasha Levin
2012-08-29 15:38 ` Michael S. Tsirkin
2012-08-29 15:38 ` Michael S. Tsirkin
2012-08-29 16:50 ` Sasha Levin
2012-08-29 16:50 ` Sasha Levin
2012-09-06 1:02 ` Rusty Russell
2012-09-06 1:02 ` Rusty Russell
2012-09-06 5:02 ` Michael S. Tsirkin
2012-09-06 5:02 ` Michael S. Tsirkin
2012-09-06 7:57 ` Rusty Russell
2012-09-06 7:57 ` Rusty Russell
2012-09-06 8:45 ` Michael S. Tsirkin
2012-09-06 8:45 ` Michael S. Tsirkin
2012-09-06 23:49 ` Rusty Russell
2012-09-07 0:06 ` Michael S. Tsirkin
2012-09-07 0:06 ` Michael S. Tsirkin
2012-09-10 15:47 ` Thomas Lendacky
2012-09-10 16:08 ` Michael S. Tsirkin
2012-09-10 16:08 ` Michael S. Tsirkin
2012-09-12 6:13 ` Rusty Russell
2012-09-12 10:44 ` Sasha Levin
2012-09-12 10:44 ` Sasha Levin
2012-10-23 15:14 ` Michael S. Tsirkin
2012-10-23 15:14 ` Michael S. Tsirkin
2012-09-12 6:13 ` Rusty Russell
2012-09-10 16:01 ` Thomas Lendacky
2012-09-10 16:01 ` Thomas Lendacky
2012-09-06 23:49 ` Rusty Russell
2012-09-10 15:52 ` Paolo Bonzini
2012-09-10 15:52 ` Paolo Bonzini
2012-09-06 0:02 ` Rusty Russell [this message]
2012-09-06 0:02 ` Rusty Russell
2012-08-29 15:38 ` Michael S. Tsirkin
2012-08-29 15:38 ` Michael S. Tsirkin
2012-08-29 17:14 ` Sasha Levin
2012-08-29 17:14 ` Sasha Levin
2012-08-29 18:12 ` Michael S. Tsirkin
2012-08-29 18:12 ` Michael S. Tsirkin
2012-08-29 20:46 ` Sasha Levin
2012-08-29 20:46 ` Sasha Levin
2012-08-29 22:52 ` Michael S. Tsirkin
2012-08-29 22:52 ` Michael S. Tsirkin
2012-08-28 13:20 ` [PATCH v2 1/2] virtio-ring: Use threshold for switching to indirect descriptors Michael S. Tsirkin
2012-08-28 13:20 ` Michael S. Tsirkin
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='874nncj9x5.fsf__10496.4864826345$1346900525$gmane$org@rustcorp.com.au' \
--to=rusty@rustcorp.com.au \
--cc=avi@redhat.com \
--cc=kvm@vger.kernel.org \
--cc=levinsasha928@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mst@redhat.com \
--cc=virtualization@lists.linux-foundation.org \
/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.