BZ 20822 :powerpc: race condition in __lll_unlock_elision

[Florian Weimer]

On 11/22/2016 09:44 AM, Torvald Riegel wrote:

>> In this case (with the proposed patch) the adapt_count update is within
>> the critical region and adding C11 atomics does not add value and would
>> generally make the logical harder to read.
>>
>> __atomic_store_n (adapt_count, (__atomic_load_n(adapt_count,
>> __ATOMIC_RELAXED) - 1), __ATOMIC_RELAXED)
>>
>> Is a lot of syntax without any real value.
>
> First, it would actually be this in glibc:
> atomic_store_relaxed (&adapt_count,
>   atomic_load_relaxed(&adapt_count) - 1);
>
> Second, it does add value in that it makes it clear that there are
> concurrent accesses elsewhere that are not in transactions.  We have
> consensus to use the new C11-like atomics in new or changed code, and I
> don't see a reason to make an exception here.

A possible reason: The code does not actually follow the C11 memory 
model, so using C11-like atomics is rather misleading.  And unlike code 
using incorrect synchronization or legacy atomics, we don't have a path 
towards the C11 model because what the code does is completely outside 
it, so there's no argument on favor of gradual/partial conversion.

Florian