Messages in this thread | | | Date | Wed, 8 Apr 2015 10:42:55 +0200 | From | "Michael S. Tsirkin" <> | Subject | Re: [PATCH] virtio_ring: Update weak barriers to use dma_wmb/rmb |
| |
On Tue, Apr 07, 2015 at 05:47:42PM -0700, Alexander Duyck wrote: > This change makes it so that instead of using smp_wmb/rmb which varies > depending on the kernel configuration we can can use dma_wmb/rmb which for > most architectures should be equal to or slightly more strict than > smp_wmb/rmb. > > The advantage to this is that these barriers are available to uniprocessor > builds as well so the performance should improve under such a > configuration. > > Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
Well the generic implementation has: #ifndef dma_rmb #define dma_rmb() rmb() #endif
#ifndef dma_wmb #define dma_wmb() wmb() #endif
So for these arches you are slightly speeding up UP but slightly hurting SMP - I think we did benchmark the difference as measureable in the past.
Additionally, isn't this relying on undocumented behaviour? The documentation says: "These are for use with consistent memory" and virtio does not bother to request consistent memory allocations.
One wonders whether these will always be strong enough.
> --- > include/linux/virtio_ring.h | 23 ++++------------------- > 1 file changed, 4 insertions(+), 19 deletions(-) > > diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h > index 67e06fe18c03..8e50888a6d59 100644 > --- a/include/linux/virtio_ring.h > +++ b/include/linux/virtio_ring.h > @@ -21,19 +21,20 @@ > * actually quite cheap. > */ > > -#ifdef CONFIG_SMP > static inline void virtio_mb(bool weak_barriers) > { > +#ifdef CONFIG_SMP > if (weak_barriers) > smp_mb(); > else > +#endif > mb(); > } > > static inline void virtio_rmb(bool weak_barriers) > { > if (weak_barriers) > - smp_rmb(); > + dma_rmb(); > else > rmb(); > } > @@ -41,26 +42,10 @@ static inline void virtio_rmb(bool weak_barriers) > static inline void virtio_wmb(bool weak_barriers) > { > if (weak_barriers) > - smp_wmb(); > + dma_wmb(); > else > wmb(); > } > -#else > -static inline void virtio_mb(bool weak_barriers) > -{ > - mb(); > -} > - > -static inline void virtio_rmb(bool weak_barriers) > -{ > - rmb(); > -} > - > -static inline void virtio_wmb(bool weak_barriers) > -{ > - wmb(); > -} > -#endif > > struct virtio_device; > struct virtqueue;
| |