[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC PATCH 3/4] include/qemu/atomic.h: default to __ato
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [RFC PATCH 3/4] include/qemu/atomic.h: default to __atomic functions |
Date: |
Mon, 25 Jan 2016 23:02:59 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.0 |
On 25/01/2016 19:23, Alex Bennée wrote:
>>> + })
>>> +
>>> +#define atomic_mb_set(ptr, i) ((void)atomic_xchg(ptr, i))
>>> +#define atomic_mb_read(ptr) \
>>> + ({ \
>>> + typeof(*ptr) _val; \
>>> + __atomic_load(ptr, &_val, __ATOMIC_ACQUIRE); \
>>> + _val; \
>>> + })
>>
>> Please leave these defined in terms of relaxed accesses and memory
>> barriers.
>
> May I ask why? Doesn't the __ATOMIC_ACQUIRE ensure the load barrier is
> in place? Or should it be a fill CST barrier?
First, because they are defined like this in docs/atomics.txt. :)
Second, because the right definition would be
__atomic_load(__ATOMIC_SEQ_CST) and __atomic_store(__ATOMIC_SEQ_CST).
These produce even better code than the memory barriers on x86 but,
unfortunately, on POWER they are implemented in such a way that make
things very slow.
Basically, instead of
mb_read -> load; rmb()
mb_set -> wmb(); store(); mb()
POWER uses this:
mb_read -> load; mb()
mb_set -> wmb(); store(); mb()
There are reasons for these, and they are proved in
http://www.cl.cam.ac.uk/~pes20/cppppc/popl079-batty.pdf (I'm not
pretending I understand this paper except inasmuch as it was necessary
to write docs/atomics.txt). However, the cases that actually matter
basically never arise. Again, this is documented in docs/atomics.txt
around line 90.
As an alternative to explicit memory barriers, you can use this on POWER:
mb_read -> load-acquire
mb_set -> store-release + mb()
and seq-cst load/store everywhere else.
Paolo
[Qemu-devel] [RFC PATCH 3/4] include/qemu/atomic.h: default to __atomic functions, Alex Bennée, 2016/01/25
[Qemu-devel] [RFC PATCH 4/4] tsan: various fixes for make check, Alex Bennée, 2016/01/25