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

[Carlos O'Donell]

On 08/20/2015 03:51 PM, 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 withdraw my suggestion.

I can't come up with a strong enough argument that isn't already solved by
a high quality comment about how the macro works.

Cheers,
Carlos.