On Tue, Feb 10, 2015 at 6:24 PM, Michael S. Tsirkin <mst@xxxxxxxxxx>
wrote:
On Mon, Feb 09, 2015 at 03:39:21AM -0500, Jason Wang wrote:
Currently, we do nothing to prevent the callbacks in
virtqueue_disable_cb() when event index is used. This may cause
spurious interrupts which may damage the performance. This patch
tries
to publish avail event as the used even to prevent the callbacks.
Signed-off-by: Jason Wang <jasowang@xxxxxxxxxx>
I'm surprised that this ever happens though.
Normally we call this after getting an interrupt, and
interrupts won't trigger again until the rings wraps around.
It was used to disable tx interrupt during start_xmit().
When I tested this, touching an extra cache line was more
expensive.
Does this really reduce number of interrupts?
Could you pls share some numbers with and without this patch?
Yes. It does reduce, see the following test results on multiple
sessions of TCP_RR
Datacopy:
/size/sessions/+-throughput%/+-cpu%/+-per_cpu_throughput%/+-tx_interrupts%/
/1/50/+0.1/+0.7/-0.6/-72.3/
/64/50/+1.7/+1.0/+0.7/-72.2/
/256/50/+0.0/-0.8/+0.9/-72.4/
Zerocopy:
/size/sessions/+-throughput%/+-cpu%/+-per_cpu_throughput%/+-tx_interrupts%/
/1/50/+3.2/-1.3/+4.6/-71.7/
/64/50/+1.9/-0.2/+2.1/-70.8/
/256/50/+4.4/+0.6/+3.9/-84.2/
No obvious changes for stream rx and tx.
---
drivers/virtio/virtio_ring.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/virtio/virtio_ring.c
b/drivers/virtio/virtio_ring.c
index 545fed5..e9ffbfb 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -539,6 +539,8 @@ void virtqueue_disable_cb(struct virtqueue *_vq)
struct vring_virtqueue *vq = to_vvq(_vq);
vq->vring.avail->flags |= cpu_to_virtio16(_vq->vdev,
VRING_AVAIL_F_NO_INTERRUPT);
+ vring_used_event(&vq->vring) = cpu_to_virtio16(_vq->vdev,
+ vq->vring.avail->idx);
Hmm in fact, can't this actually cause an extra interrupt
when avail->idx is completed?
We need to try best to avoid any interrupt after this and
virtqueue_disable_cb() were always need virtqueue_enable_cb() or
virtqueue_enable_cb_delayed() afterwards. Those two function will check
the pending used buffers and set a proper used event.
I think that if you really can show disabling interrupts like this
helps, you should
set some invalid value like 0xfffff, or move it back to
vq->vring.avail->idx - 1.
No?
I tested avail->idx + vq.num but not obvious changes in the performance
compared to just use avail->idx here. So 0xffff probably won't help
much. Since we call virtqueue_enable_cb() or
virtqueue_disable_cb_delayed() afterwards, it doesn't matter that
whether avail->idx or avail->idx - 1 is used.
}
EXPORT_SYMBOL_GPL(virtqueue_disable_cb);
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
