Re: [Qemu-devel] [PATCH] virtio: Make memory barriers be memory barriers

[David Gibson]

On Fri, Sep 02, 2011 at 06:45:50PM +0300, Michael S. Tsirkin wrote:
> On Thu, Sep 01, 2011 at 04:31:09PM -0400, Paolo Bonzini wrote:
> > > > > Why not limit the change to ppc then?
> > > >
> > > > Because the bug is masked by the x86 memory model, but it is still
> > > > there even there conceptually. It is not really true that x86 does
> > > > not need memory barriers, though it doesn't in this case:
> > > >
> > > > http://bartoszmilewski.wordpress.com/2008/11/05/who-ordered-memory-fences-on-an-x86/
> > > >
> > > > Paolo
> > > 
> > > Right.
> > > To summarize, on x86 we probably want wmb and rmb to be compiler
> > > barrier only. Only mb might in theory need to be an mfence.
> > 
> > No, wmb needs to be sfence and rmb needs to be lfence.  GCC does
> > not provide those, so they should become __sync_synchronize() too,
> > or you should use inline assembly.
> > 
> > > But there might be reasons why that is not an issue either
> > > if we look closely enough.
> > 
> > Since the ring buffers are not using locked instructions (no xchg
> > or cmpxchg) the barriers simply must be there, even on x86.  Whether
> > it works in practice is not interesting, only the formal model is
> > interesting.
> > 
> > Paolo
> 
> Well, can you describe an issue in virtio that lfence/sfence help solve
> in terms of a memory model please?
> Pls note that guest uses smp_ variants for barriers.

Ok, so, I'm having a bit of trouble with the fact that I'm having to
argue the case that things the protocol requiress to be memory
barriers actually *be* memory barriers on all platforms.

I mean argue for a richer set of barriers, with per-arch minimal
implementations instead of the large but portable hammer of
sync_synchronize, if you will.  But just leaving them out on x86!?
Seriously, wtf?  Do you enjoy having software that works chiefly by
accident?

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson