[compiler-rt] r203112 - tsan: weaken concurrency guarantees in deadlock detector mutex hashmap
Dmitry Vyukov
dvyukov at google.com
Thu Mar 6 04:04:27 PST 2014
Author: dvyukov
Date: Thu Mar 6 06:04:26 2014
New Revision: 203112
URL: http://llvm.org/viewvc/llvm-project?rev=203112&view=rev
Log:
tsan: weaken concurrency guarantees in deadlock detector mutex hashmap
read locking on every access is too expensive
Modified:
compiler-rt/trunk/lib/sanitizer_common/sanitizer_addrhashmap.h
compiler-rt/trunk/lib/sanitizer_common/sanitizer_mutex.h
Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_addrhashmap.h
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_addrhashmap.h?rev=203112&r1=203111&r2=203112&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_addrhashmap.h (original)
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_addrhashmap.h Thu Mar 6 06:04:26 2014
@@ -44,7 +44,7 @@ template<typename T, uptr kSize>
class AddrHashMap {
private:
struct Cell {
- RWMutex mtx;
+ StaticSpinMutex mtx;
atomic_uintptr_t addr;
T val;
};
@@ -66,17 +66,17 @@ class AddrHashMap {
uptr addr_;
bool created_;
bool remove_;
- bool write_;
};
private:
friend class Handle;
Cell *table_;
- static const uptr kRemoved = 1;
+ static const uptr kLocked = 1;
+ static const uptr kRemoved = 2;
- Cell *acquire(uptr addr, bool remove, bool *created, bool *write);
- void release(uptr addr, bool remove, bool write, Cell *c);
+ Cell *acquire(uptr addr, bool remove, bool *created);
+ void release(uptr addr, bool remove, bool created, Cell *c);
uptr hash(uptr addr);
};
@@ -86,12 +86,12 @@ AddrHashMap<T, kSize>::Handle::Handle(Ad
map_ = map;
addr_ = addr;
remove_ = remove;
- cell_ = map_->acquire(addr_, remove_, &created_, &write_);
+ cell_ = map_->acquire(addr_, remove_, &created_);
}
template<typename T, uptr kSize>
AddrHashMap<T, kSize>::Handle::~Handle() {
- map_->release(addr_, remove_, write_, cell_);
+ map_->release(addr_, remove_, created_, cell_);
}
template<typename T, uptr kSize>
@@ -116,7 +116,7 @@ AddrHashMap<T, kSize>::AddrHashMap() {
template<typename T, uptr kSize>
typename AddrHashMap<T, kSize>::Cell *AddrHashMap<T, kSize>::acquire(uptr addr,
- bool remove, bool *created, bool *write) {
+ bool remove, bool *created) {
// When we access the element associated with addr,
// we lock its home cell (the cell associated with hash(addr).
// If the element was just created or is going to be removed,
@@ -126,19 +126,21 @@ typename AddrHashMap<T, kSize>::Cell *Ad
// inserts.
// Note that the home cell is not necessary the cell where the element is
// stored.
- *write = false;
*created = false;
uptr h0 = hash(addr);
Cell *c0 = &table_[h0];
- // First try to find an existing element under read lock.
- if (!remove) {
- c0->mtx.ReadLock();
+ // First try to find an existing element w/o read mutex.
+ {
uptr h = h0;
for (;;) {
Cell *c = &table_[h];
uptr addr1 = atomic_load(&c->addr, memory_order_acquire);
if (addr1 == 0) // empty cell denotes end of the cell chain for the elem
break;
+ // Locked cell means that another thread can be concurrently inserting
+ // the same element, fallback to mutex.
+ if (addr1 == kLocked)
+ break;
if (addr1 == addr) // ok, found it
return c;
h++;
@@ -146,10 +148,10 @@ typename AddrHashMap<T, kSize>::Cell *Ad
h = 0;
CHECK_NE(h, h0); // made the full cycle
}
- c0->mtx.ReadUnlock();
}
- // Now try to create it under write lock.
- *write = true;
+ if (remove)
+ return 0;
+ // Now try to create it under the mutex.
c0->mtx.Lock();
uptr h = h0;
for (;;) {
@@ -157,12 +159,9 @@ typename AddrHashMap<T, kSize>::Cell *Ad
uptr addr1 = atomic_load(&c->addr, memory_order_acquire);
if (addr1 == addr) // another thread has inserted it ahead of us
return c;
- if (remove && addr1 == 0) { // the element has not existed before
- c0->mtx.Unlock();
- return 0;
- }
- if (!remove && (addr1 == 0 ||addr1 == kRemoved) &&
- atomic_compare_exchange_strong(&c->addr, &addr1, addr,
+ // Skip kLocked, since we hold the home cell mutex, it can't be our elem.
+ if ((addr1 == 0 || addr1 == kRemoved) &&
+ atomic_compare_exchange_strong(&c->addr, &addr1, kLocked,
memory_order_acq_rel)) {
// we've created the element
*created = true;
@@ -176,23 +175,26 @@ typename AddrHashMap<T, kSize>::Cell *Ad
}
template<typename T, uptr kSize>
-void AddrHashMap<T, kSize>::release(uptr addr, bool remove, bool write,
+void AddrHashMap<T, kSize>::release(uptr addr, bool remove, bool created,
Cell *c) {
if (c == 0)
return;
// if we are going to remove, we must hold write lock
- CHECK_EQ(!remove || write, true);
- CHECK_EQ(atomic_load(&c->addr, memory_order_relaxed), addr);
- // denote that the cell is empty now
- if (remove)
- atomic_store(&c->addr, kRemoved, memory_order_release);
- // unlock the home cell
- uptr h0 = hash(addr);
- Cell *c0 = &table_[h0];
- if (write)
+ uptr addr1 = atomic_load(&c->addr, memory_order_relaxed);
+ if (created) {
+ // denote completion of insertion
+ atomic_store(&c->addr, addr, memory_order_release);
+ // unlock the home cell
+ uptr h0 = hash(addr);
+ Cell *c0 = &table_[h0];
c0->mtx.Unlock();
- else
- c0->mtx.ReadUnlock();
+ } else {
+ CHECK_EQ(addr, addr1);
+ if (remove) {
+ // denote that the cell is empty now
+ atomic_store(&c->addr, kRemoved, memory_order_release);
+ }
+ }
}
template<typename T, uptr kSize>
Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_mutex.h
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_mutex.h?rev=203112&r1=203111&r2=203112&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_mutex.h (original)
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_mutex.h Thu Mar 6 06:04:26 2014
@@ -83,88 +83,6 @@ class BlockingMutex {
uptr owner_; // for debugging
};
-// Reader-writer spin mutex.
-class RWMutex {
- public:
- RWMutex() {
- atomic_store(&state_, kUnlocked, memory_order_relaxed);
- }
-
- ~RWMutex() {
- CHECK_EQ(atomic_load(&state_, memory_order_relaxed), kUnlocked);
- }
-
- void Lock() {
- u32 cmp = kUnlocked;
- if (atomic_compare_exchange_strong(&state_, &cmp, kWriteLock,
- memory_order_acquire))
- return;
- LockSlow();
- }
-
- void Unlock() {
- u32 prev = atomic_fetch_sub(&state_, kWriteLock, memory_order_release);
- DCHECK_NE(prev & kWriteLock, 0);
- (void)prev;
- }
-
- void ReadLock() {
- u32 prev = atomic_fetch_add(&state_, kReadLock, memory_order_acquire);
- if ((prev & kWriteLock) == 0)
- return;
- ReadLockSlow();
- }
-
- void ReadUnlock() {
- u32 prev = atomic_fetch_sub(&state_, kReadLock, memory_order_release);
- DCHECK_EQ(prev & kWriteLock, 0);
- DCHECK_GT(prev & ~kWriteLock, 0);
- (void)prev;
- }
-
- void CheckLocked() {
- CHECK_NE(atomic_load(&state_, memory_order_relaxed), kUnlocked);
- }
-
- private:
- atomic_uint32_t state_;
-
- enum {
- kUnlocked = 0,
- kWriteLock = 1,
- kReadLock = 2
- };
-
- void NOINLINE LockSlow() {
- for (int i = 0;; i++) {
- if (i < 10)
- proc_yield(10);
- else
- internal_sched_yield();
- u32 cmp = atomic_load(&state_, memory_order_relaxed);
- if (cmp == kUnlocked &&
- atomic_compare_exchange_weak(&state_, &cmp, kWriteLock,
- memory_order_acquire))
- return;
- }
- }
-
- void NOINLINE ReadLockSlow() {
- for (int i = 0;; i++) {
- if (i < 10)
- proc_yield(10);
- else
- internal_sched_yield();
- u32 prev = atomic_load(&state_, memory_order_acquire);
- if ((prev & kWriteLock) == 0)
- return;
- }
- }
-
- RWMutex(const RWMutex&);
- void operator = (const RWMutex&);
-};
-
template<typename MutexType>
class GenericScopedLock {
public:
@@ -205,8 +123,6 @@ class GenericScopedReadLock {
typedef GenericScopedLock<StaticSpinMutex> SpinMutexLock;
typedef GenericScopedLock<BlockingMutex> BlockingMutexLock;
-typedef GenericScopedLock<RWMutex> RWMutexLock;
-typedef GenericScopedReadLock<RWMutex> RWMutexReadLock;
} // namespace __sanitizer
More information about the llvm-commits
mailing list