Commit 7b21e34f authored by Rusty Russell's avatar Rusty Russell
Browse files

virtio: harsher barriers for rpmsg. 

We were cheating with our barriers; using the smp ones rather than the
real device ones.  That was fine, until rpmsg came along, which is
used to talk to a real device (a non-SMP CPU).

Unfortunately, just putting back the real barriers (reverting
d57ed95d) causes a performance regression on virtio-pci.  In
particular, Amos reports netbench's TCP_RR over virtio_net CPU
utilization increased up to 35% while throughput went down by up to
14%.

By comparison, this branch is in the noise.

Reference: https://lkml.org/lkml/2011/12/11/22



Signed-off-by: default avatarRusty Russell <rusty@rustcorp.com.au>
parent e343a895

drivers/lguest/lguest_device.c

+5 −3
Original line number Diff line number Diff line
@@ -292,10 +292,12 @@ static struct virtqueue *lg_find_vq(struct virtio_device *vdev, 


	/*

	 * OK, tell virtio_ring.c to set up a virtqueue now we know its size

	 * and we've got a pointer to its pages.

	 * and we've got a pointer to its pages.  Note that we set weak_barriers

	 * to 'true': the host just a(nother) SMP CPU, so we only need inter-cpu

	 * barriers.

	 */

	vq = vring_new_virtqueue(lvq->config.num, LGUEST_VRING_ALIGN,

				 vdev, lvq->pages, lg_notify, callback, name);

	vq = vring_new_virtqueue(lvq->config.num, LGUEST_VRING_ALIGN, vdev,

				 true, lvq->pages, lg_notify, callback, name);

	if (!vq) {

		err = -ENOMEM;

		goto unmap;

drivers/s390/kvm/kvm_virtio.c

+1 −1
Original line number Diff line number Diff line
@@ -198,7 +198,7 @@ static struct virtqueue *kvm_find_vq(struct virtio_device *vdev, 
		goto out;



	vq = vring_new_virtqueue(config->num, KVM_S390_VIRTIO_RING_ALIGN,

				 vdev, (void *) config->address,

				 vdev, true, (void *) config->address,

				 kvm_notify, callback, name);

	if (!vq) {

		err = -ENOMEM;

drivers/virtio/virtio_mmio.c

+2 −2
Original line number Diff line number Diff line
@@ -310,8 +310,8 @@ static struct virtqueue *vm_setup_vq(struct virtio_device *vdev, unsigned index, 
			vm_dev->base + VIRTIO_MMIO_QUEUE_PFN);



	/* Create the vring */

	vq = vring_new_virtqueue(info->num, VIRTIO_MMIO_VRING_ALIGN,

				 vdev, info->queue, vm_notify, callback, name);

	vq = vring_new_virtqueue(info->num, VIRTIO_MMIO_VRING_ALIGN, vdev,

				 true, info->queue, vm_notify, callback, name);

	if (!vq) {

		err = -ENOMEM;

		goto error_new_virtqueue;

drivers/virtio/virtio_pci.c

+2 −2
Original line number Diff line number Diff line
@@ -414,8 +414,8 @@ static struct virtqueue *setup_vq(struct virtio_device *vdev, unsigned index, 
		  vp_dev->ioaddr + VIRTIO_PCI_QUEUE_PFN);



	/* create the vring */

	vq = vring_new_virtqueue(info->num, VIRTIO_PCI_VRING_ALIGN,

				 vdev, info->queue, vp_notify, callback, name);

	vq = vring_new_virtqueue(info->num, VIRTIO_PCI_VRING_ALIGN, vdev,

				 true, info->queue, vp_notify, callback, name);

	if (!vq) {

		err = -ENOMEM;

		goto out_activate_queue;

drivers/virtio/virtio_ring.c

+21 −13
Original line number Diff line number Diff line
@@ -28,17 +28,20 @@ 
#ifdef CONFIG_SMP

/* Where possible, use SMP barriers which are more lightweight than mandatory

 * barriers, because mandatory barriers control MMIO effects on accesses

 * through relaxed memory I/O windows (which virtio does not use). */

#define virtio_mb() smp_mb()

#define virtio_rmb() smp_rmb()

#define virtio_wmb() smp_wmb()

 * through relaxed memory I/O windows (which virtio-pci does not use). */

#define virtio_mb(vq) \

	do { if ((vq)->weak_barriers) smp_mb(); else mb(); } while(0)

#define virtio_rmb(vq) \

	do { if ((vq)->weak_barriers) smp_rmb(); else rmb(); } while(0)

#define virtio_wmb(vq) \

	do { if ((vq)->weak_barriers) smp_rmb(); else rmb(); } while(0)

#else

/* We must force memory ordering even if guest is UP since host could be

 * running on another CPU, but SMP barriers are defined to barrier() in that

 * configuration. So fall back to mandatory barriers instead. */

#define virtio_mb() mb()

#define virtio_rmb() rmb()

#define virtio_wmb() wmb()

#define virtio_mb(vq) mb()

#define virtio_rmb(vq) rmb()

#define virtio_wmb(vq) wmb()

#endif



#ifdef DEBUG
@@ -77,6 +80,9 @@ struct vring_virtqueue 
	/* Actual memory layout for this queue */

	struct vring vring;



	/* Can we use weak barriers? */

	bool weak_barriers;



	/* Other side has made a mess, don't try any more. */

	bool broken;
@@ -245,14 +251,14 @@ void virtqueue_kick(struct virtqueue *_vq) 
	START_USE(vq);

	/* Descriptors and available array need to be set before we expose the

	 * new available array entries. */

	virtio_wmb();

	virtio_wmb(vq);



	old = vq->vring.avail->idx;

	new = vq->vring.avail->idx = old + vq->num_added;

	vq->num_added = 0;



	/* Need to update avail index before checking if we should notify */

	virtio_mb();

	virtio_mb(vq);



	if (vq->event ?

	    vring_need_event(vring_avail_event(&vq->vring), new, old) :
@@ -314,7 +320,7 @@ void *virtqueue_get_buf(struct virtqueue *_vq, unsigned int *len) 
	}



	/* Only get used array entries after they have been exposed by host. */

	virtio_rmb();

	virtio_rmb(vq);



	i = vq->vring.used->ring[vq->last_used_idx%vq->vring.num].id;

	*len = vq->vring.used->ring[vq->last_used_idx%vq->vring.num].len;
@@ -337,7 +343,7 @@ void *virtqueue_get_buf(struct virtqueue *_vq, unsigned int *len) 
	 * the read in the next get_buf call. */

	if (!(vq->vring.avail->flags & VRING_AVAIL_F_NO_INTERRUPT)) {

		vring_used_event(&vq->vring) = vq->last_used_idx;

		virtio_mb();

		virtio_mb(vq);

	}



	END_USE(vq);
@@ -366,7 +372,7 @@ bool virtqueue_enable_cb(struct virtqueue *_vq) 
	 * entry. Always do both to keep code simple. */

	vq->vring.avail->flags &= ~VRING_AVAIL_F_NO_INTERRUPT;

	vring_used_event(&vq->vring) = vq->last_used_idx;

	virtio_mb();

	virtio_mb(vq);

	if (unlikely(more_used(vq))) {

		END_USE(vq);

		return false;
@@ -393,7 +399,7 @@ bool virtqueue_enable_cb_delayed(struct virtqueue *_vq) 
	/* TODO: tune this threshold */

	bufs = (u16)(vq->vring.avail->idx - vq->last_used_idx) * 3 / 4;

	vring_used_event(&vq->vring) = vq->last_used_idx + bufs;

	virtio_mb();

	virtio_mb(vq);

	if (unlikely((u16)(vq->vring.used->idx - vq->last_used_idx) > bufs)) {

		END_USE(vq);

		return false;
@@ -453,6 +459,7 @@ EXPORT_SYMBOL_GPL(vring_interrupt); 
struct virtqueue *vring_new_virtqueue(unsigned int num,

				      unsigned int vring_align,

				      struct virtio_device *vdev,

				      bool weak_barriers,

				      void *pages,

				      void (*notify)(struct virtqueue *),

				      void (*callback)(struct virtqueue *),
@@ -476,6 +483,7 @@ struct virtqueue *vring_new_virtqueue(unsigned int num, 
	vq->vq.vdev = vdev;

	vq->vq.name = name;

	vq->notify = notify;

	vq->weak_barriers = weak_barriers;

	vq->broken = false;

	vq->last_used_idx = 0;

	vq->num_added = 0;

CRA Git | Maintained and supported by SUSTech CRA and CCSE