diff mbox

[1/3] kvm tools: Make virtio-net kvm__irq_line thread safe

Message ID 4DBA6AE5.8090703@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Asias He April 29, 2011, 7:38 a.m. UTC
On 04/29/2011 03:15 PM, Pekka Enberg wrote:
> On Fri, Apr 29, 2011 at 10:13 AM, Asias He <asias.hejun@gmail.com> wrote:
>> No. We only need it in RX path. Sasha's threadpool patch breaks this.
>> I'm just moving it back.
> 
> OK, cool! Can you send the fix as a separate patch?
> 

Here you go.

Comments

Ingo Molnar April 29, 2011, 7:45 a.m. UTC | #1
* Asias He <asias.hejun@gmail.com> wrote:

> On 04/29/2011 03:15 PM, Pekka Enberg wrote:
> > On Fri, Apr 29, 2011 at 10:13 AM, Asias He <asias.hejun@gmail.com> wrote:
> >> No. We only need it in RX path. Sasha's threadpool patch breaks this.
> >> I'm just moving it back.
> > 
> > OK, cool! Can you send the fix as a separate patch?
> > 
> 
> Here you go.
> 
> -- 
> Best Regards,
> Asias He

> >From 9c188853435be23ad2a73b5062690eb9f2ff1d25 Mon Sep 17 00:00:00 2001
> From: Asias He <asias.hejun@gmail.com>
> Date: Fri, 29 Apr 2011 10:43:35 +0800
> Subject: [PATCH] kvm tools: Fix virtio-net RX path IRQ latency
> 
> The TX path does not need this ;-)

no explanation ...

> Signed-off-by: Asias He <asias.hejun@gmail.com>
> ---
>  tools/kvm/virtio-net.c |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/kvm/virtio-net.c b/tools/kvm/virtio-net.c
> index 58b3de4..310c061 100644
> --- a/tools/kvm/virtio-net.c
> +++ b/tools/kvm/virtio-net.c
> @@ -77,9 +77,10 @@ static void virtio_net_rx_callback(struct kvm *self, void *param)
>  		head = virt_queue__get_iov(vq, iov, &out, &in, self);
>  		len = readv(net_device.tap_fd, iov, in);
>  		virt_queue__set_used_elem(vq, head, len);
> -	}
>  
> -	kvm__irq_line(self, VIRTIO_NET_IRQ, 1);
> +		/* We should interrupt guest right now, otherwise latency is huge. */
> +		kvm__irq_line(self, VIRTIO_NET_IRQ, 1);
> +	}

I'm wondering, why is this needed?

The change seems to *reduce* the number of irq events on the guest side. How 
can this reduce stalls?

I.e. it would be nice to see an explanation about what it did before, how that 
caused problems and how this fix is the best solution to that. Otherwise it 
gives a "tinker the state machine until the stall goes away" kind of hack, 
which risks leaving some real bug ununderstood, unfixed or possibly 
reintroduced in the future.

The explanation is often more important than the fix itself!

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

From 9c188853435be23ad2a73b5062690eb9f2ff1d25 Mon Sep 17 00:00:00 2001
From: Asias He <asias.hejun@gmail.com>
Date: Fri, 29 Apr 2011 10:43:35 +0800
Subject: [PATCH] kvm tools: Fix virtio-net RX path IRQ latency

The TX path does not need this ;-)

Signed-off-by: Asias He <asias.hejun@gmail.com>
---
 tools/kvm/virtio-net.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/kvm/virtio-net.c b/tools/kvm/virtio-net.c
index 58b3de4..310c061 100644
--- a/tools/kvm/virtio-net.c
+++ b/tools/kvm/virtio-net.c
@@ -77,9 +77,10 @@  static void virtio_net_rx_callback(struct kvm *self, void *param)
 		head = virt_queue__get_iov(vq, iov, &out, &in, self);
 		len = readv(net_device.tap_fd, iov, in);
 		virt_queue__set_used_elem(vq, head, len);
-	}
 
-	kvm__irq_line(self, VIRTIO_NET_IRQ, 1);
+		/* We should interrupt guest right now, otherwise latency is huge. */
+		kvm__irq_line(self, VIRTIO_NET_IRQ, 1);
+	}
 }
 
 static void virtio_net_tx_callback(struct kvm *self, void *param)
-- 
1.7.4.4