Messages in this thread | | | Subject | Re: About compiler memory barrier for atomic_set/atomic_read on x86 | From | "Yin, Fengwei" <> | Date | Wed, 4 Sep 2019 07:38:36 +0800 |
| |
Hi Peter,
On 9/3/2019 10:06 PM, Peter Zijlstra wrote: > On Tue, Sep 03, 2019 at 09:23:41PM +0800, Yin, Fengwei wrote: >> Hi Peter, >> There is one question regarding following commit: >> >> commit 69d927bba39517d0980462efc051875b7f4db185 >> Author: Peter Zijlstra <peterz@infradead.org> >> Date: Wed Apr 24 13:38:23 2019 +0200 >> >> x86/atomic: Fix smp_mb__{before,after}_atomic() >> >> Recent probing at the Linux Kernel Memory Model uncovered a >> 'surprise'. Strongly ordered architectures where the atomic RmW >> primitive implies full memory ordering and >> smp_mb__{before,after}_atomic() are a simple barrier() (such as x86) >> >> This change made atomic RmW operations include compiler barrier. And made >> __smp_mb__before_atomic/__smp_mb__after_atomic not include compiler >> barrier any more for x86. >> >> We face the issue to handle atomic_set/atomic_read which is mapped to >> WRITE_ONCE/READ_ONCE on x86. These two functions don't include compiler >> barrier actually (if operator size is less than 8 bytes). >> >> Before the commit 69d927bba39517d0980462efc051875b7f4db185, we could use >> __smp_mb__before_atomic/__smp_mb__after_atomic together with these two >> functions to make sure the memory order. It can't work after the commit >> 69d927bba39517d0980462efc051875b7f4db185. I am wandering whether >> we should make atomic_set/atomic_read also include compiler memory >> barrier on x86? Thanks. > > No; using smp_mb__{before,after}_atomic() with atomic_{set,read}() is > _wrong_! And it is documented as such; see Documentation/atomic_t.txt.
Thanks a lot for direct me to this doc. And yes, from this doc: - smp_mb__{before,after}_atomic() only apply to the RMW atomic ops - non-RMW operations are unordered;
I checked the /Documentation/memory-barriers.txt too. In section "COMPILER BARRIER", "However, READ_ONCE() and WRITE_ONCE() can be thought of as weak forms of barrier() that affect only the specific accesses flagged by the READ_ONCE() or WRITE_ONCE()".
For x86 READ_ONCE/WRITE_ONCE doesn't have compiler barrier if the operator size is less than 8 bytes. Should we update x86 code?
So, if I use atomic_set/read, to prevent the compiler from moving memory access around, I should use compiler barrier explicitly. Right?
Regards Yin, Fengwei
>
| |