svn commit: r238907 - projects/calloutng/sys/kern

[David Chisnall]

On 18 Sep 2012, at 11:15, Dimitry Andric wrote:

> Please use gcc's __sync_synchronize() builtin[1] instead, which is
> specifically for this purpose.  Clang also supports it.
> 
> The builtin will emit actual memory barrier instructions, if the target
> architecture supports it, otherwise it will emit the same asm statement
> you show above.  See contrib/gcc/builtins.c, around line 5584, function
> expand_builtin_synchronize().

From Attilio's description of the problem in IRC, I believe that atomic_signal_fence() is the correct thing to use here.  He stated that he cares about reordering of memory access with regard to the current CPU, but not with regard to other CPUs / threads.  He also said that he only cares about the compiler performing the reordering, not about the CPU, but I suspect that is incorrect as there are numerous subtle bugs that can creep in on weakly-ordered architectures (e.g. Alpha, ARMv8) if you only have a compiler barrier.

That said, this is likely to be incorrect, because it's very unusual for that to actually be the requirement, especially in multithreaded code (where the atomic.h stuff is actually important).  In most of the cases where __compiler_membar() is being used, you actually want at least a partial barrier.  

On 18 Sep 2012, at 11:22, Konstantin Belousov wrote:

> We do not need CPU barriers there, which are already handled by the atomic
> asms. It is only to prevent compiler from exploiting the reorder.

If the atomic asm does not state that it clobbers memory, then it is a bug.  If it does clobber memory, then following it with an empty asm statement that also clobbers memory is redundant.  Looking in atomic.h on amd64, they all do already clobber memory...

The atomic operations are memory barriers themselves, although our versions are often much stronger barriers than are required.  I would like to see us slowly deprecate atomic.h in favour of stdatomic.h, which has three significant advantages:

1) It's part of the C standard
2) It provides a well-defined set of barrier types for every operation
3) It's implemented with compiler assistance (including GCC 4.2.1 support in our current version, although that version always enforces sequentially consistent ordering) and allows the compiler more freedom to perform safe optimisations around it

David