Re: [PATCH] Add atomic operations required by the new condition variable.

[Torvald Riegel <triegel at redhat dot com>]

On Tue, 2016-06-14 at 16:43 +0200, Florian Weimer wrote:
> On 05/27/2016 01:16 PM, Torvald Riegel wrote:
> > On Fri, 2016-05-27 at 11:13 +0200, Florian Weimer wrote:
> >> On 05/26/2016 01:04 AM, Torvald Riegel wrote:
> >>> +# ifndef atomic_exchange_relaxed
> >>> +/* XXX This unnecessarily has acquire MO.  */
> >>
> >> I don't understand the use of the XXX marker here.  If there is a
> >> potential bug, this needs a longer explanation.  If this is just use of
> >> an unnecessarily strong memory order, a generic remark somewhere in the
> >> file that âarchitectures might override the following defines with
> >> relaxed MO implementation for improved performanceâ or something like
> >> that should address this, and that doesn't warrant an XXX marker.
> >
> > It's not a bug, but a less-than-ideal implementation.  What's the marker
> > for this then?  (Having no marker isn't ideal because then we can't grep
> > for this, for example.)
> >
> > Architectures should not do their arch-custom stuff in atomics if they
> > can.  Note that the XXX is on code in #if !USE_ATOMIC_COMPILER_BUILTINS
> > so it will go away eventually once arch maintainers confirm that the new
> > atomic builtins work on their machines.
> 
> I file bugs in such cases (when there is a specific task to be done).

The specific task is to avoid that we're having this issue in the first
case by letting archs set USE_ATOMIC_COMPILER_BUILTINS to 1 if possible.
Which we already know :)

> >> (Ideally, our atomics and how to implement them should be documented in
> >> internals manual,
> >
> > I've said it before and it still stands: C11 is the documentation for
> > the new-style atomics we use.  This is documented on the concurrency
> > page on the wiki.  What else is there that would need to be documented?
> 
> We recently had the conversation on IRC.  I find it difficult to map the 
> C11 atomics from the standard to the glibc implementation because C11 
> has more atomics, and glibc has the old atomics.

OK, that's something we can improve.

> Maybe an explicit list of all modern glibc atomics somewhere would help 
> to avoid all doubts.

I wouldn't like to maintain it on the wiki or such, but we could decide
to list the available atomics at the top of include/atomic.h, or
something like that.  Would that have been helpful for you?