From 330f5e5fd9cb7b945170177df4bad14853b39b67 Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Tue, 4 Nov 2008 11:45:22 -0800 Subject: [PATCH] lockdep: separate from Mutex; include checks for RWLock --- src/Makefile.am | 3 +- src/common/Mutex.h | 19 ++-- src/common/RWLock.h | 11 +- src/common/lockdep.cc | 192 +++++++++++++++++++++++++++++++++ src/common/lockdep.h | 11 ++ src/os/JournalingObjectStore.h | 1 + 6 files changed, 229 insertions(+), 8 deletions(-) create mode 100644 src/common/lockdep.cc create mode 100644 src/common/lockdep.h diff --git a/src/Makefile.am b/src/Makefile.am index 3fc0ccef47da3..dbb11742514c7 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -167,7 +167,7 @@ libcommon_a_SOURCES = \ msg/Message.cc \ common/Logger.cc \ common/Clock.cc \ - common/Mutex.cc \ + common/lockdep.cc \ common/Timer.cc \ common/Finisher.cc \ common/sctp_crc32.c\ @@ -254,6 +254,7 @@ noinst_HEADERS = \ client/fuse.h\ client/fuse_ll.h\ client/hadoop/CephFSInterface.h\ + common/lockdep.h\ common/BackTrace.h\ common/Clock.h\ common/Cond.h\ diff --git a/src/common/Mutex.h b/src/common/Mutex.h index 08eb63d4c9253..a6490cc975192 100755 --- a/src/common/Mutex.h +++ b/src/common/Mutex.h @@ -17,11 +17,10 @@ #include #include "include/assert.h" +#include "lockdep.h" #define LOCKDEP -extern int g_lockdep; - class Mutex { private: @@ -38,10 +37,18 @@ private: Mutex( const Mutex &M ) {} #ifdef LOCKDEP - void _register(); - void _will_lock(); // about to lock - void _locked(); // just locked - void _unlocked(); // just unlocked + void _register() { + id = lockdep_register(name); + } + void _will_lock() { // about to lock + id = lockdep_will_lock(name, id); + } + void _locked() { // just locked + id = lockdep_locked(name, id); + } + void _unlocked() { // just unlocked + id = lockdep_unlocked(name, id); + } #else void _register() {} void _will_lock() {} // about to lock diff --git a/src/common/RWLock.h b/src/common/RWLock.h index 14e158a64ab97..d6e0da733f5b8 100644 --- a/src/common/RWLock.h +++ b/src/common/RWLock.h @@ -18,15 +18,19 @@ #define _RWLock_Posix_ #include +#include "lockdep.h" class RWLock { mutable pthread_rwlock_t L; + const char *name; + int id; public: - RWLock() { + RWLock(const char *n) : name(n), id(-1) { pthread_rwlock_init(&L, NULL); + if (g_lockdep) id = lockdep_register(name); } virtual ~RWLock() { @@ -35,14 +39,19 @@ class RWLock } void unlock() { + if (g_lockdep) id = lockdep_unlocked(name, id); pthread_rwlock_unlock(&L); } void get_read() { + if (g_lockdep) id = lockdep_will_lock(name, id); pthread_rwlock_rdlock(&L); + if (g_lockdep) id = lockdep_locked(name, id); } void put_read() { unlock(); } void get_write() { + if (g_lockdep) id = lockdep_will_lock(name, id); pthread_rwlock_wrlock(&L); + if (g_lockdep) id = lockdep_locked(name, id); } void put_write() { unlock(); } }; diff --git a/src/common/lockdep.cc b/src/common/lockdep.cc new file mode 100644 index 0000000000000..db9069968920f --- /dev/null +++ b/src/common/lockdep.cc @@ -0,0 +1,192 @@ + +#include "lockdep.h" + +int g_lockdep = 0; + +#include "include/types.h" +#include "Clock.h" +#include "BackTrace.h" + +#include + +#include "config.h" + +#define dout(l) if (l<=g_conf.debug_lockdep) *_dout << g_clock.now() << " " << pthread_self() << " lockdep: " +#define derr(l) if (l<=g_conf.debug_lockdep) *_derr << g_clock.now() << " " << pthread_self() << " lockdep: " + + +pthread_mutex_t lockdep_mutex = PTHREAD_MUTEX_INITIALIZER; + + +#define MAX_LOCKS 100 // increase me as needed + +hash_map lock_ids; +map lock_names; + +int last_id = 0; + +hash_map > held; +BackTrace *follows[MAX_LOCKS][MAX_LOCKS]; // follows[a][b] means b taken after a + + +#define BACKTRACE_SKIP 3 + + +int lockdep_register(const char *name) +{ + int id; + + pthread_mutex_lock(&lockdep_mutex); + + if (last_id == 0) + for (int i=0; i::iterator p = lock_ids.find(name); + if (p == lock_ids.end()) { + assert(last_id < MAX_LOCKS); + id = last_id++; + lock_ids[name] = id; + lock_names[id] = name; + dout(10) << "registered '" << name << "' as " << id << std::endl; + } else { + id = p->second; + dout(20) << "had '" << name << "' as " << id << std::endl; + } + + pthread_mutex_unlock(&lockdep_mutex); + + return id; +} + + +// does a follow b? +static bool does_follow(int a, int b) +{ + if (follows[a][b]) { + *_dout << std::endl; + dout(0) << "------------------------------------" << std::endl; + dout(0) << "existing dependency " << lock_names[a] << " (" << a << ") -> " << lock_names[b] << " (" << b << ") at: " << std::endl; + follows[a][b]->print(*_dout); + *_dout << std::endl; + return true; + } + + for (int i=0; i " << lock_names[i] << " (" << i << ") at:" << std::endl; + follows[a][i]->print(*_dout); + *_dout << std::endl; + return true; + } + } + + return false; +} + +int lockdep_will_lock(const char *name, int id) +{ + pthread_t p = pthread_self(); + if (id < 0) id = lockdep_register(name); + + pthread_mutex_lock(&lockdep_mutex); + dout(20) << "_will_lock " << name << " (" << id << ")" << std::endl; + + // check dependency graph + map &m = held[p]; + for (map::iterator p = m.begin(); + p != m.end(); + p++) { + if (p->first == id) { + *_dout << std::endl; + dout(0) << "recursive lock of " << name << " (" << id << ")" << std::endl; + BackTrace *bt = new BackTrace(BACKTRACE_SKIP); + bt->print(*_dout); + if (g_lockdep >= 2) { + *_dout << std::endl; + dout(0) << "previously locked at" << std::endl; + p->second->print(*_dout); + } + *_dout << std::endl; + assert(0); + } + else if (!follows[p->first][id]) { + // new dependency + + // did we just create a cycle? + BackTrace *bt = new BackTrace(BACKTRACE_SKIP); + if (does_follow(id, p->first)) { + dout(0) << "new dependency " << lock_names[p->first] << " (" << p->first << ") -> " << name << " (" << id << ")" + << " creates a cycle at" + << std::endl; + bt->print(*_dout); + *_dout << std::endl; + + dout(0) << "btw, i am holding these locks:" << std::endl; + for (map::iterator q = m.begin(); + q != m.end(); + q++) { + dout(0) << " " << lock_names[q->first] << " (" << q->first << ")" << std::endl; + if (g_lockdep >= 2) { + q->second->print(*_dout); + *_dout << std::endl; + } + } + + *_dout << std::endl; + *_dout << std::endl; + + // don't add this dependency, or we'll get aMutex. cycle in the graph, and + // does_follow() won't terminate. + + assert(0); // actually, we should just die here. + } else { + follows[p->first][id] = bt; + dout(10) << lock_names[p->first] << " -> " << name << " at" << std::endl; + //bt->print(*_dout); + } + } + } + + pthread_mutex_unlock(&lockdep_mutex); + return id; +} + +int lockdep_locked(const char *name, int id) +{ + pthread_t p = pthread_self(); + + if (id < 0) id = lockdep_register(name); + + pthread_mutex_lock(&lockdep_mutex); + dout(20) << "_locked " << name << std::endl; + if (g_lockdep >= 2) + held[p][id] = new BackTrace(BACKTRACE_SKIP); + else + held[p][id] = 0; + pthread_mutex_unlock(&lockdep_mutex); + return id; +} + +int lockdep_unlocked(const char *name, int id) +{ + pthread_t p = pthread_self(); + + if (id < 0) id = lockdep_register(name); + + pthread_mutex_lock(&lockdep_mutex); + dout(20) << "_unlocked " << name << std::endl; + + // don't assert.. lockdep may be enabled at any point in time + //assert(held.count(p)); + //assert(held[p].count(id)); + + delete held[p][id]; + held[p].erase(id); + pthread_mutex_unlock(&lockdep_mutex); + return id; +} + + diff --git a/src/common/lockdep.h b/src/common/lockdep.h new file mode 100644 index 0000000000000..00026b30b9721 --- /dev/null +++ b/src/common/lockdep.h @@ -0,0 +1,11 @@ +#ifndef _CEPH_LOCKDEP_H +#define _CEPH_LOCKDEP_H + +extern int g_lockdep; + +extern int lockdep_register(const char *n); +extern int lockdep_will_lock(const char *n, int id); +extern int lockdep_locked(const char *n, int id); +extern int lockdep_unlocked(const char *n, int id); + +#endif diff --git a/src/os/JournalingObjectStore.h b/src/os/JournalingObjectStore.h index 028d87d4ad621..015275ad624cd 100644 --- a/src/os/JournalingObjectStore.h +++ b/src/os/JournalingObjectStore.h @@ -101,6 +101,7 @@ protected: public: JournalingObjectStore() : op_seq(0), committing_op_seq(0), journal(0), + op_lock("JournalingObjectStore::op_lock"), journal_lock("JournalingObjectStore::journal_lock"), lock("JournalingObjectStore::lock") { } -- 2.39.5