Re: [PATCH 3/9] kvm: qemu: Remove virtio_net tx ring-full heuristic
Mark McLoughlin wrote:
virtio_net tries to guess when it has received a tx
notification from the guest whether it indicates that the
guest has no more room in the tx ring and it should
immediately flush the queued buffers.
The heuristic is based on the fact that there are 128
buffer entries in the ring and each packet uses 2 buffers
(i.e. the virtio_net_hdr and the packet's linear data).
Using GSO or increasing the size of the rings will break
that heuristic, so let's remove it and assume that any
notification from the guest after we've disabled
notifications indicates that we should flush our buffers.
Signed-off-by: Mark McLoughlin <[EMAIL PROTECTED]>
---
qemu/hw/virtio-net.c |3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/qemu/hw/virtio-net.c b/qemu/hw/virtio-net.c
index 31867f1..4adfa42 100644
--- a/qemu/hw/virtio-net.c
+++ b/qemu/hw/virtio-net.c
@@ -175,8 +175,7 @@ static void virtio_net_handle_tx(VirtIODevice *vdev,
VirtQueue *vq)
{
VirtIONet *n = to_virtio_net(vdev);
-if (n->tx_timer_active &&
- (vq->vring.avail->idx - vq->last_avail_idx) == 64) {
+if (n->tx_timer_active) {
Actually it was worthless anyway since if we set the timer, the flag
below would have been cleared,
causing the guest not to notify the host, thus the ==64 never called.
So I'm in favour too.
vq->vring.used->flags &= ~VRING_USED_F_NO_NOTIFY;
qemu_del_timer(n->tx_timer);
n->tx_timer_active = 0;
As stated by newer messages, we should handle the first tx notification
if the timer wasn't active to shorten latency.
Cheers, Dor
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/9] kvm: qemu: Remove virtio_net tx ring-full heuristic
Mark McLoughlin wrote:
virtio_net tries to guess when it has received a tx
notification from the guest whether it indicates that the
guest has no more room in the tx ring and it should
immediately flush the queued buffers.
The heuristic is based on the fact that there are 128
buffer entries in the ring and each packet uses 2 buffers
(i.e. the virtio_net_hdr and the packet's linear data).
Using GSO or increasing the size of the rings will break
that heuristic, so let's remove it and assume that any
notification from the guest after we've disabled
notifications indicates that we should flush our buffers.
Signed-off-by: Mark McLoughlin <[EMAIL PROTECTED]>
---
qemu/hw/virtio-net.c |3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/qemu/hw/virtio-net.c b/qemu/hw/virtio-net.c
index 31867f1..4adfa42 100644
--- a/qemu/hw/virtio-net.c
+++ b/qemu/hw/virtio-net.c
@@ -175,8 +175,7 @@ static void virtio_net_handle_tx(VirtIODevice *vdev,
VirtQueue *vq)
{
VirtIONet *n = to_virtio_net(vdev);
-if (n->tx_timer_active &&
- (vq->vring.avail->idx - vq->last_avail_idx) == 64) {
+if (n->tx_timer_active) {
vq->vring.used->flags &= ~VRING_USED_F_NO_NOTIFY;
qemu_del_timer(n->tx_timer);
n->tx_timer_active = 0;
Actually we can improve latency a bit more by using this timer only for
high throughput
scenario. For example, if during the previous timer period no/few
packets were accumulated,
we can set the flag off and not issue new timer. This way we'll get
notified immediately without timer
latency. When lots of packets will be transmitted, we'll go back to this
batch mode again.
Cheers, Dor
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/9] kvm: qemu: Remove virtio_net tx ring-full heuristic
On Friday 25 July 2008 09:22:53 Dor Laor wrote:
> Mark McLoughlin wrote:
> > vq->vring.used->flags &= ~VRING_USED_F_NO_NOTIFY;
> > qemu_del_timer(n->tx_timer);
> > n->tx_timer_active = 0;
>
> As stated by newer messages, we should handle the first tx notification
> if the timer wasn't active to shorten latency.
> Cheers, Dor
Here's what lguest does at the moment. Basically, we cut the timeout a tiny
bit each time, until we get *fewer* packets than last time. Then we bump it
up again.
Rough, but seems to work (it should be a per-device var of course, not a
static).
@@ -921,6 +922,7 @@ static void handle_net_output(int fd, st
unsigned int head, out, in, num = 0;
int len;
struct iovec iov[vq->vring.num];
+ static int last_timeout_num;
if (!timeout)
net_xmit_notify++;
@@ -941,6 +943,14 @@ static void handle_net_output(int fd, st
/* Block further kicks and set up a timer if we saw anything. */
if (!timeout && num)
block_vq(vq);
+
+ if (timeout) {
+ if (num < last_timeout_num)
+ timeout_usec += 10;
+ else if (timeout_usec > 1)
+ timeout_usec--;
+ last_timeout_num = num;
+ }
}
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/9] kvm: qemu: Remove virtio_net tx ring-full heuristic
On Fri, 2008-07-25 at 02:22 +0300, Dor Laor wrote:
> Mark McLoughlin wrote:
> > virtio_net tries to guess when it has received a tx
> > notification from the guest whether it indicates that the
> > guest has no more room in the tx ring and it should
> > immediately flush the queued buffers.
...
> > -if (n->tx_timer_active &&
> > - (vq->vring.avail->idx - vq->last_avail_idx) == 64) {
> > +if (n->tx_timer_active) {
> >
> Actually it was worthless anyway since if we set the timer, the flag
> below would have been cleared,
> causing the guest not to notify the host, thus the ==64 never called.
No, that's the point I'm trying to make in the commit message - the
guest ignores the NO_NOTIFY flag when the tx queue fills up and sends a
notification anyway.
The "== 64" test seems to be an attempt to catch this ring-full
condition.
Well, actually the test was worthless for a different reason, I guess.
If tx_timer_active is set and we get a notification, then given the
current guest code the number of buffers available should *always* be
64.
Cheers,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/9] kvm: qemu: Remove virtio_net tx ring-full heuristic
On Fri, 2008-07-25 at 10:30 +1000, Rusty Russell wrote:
> On Friday 25 July 2008 09:22:53 Dor Laor wrote:
> > Mark McLoughlin wrote:
> > > vq->vring.used->flags &= ~VRING_USED_F_NO_NOTIFY;
> > > qemu_del_timer(n->tx_timer);
> > > n->tx_timer_active = 0;
> >
> > As stated by newer messages, we should handle the first tx notification
> > if the timer wasn't active to shorten latency.
> > Cheers, Dor
>
> Here's what lguest does at the moment. Basically, we cut the timeout a tiny
> bit each time, until we get *fewer* packets than last time. Then we bump it
> up again.
>
> Rough, but seems to work (it should be a per-device var of course, not a
> static).
>
> @@ -921,6 +922,7 @@ static void handle_net_output(int fd, st
> unsigned int head, out, in, num = 0;
> int len;
> struct iovec iov[vq->vring.num];
> + static int last_timeout_num;
>
> if (!timeout)
> net_xmit_notify++;
> @@ -941,6 +943,14 @@ static void handle_net_output(int fd, st
> /* Block further kicks and set up a timer if we saw anything. */
> if (!timeout && num)
> block_vq(vq);
> +
> + if (timeout) {
> + if (num < last_timeout_num)
> + timeout_usec += 10;
> + else if (timeout_usec > 1)
> + timeout_usec--;
> + last_timeout_num = num;
> + }
Yeah, I gave this a try in kvm and in the host->guest case the timeout
just grew and grew. In the guest->host case, it did stabilise at around
50us with high throughput.
Basically, I think in the host->guest case the number of buffers was
very variable so the timeout would get bumped by 10, reduced by a small
amount, bumped by 10, reduced by a small amount, ...
But, I agree the general principal seems about right; just needs some
tweaking.
Cheers,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
