linux-kernel - Re: About compiler memory barrier for atomic_set/atomic

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <a9ea1348-b7cb-c589-1be4-51efadaf1271@intel.com>
Date:   Wed, 4 Sep 2019 07:38:36 +0800
From:   "Yin, Fengwei" <fengwei.yin@...el.com>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     linux-kernel@...r.kernel.org, "He, Min" <min.he@...el.com>,
        "Zhao, Yakui" <yakui.zhao@...el.com>
Subject: Re: About compiler memory barrier for atomic_set/atomic_read on x86

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@...radead.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

>