Re: [PATCH RFC v5 net-next 2/6] virtio_ring: try to disable event index callbacks in virtqueue_disable_cb()

From: Jason Wang
Date: Wed Feb 11 2015 - 00:55:52 EST

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

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



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.

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.

To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at