Re: [PATCH RFC v5 net-next 1/6] virtio_ring: fix virtqueue_enable_cb() when only 1 buffers were pending

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On Tue, Feb 10, 2015 at 9:03 AM, Rusty Russell <rusty@xxxxxxxxxxxxxxx> wrote:
Jason Wang <jasowang@xxxxxxxxxx> writes:
 We currently does:

 bufs = (avail->idx - last_used_idx) * 3 / 4;

This is ok now since we only try to enable the delayed callbacks when
 the queue is about to be full. This may not work well when there is
 only one pending buffer in the virtqueue (this may be the case after
 tx interrupt was enabled). Since virtqueue_enable_cb() will return
 false which may cause unnecessary triggering of napis. This patch
 correct this by only calculate the four thirds when bufs is not one.

I mildly prefer to avoid the branch, by changing the calculation like
so:

        /* Set bufs >= 1, even if there's only one pending buffer */
        bufs = (bufs + 1) * 3 / 4;

Ok.

But it's not clear to me how much this happens.

Depends on the traffic type. In some case e.g one session TCP_RR test (which has at most 1 pending packet), it may happen very often.
 I'm happy with the
patch though, as currently virtqueue_enable_cb_delayed() is the same
as virtqueue_enable_cb() if there's only been one buffer added.

Yes. Thanks.

Cheers,
Rusty.

 Signed-off-by: Jason Wang <jasowang@xxxxxxxxxx>
 ---
  drivers/virtio/virtio_ring.c | 5 ++++-
  1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
 index 00ec6b3..545fed5 100644
 --- a/drivers/virtio/virtio_ring.c
 +++ b/drivers/virtio/virtio_ring.c
@@ -636,7 +636,10 @@ bool virtqueue_enable_cb_delayed(struct virtqueue *_vq)
  	 * entry. Always do both to keep code simple. */
vq->vring.avail->flags &= cpu_to_virtio16(_vq->vdev, ~VRING_AVAIL_F_NO_INTERRUPT);
  	/* TODO: tune this threshold */
- bufs = (u16)(virtio16_to_cpu(_vq->vdev, vq->vring.avail->idx) - vq->last_used_idx) * 3 / 4;
 +	bufs = (u16)(virtio16_to_cpu(_vq->vdev, vq->vring.avail->idx) -
 +		                     vq->last_used_idx);
 +	if (bufs != 1)
 +		bufs = bufs * 3 / 4;
vring_used_event(&vq->vring) = cpu_to_virtio16(_vq->vdev, vq->last_used_idx + bufs);
  	virtio_mb(vq->weak_barriers);
if (unlikely((u16)(virtio16_to_cpu(_vq->vdev, vq->vring.used->idx) - vq->last_used_idx) > bufs)) { -- 1.8.3.1

 _______________________________________________
 Virtualization mailing list
 Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
 https://lists.linuxfoundation.org/mailman/listinfo/virtualization

_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization




[Index of Archives]     [KVM Development]     [Libvirt Development]     [Libvirt Users]     [CentOS Virtualization]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux