Re: [Qemu-devel] [PATCH 5/5] atomic: base mb_read/mb_set on load-acquire and store-release

[Paolo Bonzini]

On 12/10/2016 11:28, Alex Bennée wrote:
> 
> Paolo Bonzini <address@hidden> writes:
> 
>> This introduces load-acquire and store-release operations in QEMU.
>> For now, just use them as an implementation detail of atomic_mb_read
>> and atomic_mb_set.
>>
>> Since docs/atomics.txt documents that atomic_mb_read only synchronizes
>> with an atomic_mb_set of the same variable, we can use the new implementation
>> everywhere instead of seq-cst loads and stores.
>>
>> Signed-off-by: Paolo Bonzini <address@hidden>
>> ---
> <snip>
> 
>> +/* This is more efficient than a store plus a fence.  */
>> +#if defined(__i386__) || defined(__x86_64__) || defined(__s390x__)
>> +#define atomic_mb_set(ptr, i)  ((void)atomic_xchg(ptr, i))
>> +#endif
> 
> Is this working around a compiler issue? Shouldn't it already be using
> the best instructions for the constraint?

Store release + memory barrier can be compiled into a seqcst store, and
is more efficient if you have a single instruction for that purpose, but
I don't know of any compiler that does it.

Paolo

>> +
>> +#ifndef atomic_mb_read
>> +#define atomic_mb_read(ptr)                             \
>> +    atomic_load_acquire(ptr)
>> +#endif
>> +
>> +#ifndef atomic_mb_set
>> +#define atomic_mb_set(ptr, i)  do {                     \
>> +    atomic_store_release(ptr, i);                       \
>> +    smp_mb();                                           \
>> +} while(0)
>> +#endif
>> +
>>  #endif /* QEMU_ATOMIC_H */
> 
> 
> --
> Alex Bennée
>