diff options
| author | Hans Boehm <boehm@acm.org> | 2016-08-15 11:32:33 +0300 |
|---|---|---|
| committer | Ivan Maidanski <ivmai@mail.ru> | 2016-08-15 11:32:33 +0300 |
| commit | 85dd735949d712fa5db4bde0f0fc74f15a624222 (patch) | |
| tree | eca7106b4a660a2f93e50ccce4912abf96d07894 /src | |
| parent | dbb03137bf0ed891c4ebe981fa4ae20ab0a3aaeb (diff) | |
| download | libatomic_ops-85dd735949d712fa5db4bde0f0fc74f15a624222.tar.gz | |
Fix store-load ordering in AO_stack_pop_explicit_aux_acquire (PowerPC)
Issue #15.
The core issue is that AO_stack_pop_explicit_aux_acquire really needs
to ensure that the store to the blacklist via
AO_compare_and_swap_acquire becomes visible before the load to check
the list head. This effectively needs store-load ordering.
Currently the only ordering here is imposed by the _acquire on the
compare_and_swap. On PowerPC that turns into an lwsync, which is too
weak to enforce store to load ordering.
This patch should fix the issue. But this is suboptimal on x86, and
we may want to make the fence conditional on "not x86", where the CAS
already includes sufficient ordering. (With C++11 atomics, this would
also be tricky and probably involve making a bunch of accesses seq_cst.)
* src/atomic_ops_stack.c [AO_USE_ALMOST_LOCK_FREE]
(AO_stack_pop_explicit_aux_acquire): Call AO_compare_and_swap instead
of AO_compare_and_swap_acquire; call AO_nop_full just before
(first != AO_load(list)).
Diffstat (limited to 'src')
| -rw-r--r-- | src/atomic_ops_stack.c | 3 |
1 files changed, 2 insertions, 1 deletions
diff --git a/src/atomic_ops_stack.c b/src/atomic_ops_stack.c index 642bac0..55e5711 100644 --- a/src/atomic_ops_stack.c +++ b/src/atomic_ops_stack.c @@ -133,7 +133,7 @@ AO_stack_pop_explicit_aux_acquire(volatile AO_t *list, AO_stack_aux * a) for (i = 0; ; ) { if (PRECHECK(a -> AO_stack_bl[i]) - AO_compare_and_swap_acquire(a->AO_stack_bl+i, 0, first)) + AO_compare_and_swap(a->AO_stack_bl+i, 0, first)) break; ++i; if ( i >= AO_BL_SIZE ) @@ -151,6 +151,7 @@ AO_stack_pop_explicit_aux_acquire(volatile AO_t *list, AO_stack_aux * a) /* We need to make sure that first is still the first entry on the */ /* list. Otherwise it's possible that a reinsertion of it was */ /* already started before we added the black list entry. */ + AO_nop_full(); /* TODO: Suboptimal on x86 */ # if defined(__alpha__) && (__GNUC__ == 4) if (first != AO_load(list)) /* Workaround __builtin_expect bug found in */ |