This is the mail archive of the
mailing list for the glibc project.
Re: BZ 20822 :powerpc: race condition in __lll_unlock_elision
- From: Florian Weimer <fweimer at redhat dot com>
- To: Torvald Riegel <triegel at redhat dot com>, munroesj at linux dot vnet dot ibm dot com
- Cc: Rajalakshmi Srinivasaraghavan <raji at linux dot vnet dot ibm dot com>, libc-alpha at sourceware dot org, aaron Sawdey <acsawdey at linux dot vnet dot ibm dot com>, Ulrich Weigand <Ulrich dot Weigand at de dot ibm dot com>, Steve Munroe <sjmunroe at us dot ibm dot co>, carlos at redhat dot com, adhemerval dot zanella at linaro dot org, adconrad at ubuntu dot com, wschmidt at linux dot vnet dot ibm dot com
- Date: Tue, 22 Nov 2016 09:55:29 +0100
- Subject: Re: BZ 20822 :powerpc: race condition in __lll_unlock_elision
- Authentication-results: sourceware.org; auth=none
- References: <email@example.com> <firstname.lastname@example.org> <1479771736.9880.42.camel@oc7878010663> <email@example.com>
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_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.