From 6278899a3849b7ee46b6460dff93e555e77d99a2 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Tue, 25 Aug 2020 22:42:27 +1000 Subject: [PATCH] Use memory_order_acq_rel in isc_refcount_decrement. While if (isc_refcount_decrement() == 1) { // memory_order_release isc_refcount_destroy(); // memory_order_acquire ... } is theoretically the most efficent in practice, using memory_order_acq_rel produces the same code on x86_64 and doesn't trigger tsan data races (which use a idealistic model) if isc_refcount_destroy() is not called immediately. In fact isc_refcount_destroy() could be removed if we didn't want to check for the count being 0 when isc_refcount_destroy() is called. https://stackoverflow.com/questions/49112732/memory-order-in-shared-pointer-destructor --- lib/isc/include/isc/atomic.h | 2 ++ lib/isc/include/isc/refcount.h | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/isc/include/isc/atomic.h b/lib/isc/include/isc/atomic.h index 34344e244b..420a947028 100644 --- a/lib/isc/include/isc/atomic.h +++ b/lib/isc/include/isc/atomic.h @@ -65,6 +65,8 @@ atomic_fetch_or_explicit((o), (v), memory_order_release) #define atomic_exchange_acq_rel(o, v) \ atomic_exchange_explicit((o), (v), memory_order_acq_rel) +#define atomic_fetch_sub_acq_rel(o, v) \ + atomic_fetch_sub_explicit((o), (v), memory_order_acq_rel) #define atomic_compare_exchange_weak_acq_rel(o, e, d) \ atomic_compare_exchange_weak_explicit( \ (o), (e), (d), memory_order_acq_rel, memory_order_acquire) diff --git a/lib/isc/include/isc/refcount.h b/lib/isc/include/isc/refcount.h index 74543d471f..6def63f8bd 100644 --- a/lib/isc/include/isc/refcount.h +++ b/lib/isc/include/isc/refcount.h @@ -118,7 +118,7 @@ isc_refcount_increment(isc_refcount_t *target) { static inline uint_fast32_t isc_refcount_decrement(isc_refcount_t *target) { uint_fast32_t __v; - __v = (uint_fast32_t)atomic_fetch_sub_release(target, 1); + __v = (uint_fast32_t)atomic_fetch_sub_acq_rel(target, 1); INSIST(__v > 0); return (__v); } @@ -127,7 +127,7 @@ isc_refcount_decrement(isc_refcount_t *target) { ({ \ /* cppcheck-suppress shadowVariable */ \ uint_fast32_t __v; \ - __v = atomic_fetch_sub_release(target, 1); \ + __v = atomic_fetch_sub_acq_rel(target, 1); \ INSIST(__v > 0); \ __v; \ })