Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1293875

Re: [PATCH] virtio_ring: use smp_store_mb

From "Michael S. Tsirkin" <mst@redhat.com>
Newsgroups linux.kernel
Subject Re: [PATCH] virtio_ring: use smp_store_mb
Date 2015-12-17 14:20 +0100
Message-ID <qGGDo-8kx-13@gated-at.bofh.it> (permalink)
References <qGE8x-6xd-9@gated-at.bofh.it> <qGErU-6H4-13@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Thu, Dec 17, 2015 at 11:52:38AM +0100, Peter Zijlstra wrote:
> On Thu, Dec 17, 2015 at 12:32:53PM +0200, Michael S. Tsirkin wrote:
> > +static inline void virtio_store_mb(bool weak_barriers,
> > +				   __virtio16 *p, __virtio16 v)
> > +{
> > +#ifdef CONFIG_SMP
> > +	if (weak_barriers)
> > +		smp_store_mb(*p, v);
> > +	else
> > +#endif
> > +	{
> > +		WRITE_ONCE(*p, v);
> > +		mb();
> > +	}
> > +}
> 
> This is a different barrier depending on SMP, that seems wrong.

Of course it's wrong in the sense that it's
suboptimal on UP. What we would really like is to
have, on UP, exactly the same barrier as on SMP.
This is because a UP guest can run on an SMP host.

But Linux doesn't provide this ability: if CONFIG_SMP is
not defined is optimizes most barriers out to a
compiler barrier.

Consider for example x86: what we want is xchg (NOT
mfence - see below for why) but if built without CONFIG_SMP
smp_store_mb does not include this.



> smp_mb(), as (should be) used by smp_store_mb() does not provide a
> barrier against IO. mb() otoh does.
> 
> Since this is virtIO I would expect you always want mb().

No because it's VIRTio not real io :) It just switches to the hyprevisor
mode - kind of like a function call really.
The weak_barriers flag is cleared for when it's used
with real devices with real IO.


All this is explained in some detail at the top of
include/linux/virtio.h



-- 
MST
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH] virtio_ring: use smp_store_mb "Michael S. Tsirkin" <mst@redhat.com> - 2015-12-17 11:40 +0100
  Re: [PATCH] virtio_ring: use smp_store_mb Peter Zijlstra <peterz@infradead.org> - 2015-12-17 12:00 +0100
    Re: [PATCH] virtio_ring: use smp_store_mb "Michael S. Tsirkin" <mst@redhat.com> - 2015-12-17 14:20 +0100
      Re: [PATCH] virtio_ring: use smp_store_mb Peter Zijlstra <peterz@infradead.org> - 2015-12-17 15:00 +0100
        Re: [PATCH] virtio_ring: use smp_store_mb "Michael S. Tsirkin" <mst@redhat.com> - 2015-12-17 15:40 +0100
          Re: [PATCH] virtio_ring: use smp_store_mb Peter Zijlstra <peterz@infradead.org> - 2015-12-17 15:40 +0100
            Re: [PATCH] virtio_ring: use smp_store_mb "Michael S. Tsirkin" <mst@redhat.com> - 2015-12-17 15:50 +0100
            new barrier type for paravirt (was Re: [PATCH] virtio_ring: use  smp_store_mb) "Michael S. Tsirkin" <mst@redhat.com> - 2015-12-20 10:30 +0100
              Re: [Xen-devel] new barrier type for paravirt (was Re: [PATCH]  virtio_ring: use smp_store_mb) Andrew Cooper <andrew.cooper3@citrix.com> - 2015-12-20 18:10 +0100
                Re: [Xen-devel] new barrier type for paravirt (was Re: [PATCH]  virtio_ring: use smp_store_mb) Peter Zijlstra <peterz@infradead.org> - 2015-12-20 21:10 +0100
                Re: [Xen-devel] new barrier type for paravirt (was Re: [PATCH]  virtio_ring: use smp_store_mb) "Michael S. Tsirkin" <mst@redhat.com> - 2015-12-21 08:20 +0100
                [PATCH RFC] smp_store_mb should use smp_mb "Michael S. Tsirkin" <mst@redhat.com> - 2015-12-21 08:30 +0100
              Re: [Xen-devel] new barrier type for paravirt (was Re: [PATCH] virtio_ring:  use smp_store_mb) David Vrabel <david.vrabel@citrix.com> - 2015-12-21 11:50 +0100
                Re: [Xen-devel] new barrier type for paravirt (was Re: [PATCH]  virtio_ring: use smp_store_mb) "Michael S. Tsirkin" <mst@redhat.com> - 2015-12-21 13:00 +0100
                Re: [Qemu-devel] [Xen-devel] new barrier type for paravirt (was Re:  [PATCH] virtio_ring: use smp_store_mb) Stefano Stabellini <stefano.stabellini@eu.citrix.com> - 2015-12-21 16:00 +0100
  Re: [PATCH] virtio_ring: use smp_store_mb Peter Zijlstra <peterz@infradead.org> - 2015-12-17 12:30 +0100
    Re: [PATCH] virtio_ring: use smp_store_mb "Michael S. Tsirkin" <mst@redhat.com> - 2015-12-17 14:30 +0100
      Re: [PATCH] virtio_ring: use smp_store_mb Peter Zijlstra <peterz@infradead.org> - 2015-12-17 15:10 +0100
        Re: [PATCH] virtio_ring: use smp_store_mb "Michael S. Tsirkin" <mst@redhat.com> - 2015-12-17 15:40 +0100
          Re: [PATCH] virtio_ring: use smp_store_mb Peter Zijlstra <peterz@infradead.org> - 2015-12-17 16:10 +0100
            Re: [PATCH] virtio_ring: use smp_store_mb Will Deacon <will.deacon@arm.com> - 2015-12-17 17:00 +0100
              Re: [PATCH] virtio_ring: use smp_store_mb "Michael S. Tsirkin" <mst@redhat.com> - 2015-12-17 20:30 +0100

csiph-web