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

[Konstantin Belousov]

On Tue, Sep 18, 2012 at 11:29:31AM +0100, David Chisnall wrote:
> 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.
>

How can atomic_SIGNAL_fence be even remotely conceptually right for the
kernel execution environment ? The fact that current implementation
happens to offer similar guarantees is a coincidense.


> 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...
Agreed, but you did not looked at atomic.h. The memory clobbering is
only performed for atomics which do not need asm. The load acquire and
store release are implemented as the normal load and stores on x86, due
to existing architecture guarantees.

>
> The atomic operations are memory barriers themselves, although our
> versions are often much stronger barriers than are required. I would
Exactly which atomic operations for x86 do you reference there ?
Are you aware about e.g. AMD recommendations for the barrier/lock
prefix use (look into atomic.h) ?

> 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 196 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/svn-src-projects/attachments/20120918/1eea0612/attachment.pgp