Re: About compiler memory barrier for atomic_set/atomic_read on x86

[Yin, Fengwei]

Hi Peter,

On 9/4/2019 7:38 AM, Yin, Fengwei wrote:
> 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@xxxxxxxxxxxxx>
> > > 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?
It looks like atomic_set_release/read_acquire could be used in my case.

Regards
Yin, Fengwei

> Regards
> Yin, Fengwei
>
> >