[PATCH v2] PowerPC: Fix a race condition when eliding a lock

[Torvald Riegel]

On Thu, 2015-08-20 at 16:51 -0300, Adhemerval Zanella wrote:
> 
> 
> >> +
> >> +/* Returns 0 if the lock defined by is_lock_free was elided.
> >> +   ADAPT_COUNT is a per-lock state variable.  */
> >> +# define ELIDE_LOCK(adapt_count, is_lock_free)				\
> >> +  ({									\
> >> +    int ret = 0;							\
> >> +    if (adapt_count > 0)						\
> >> +      (adapt_count)--;							\
> >> +    else								\
> >> +      for (int i = __elision_aconf.try_tbegin; i > 0; i--)		\
> >> +	{								\
> >> +	  if (__builtin_tbegin (0))					\
> >> +	    {								\
> >> +	      if (is_lock_free)						\
> > 
> > This should use a relaxed MO atomic access to highlight that this data *is*
> > shared between threads, but that there are no ordering guarantees. It should
> > result in a normal load, but it makes it clear in the code that some other
> > thread is also accessing this data, and that the transaction as a barrier
> > is all that is protecting this from being a data-race (undefined behaviour).
> 
> Should we really highlight it using atomic operations? The idea is exactly
> that any access within a transaction will not result in undefined behavior,
> so no need to atomic for synchronization.

I think it's better to highlight it.  If the data would only be accessed
from within transaction, the highlighting wouldn't be necessary.  But it
is mixed transactional/nontransactional synchronization in this case, so
we need the annotation.

This also makes moving to proper atomic types easier should we ever
decide to do that.