mirror of https://github.com/facebook/rocksdb.git
Fix TSAN reports on AutoHCC (#11792)
Summary: Forgot to run TSAN test on latest revision of https://github.com/facebook/rocksdb/issues/11738 Pull Request resolved: https://github.com/facebook/rocksdb/pull/11792 Test Plan: Use cache_bench to reproduce TSAN errors and observe fix Reviewed By: ajkr Differential Revision: D48953196 Pulled By: pdillinger fbshipit-source-id: 9e358b4768d8ddde86f84b451863263f661d7b80
This commit is contained in:
parent
f66df58b9e
commit
d01b1215bd
|
@ -3091,16 +3091,11 @@ AutoHyperClockTable::HandleImpl* AutoHyperClockTable::Lookup(
|
||||||
// false positive, which is re-checked after acquiring read ref, or false
|
// false positive, which is re-checked after acquiring read ref, or false
|
||||||
// negative, which is re-checked in the full Lookup.
|
// negative, which is re-checked in the full Lookup.
|
||||||
|
|
||||||
// We need to make the reads relaxed atomic to avoid TSAN reporting
|
// ReadAllowRace suppresses TSAN report on these reads. Also, using
|
||||||
// race conditions. And we can skip the cheap key match optimization
|
// & rather than && to give more flexibility to the compiler and CPU,
|
||||||
// altogether if 64-bit atomics not supported lock-free. Also, using
|
// as it is safe to access [1] even if [0] doesn't match.
|
||||||
// & rather than && to give more flexibility to the compiler and CPU.
|
if ((int{ReadAllowRace(h->hashed_key[0]) == hashed_key[0]} &
|
||||||
if (!std::atomic<uint64_t>::is_always_lock_free ||
|
int{ReadAllowRace(h->hashed_key[1]) == hashed_key[1]})) {
|
||||||
sizeof(std::atomic<uint64_t>) != sizeof(uint64_t) ||
|
|
||||||
(int{reinterpret_cast<std::atomic<uint64_t>&>(h->hashed_key[0])
|
|
||||||
.load(std::memory_order_relaxed) == hashed_key[0]} &
|
|
||||||
int{reinterpret_cast<std::atomic<uint64_t>&>(h->hashed_key[1])
|
|
||||||
.load(std::memory_order_relaxed) == hashed_key[1]})) {
|
|
||||||
// Increment acquire counter for definitive check
|
// Increment acquire counter for definitive check
|
||||||
uint64_t old_meta = h->meta.fetch_add(ClockHandle::kAcquireIncrement,
|
uint64_t old_meta = h->meta.fetch_add(ClockHandle::kAcquireIncrement,
|
||||||
std::memory_order_acquire);
|
std::memory_order_acquire);
|
||||||
|
|
|
@ -69,6 +69,13 @@ constexpr bool kMustFreeHeapAllocations = false;
|
||||||
#define TSAN_SUPPRESSION
|
#define TSAN_SUPPRESSION
|
||||||
#endif // TSAN_SUPPRESSION
|
#endif // TSAN_SUPPRESSION
|
||||||
|
|
||||||
|
// Read memory while allowing data races. Only use where it is OK to read
|
||||||
|
// the wrong value, e.g. where reading the latest value improves performance.
|
||||||
|
template <typename T>
|
||||||
|
TSAN_SUPPRESSION inline T ReadAllowRace(const T& v) {
|
||||||
|
return v;
|
||||||
|
}
|
||||||
|
|
||||||
// Compile-time CPU feature testing compatibility
|
// Compile-time CPU feature testing compatibility
|
||||||
//
|
//
|
||||||
// A way to be extra sure these defines have been included.
|
// A way to be extra sure these defines have been included.
|
||||||
|
|
Loading…
Reference in New Issue