On 3 June 2015 at 13:21, Paolo Bonzini <[email protected]> wrote: > __atomic_thread_fence does not include a compiler barrier; in the > C++11 memory model, fences take effect in combination with other > atomic operations. GCC implements this by making __atomic_load and > __atomic_store access memory as if the pointer was volatile, and > leaves no trace whatsoever of acquire and release fences in the > compiler's intermediate representation. > > In QEMU, we want memory barriers to act on all memory, but at the same > time we would like to use __atomic_thread_fence for portability reasons. > Add compiler barriers manually around the __atomic_thread_fence. > > Signed-off-by: Paolo Bonzini <[email protected]> > --- > include/qemu/atomic.h | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h > index 98e05ca..bd2c075 100644 > --- a/include/qemu/atomic.h > +++ b/include/qemu/atomic.h > @@ -99,7 +99,13 @@ > > #ifndef smp_wmb > #ifdef __ATOMIC_RELEASE > -#define smp_wmb() __atomic_thread_fence(__ATOMIC_RELEASE) > +/* __atomic_thread_fence does not include a compiler barrier; instead, > + * the barrier is part of __atomic_load/__atomic_store's "volatile-like" > + * semantics. If smp_wmb() is a no-op, absence of the barrier means that > + * the compiler is free to reorder stores on each side of the barrier. > + * Add one here, and similarly in smp_rmb() and smp_read_barrier_depends(). > + */ > +#define smp_wmb() ({ barrier(); __atomic_thread_fence(__ATOMIC_RELEASE); > barrier(); })
The comment says "add one" but the patch is adding two. An explanation of why you need a barrier on both sides and can't manage with just one might be helpful. thanks -- PMM
