Re: [PATCH] Implement allocate_once

[Florian Weimer <fweimer at redhat dot com>]

On 01/12/2018 12:47 PM, Torvald Riegel wrote:
> The most recent patch looks good to me.

Thanks!

> I have a minor comment for future patches:
>
> On Fri, 2018-01-12 at 10:46 +0100, Florian Weimer wrote:
> > +  while (true)
> > +    {
> > +      /* Synchronizes with the acquire MO load in allocate_once.  */
> > +      void *expected = NULL;
> > +      if (atomic_compare_exchange_weak_release (place, &expected, result))
> > +        return result;
> > +
> > +      /* The failed CAS has relaxed MO semantics, so perform another
> > +         acquire MO load.  */
> > +      void *other_result = atomic_load_acquire (place);
>
> You could just use expected here, because it has been updated, and ...
>
> > +      if (other_result == NULL)
> > +        /* Spurious failure.  Try again.  */
> > +        continue;
> > +
> > +      /* We lost the race.  Free what we allocated and return the
> > +         other result.  */
>
> ... add an atomic_thread_fence_acquire() here.

Isn't a an acquire MO load potentially cheaper than an acquire fence? 
The fence needs to synchronize with all threads, while the load only 
needs to synchronize with threads which have performed release MO store 
(or stronger) on the variable?

Florian