| Message ID | 1eeffe37-c4ec-fef7-537b-9ad17129b6c0@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Mon, 2016-10-17 at 23:18 -0400, Carlos O'Donell wrote: > Torvald, > > No code uses atomic_fetch_xor_release except for the upcoming conditional > variable rewrite. Therefore there is no user visible bug here. > > The use of atomic_compare_and_exchange_bool_rel is removed (since it doesn't > exist anymore), and is replaced by atomic_compare_exchange_weak_release. > > We use weak_release because it provides better performance in the loop > (the weak semantic) and because the xor is release MO (the release semantic). > > We don't reload expected in the loop because atomic_compare_and_exchange_weak_release > does this for us as part of the CAS failure. > > It is otherwise a fairly plain conversion that fixes building the new condvar > for 32-bit x86. > > I have pushed the new condvar into Fedora Rawhide for testing. > > OK to checkin? OK.
On 10/18/2016 07:02 AM, Torvald Riegel wrote: > On Mon, 2016-10-17 at 23:18 -0400, Carlos O'Donell wrote: >> Torvald, >> >> No code uses atomic_fetch_xor_release except for the upcoming conditional >> variable rewrite. Therefore there is no user visible bug here. >> >> The use of atomic_compare_and_exchange_bool_rel is removed (since it doesn't >> exist anymore), and is replaced by atomic_compare_exchange_weak_release. >> >> We use weak_release because it provides better performance in the loop >> (the weak semantic) and because the xor is release MO (the release semantic). >> >> We don't reload expected in the loop because atomic_compare_and_exchange_weak_release >> does this for us as part of the CAS failure. >> >> It is otherwise a fairly plain conversion that fixes building the new condvar >> for 32-bit x86. >> >> I have pushed the new condvar into Fedora Rawhide for testing. >> >> OK to checkin? > > OK. > Checked in. Verified it fixes everything with the other dependent patches applied.
Index: glibc-2.24-256-g5140d03/include/atomic.h =================================================================== --- glibc-2.24-256-g5140d03.orig/include/atomic.h +++ glibc-2.24-256-g5140d03/include/atomic.h @@ -777,18 +777,22 @@ void __atomic_link_error (void); # endif # ifndef atomic_fetch_xor_release +/* Failing the atomic_compare_exchange_weak_release reloads the value in + __atg104_expected, so we need only do the XOR again and retry. */ # define atomic_fetch_xor_release(mem, operand) \ - ({ __typeof (*(mem)) __atg104_old; \ - __typeof (mem) __atg104_memp = (mem); \ + ({ __typeof (mem) __atg104_memp = (mem); \ + __typeof (*(mem)) __atg104_expected = (*__atg104_memp); \ + __typeof (*(mem)) __atg104_desired; \ __typeof (*(mem)) __atg104_op = (operand); \ \ do \ - __atg104_old = (*__atg104_memp); \ - while (__builtin_expect \ - (atomic_compare_and_exchange_bool_rel ( \ - __atg104_memp, __atg104_old ^ __atg104_op, __atg104_old), 0));\ + __atg104_desired = __atg104_expected ^ __atg104_op; \ + while (__glibc_unlikely \ + (atomic_compare_exchange_weak_release ( \ + __atg104_memp, &__atg104_expected, __atg104_desired) \ + == 0)); \ \ - __atg104_old; }) + __atg104_expected; }) #endif #endif /* !USE_ATOMIC_COMPILER_BUILTINS */