This is the mail archive of the mailing list for the glibc project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: BZ 20822 :powerpc: race condition in __lll_unlock_elision

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,

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.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]