Add atomic operations required by the new condition variable.

  This adds just a few atomic operations required by the new condvar.  Our
policy is to add atomic operations as needed, and this patch does just
that.

Tested on x86-linux and x86_64-linux using the new condvar.

2016-05-26  Torvald Riegel  <triegel@redhat.com>

	* include/atomic.h (atomic_exchange_relaxed, atomic_fetch_and_relaxed,
	atomic_fetch_and_release, atomic_fetch_or_release,
	atomic_fetch_xor_release): New.
  

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.

(Ideally, our atomics and how to implement them should be documented in 
internals manual, or we should use GCC atomics directly so that we don't 
need our own documentation.  But that's an old and separate discussion. 
Using the GCC syntax would also avoid the unfortunate naming pattern, 
e.g. “atomic_fetch_or_release”.)

Apart from the XXX bits, the changes look okay to me.

Florian
  

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.

> (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?

> or we should use GCC atomics directly so that we don't 
> need our own documentation.

We do in cases where they are available and somebody has checked that it
works.  More arch-maintainers setting USE_ATOMIC_COMPILER_BUILTINS to 1
would be a good thing.

> But that's an old and separate discussion. 
> Using the GCC syntax would also avoid the unfortunate naming pattern, 
> e.g. “atomic_fetch_or_release”.)

Remember that this isn't just an arbitrarily chosen naming pattern, but
follows the C11 names with the difference that we always make the memory
order an explicit part of the operation's name.  IOW, if people know C11
atomics, it's *very* easy for them to use ours.  In the long run, more
developers will know C11 atomics than GCC atomic builtins.

There are two major reasons for including the memory order name:
1) We want developers to always think about which memory order is
necessary.
2) We don't want to clash with C11 function names (eg, if we should
choose to switch to C11 in the future).  But once we move to C11, the
switch will be simple and mechanical.
  

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).

>> (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.

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

>> But that's an old and separate discussion.
>> Using the GCC syntax would also avoid the unfortunate naming pattern,
>> e.g. “atomic_fetch_or_release”.)
>
> Remember that this isn't just an arbitrarily chosen naming pattern, but
> follows the C11 names with the difference that we always make the memory
> order an explicit part of the operation's name.  IOW, if people know C11
> atomics, it's *very* easy for them to use ours.  In the long run, more
> developers will know C11 atomics than GCC atomic builtins.

Sure, but it's unfortunate that “atomic_fetch_or_release” doesn't 
naturally parse as “atomic_fetch_OP_MO” because “or” can be conjunction, 
not just as a bitwise operation.

Thanks,
Florian
  

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?
  

On 06/14/2016 08:21 PM, Torvald Riegel wrote:

>> 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?

Maybe extract the list using sed from the relevant header(s) and put it 
into the internals manual?  If the naming convention is explained along 
the list, I don't think further per-macro documentation is necessary.

Florian
  

On Wed, 2016-06-15 at 09:09 +0200, Florian Weimer wrote:
> On 06/14/2016 08:21 PM, Torvald Riegel wrote:
> 
> >> 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?
> 
> Maybe extract the list using sed from the relevant header(s) and put it 
> into the internals manual?  If the naming convention is explained along 
> the list, I don't think further per-macro documentation is necessary.

That's perhaps too much machinery or things we need to remember to do
for something that's rather simple to see by looking at
include/atomic.h, if one knows where to look for it.

I've updated https://sourceware.org/glibc/wiki/Concurrency with more
details on the naming scheme and instructions for how to easily see
which atomic operations we currently declare.
  

On 06/15/2016 10:49 AM, Torvald Riegel wrote:
> I've updated https://sourceware.org/glibc/wiki/Concurrency with more
> details on the naming scheme and instructions for how to easily see
> which atomic operations we currently declare.

Thanks, this looks helpful!

Florian