[PATCH 2/4] Add atomic operations similar to those provided by C11.

[Joseph S. Myers]

On Thu, 30 Oct 2014, Torvald Riegel wrote:

> On Wed, 2014-10-29 at 23:06 +0000, Joseph S. Myers wrote:
> > On Wed, 29 Oct 2014, Torvald Riegel wrote:
> > 
> > > First, do you agree that we need to make the compiler aware of
> > > concurrency?  For example, it would be bad if the compiler assumes that
> > > it can safely reload from an atomic variable just because it was able to
> > > prove that the loading thread didn't change it in the meantime.
> > 
> > I believe that the code already makes it explicit where such reloading 
> > would be problematic,
> 
> Why do you think this is the case?  There is an atomic_forced_read, for
> example, but no atomic_read_exactly_once.

See for example elf/dl-lookup.c:do_lookup_x, using an asm to ensure reads 
happen in a particular order:

  size_t n = scope->r_nlist;
  /* Make sure we read the value before proceeding.  Otherwise we
     might use r_list pointing to the initial scope and r_nlist being
     the value after a resize.  That is the only path in dl-open.c not
     protected by GSCOPE.  A read barrier here might be to expensive.  */
  __asm volatile ("" : "+r" (n), "+m" (scope->r_list));
  struct link_map **list = scope->r_list;

(There are lots of cases of asms being used - via macros - to control 
exactly when floating-point evaluations take place and associated 
exceptions are raised, but that's not relevant to atomics.  In general, 
any asm of an empty string is probably doing something like this.)

> > either (a) using asm with appropriate clobbers so 
> > the compiler doesn't know the value is unchanged
> 
> Do you have any examples for this where the clobber is not due to a
> compiler barrier used for acquire or release fences?

I don't understand the question.

> > or (b) using volatile.
> 
> But volatile does not guarantee atomicity.  I also think that current

The point is to guarantee ordering (with the atomicity of aligned word 
loads / stores being part of the GNU C language semantics required for 
building glibc).

> But I also see no real evidence that using atomics for relaxed
> loads/stores would hurt at all.  The sanitizer use case in the BZ you
> cited is arguably special.  The pthread_once case didn't show any
> difference on the fast path.  I can test with a __atomic*-providing
> compiler with and without using __atomic to look for differences, if
> that would help you.

Yes.  We may find in particular cases (possibly all of them) that the 
change does not have significant code generation consequences, but given 
the known bug we should actually check that in each case rather than 
presuming it.

> Can we at least agree on having all glibc code use our own
> atomic_store_relaxed / atomic_load_relaxed (see the patch)?  Then we can
> still argue whether to use __atomic* to implement these.  But at least
> we can easily switch to __atomic in the future.  Any objections?

I think explicit atomic loads / stores are fine, subject to checking code 
generation before making them use __atomic_*.

-- 
Joseph S. Myers
joseph@codesourcery.com