lockdep: code cleanup and de-globalization

common/Mutex.cc: this is a duplicate of the code in common/lockdep.cc.
Delete the duplicate code.

common/lockdep.cc: require the programmer to register a CephContext to
use with lockdep. If we don't have one, we don't print anything out.

Signed-off-by: Colin McCabe <colin.mccabe@dreamhost.com>

diff --git a/src/common/Mutex.cc b/src/common/Mutex.cc
deleted file mode 100644 (file)
index b7ac971..0000000
--- a/src/common/Mutex.cc
+++ /dev/null
@@ -1,193 +0,0 @@
-
-#include "common/environment.h"
-#include "Mutex.h"
-
-#ifdef LOCKDEP
-
-#include "include/types.h"
-#include "Clock.h"
-#include "BackTrace.h"
-
-#include <ext/hash_map>
-
-#include "common/config.h"
-
-#undef dout
-#undef derr
-#define  dout(l)    if (l<=g_conf->debug_lockdep) *_dout << g_clock.now() << " " << std::hex << pthread_self() << std::dec << " lockdep: "
-#define  derr(l)    if (l<=g_conf->debug_lockdep) *_derr << g_clock.now() << " " << std::hex << pthread_self() << std::dec << " lockdep: "
-
-
-pthread_mutex_t lockdep_mutex = PTHREAD_MUTEX_INITIALIZER;
-
-
-#define MAX_LOCKS  100   // increase me as needed
-
-hash_map<const char *, int> lock_ids;
-map<int, const char *> lock_names;
-
-int last_id = 0;
-
-hash_map<pthread_t, map<int,BackTrace*> > held;
-BackTrace *follows[MAX_LOCKS][MAX_LOCKS];       // follows[a][b] means b taken after a 
-
-
-#define BACKTRACE_SKIP 3
-
-
-
-void Mutex::_register()
-{
-  pthread_mutex_lock(&lockdep_mutex);
-
-  if (last_id == 0)
-    for (int i=0; i<MAX_LOCKS; i++)
-      for (int j=0; j<MAX_LOCKS; j++)
-       follows[i][j] = NULL;
-  
-  hash_map<const char *, int>::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);
-}
-
-
-// does a follow b?
-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<MAX_LOCKS; i++) {
-    if (follows[a][i] &&
-       does_follow(i, b)) {
-      dout(0) << "existing intermediate dependency " << lock_names[a] << " (" << a << ") -> " << lock_names[i] << " (" << i << ") at:" << std::endl;
-      follows[a][i]->print(*_dout);
-      *_dout << std::endl;
-      return true;
-    }
-  }
-
-  return false;
-}
-
-void Mutex::_will_lock()
-{
-  pthread_t p = pthread_self();
-  if (id < 0) _register();
-
-  pthread_mutex_lock(&lockdep_mutex);
-  dout(20) << "_will_lock " << name << " (" << id << ")" << std::endl;
-
-  // check dependency graph
-  map<int, BackTrace *> &m = held[p];
-  for (map<int, BackTrace *>::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<int, BackTrace *>::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);
-}
-
-void Mutex::_locked()
-{
-  pthread_t p = pthread_self();
-
-  if (id < 0) _register();
-
-  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);
-}
-
-void Mutex::_unlocked()
-{
-  pthread_t p = pthread_self();
-  
-  if (id < 0) _register();
-
-  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);
-}
-
-
-
-
-
-#endif

diff --git a/src/common/Mutex.h b/src/common/Mutex.h
index 411b487d1ace669331bb7cd23cbf5943e3c3bbe8..86a27dc50fcff1e411603f671ebfe9357c293093 100644 (file)
--- a/src/common/Mutex.h
+++ b/src/common/Mutex.h
@@ -15,11 +15,10 @@
 #ifndef CEPH_MUTEX_H
 #define CEPH_MUTEX_H
 
-#include <pthread.h>
 #include "include/assert.h"
 #include "lockdep.h"
 
-#define LOCKDEP
+#include <pthread.h>
 
 using namespace ceph;
 
@@ -38,7 +37,6 @@ private:
   void operator=(Mutex &M) {}
   Mutex( const Mutex &M ) {}
 
-#ifdef LOCKDEP
   void _register() {
     id = lockdep_register(name);
   }
@@ -51,12 +49,6 @@ private:
   void _will_unlock() {  // about to unlock
     id = lockdep_will_unlock(name, id);
   }
-#else
-  void _register() {}
-  void _will_lock() {} // about to lock
-  void _locked() {}    // just locked
-  void _will_unlock() {}  // about to unlock
-#endif
 
 public:
   Mutex(const char *n, bool r = false, bool ld=true, bool bt=false) :

diff --git a/src/common/lockdep.cc b/src/common/lockdep.cc
index d3963f91eb14245c36674d7c1edb9226cc5915c1..ceb0a7439f5fcf91ee8656207336428293ae9db9 100644 (file)
--- a/src/common/lockdep.cc
+++ b/src/common/lockdep.cc
@@ -22,6 +22,9 @@
 
 /******* Constants **********/
 #define DOUT_SUBSYS lockdep
+#undef DOUT_COND
+#define DOUT_COND(cct, l) cct && l <= XDOUT_CONDVAR(cct, DOUT_SUBSYS)
+#define lockdep_dout(v) ldout(g_lockdep_ceph_ctx, v)
 #define MAX_LOCKS  100   // increase me as needed
 #define BACKTRACE_SKIP 3
 
@@ -33,14 +36,14 @@ struct lockdep_stopper_t {
     g_lockdep = 0;
   }
 };
-static lockdep_stopper_t lockdep_stopper;
 static pthread_mutex_t lockdep_mutex = PTHREAD_MUTEX_INITIALIZER;
+static CephContext *g_lockdep_ceph_ctx = NULL;
+static lockdep_stopper_t lockdep_stopper;
 static hash_map<const char *, int> lock_ids;
 static map<int, const char *> lock_names;
 static int last_id = 0;
 static hash_map<pthread_t, map<int,BackTrace*> > held;
 static BackTrace *follows[MAX_LOCKS][MAX_LOCKS];       // follows[a][b] means b taken after a
-static CephContext *g_lockdep_ceph_ctx = NULL;
 
 /******* Functions **********/
 void lockdep_register_ceph_context(CephContext *cct)
@@ -57,11 +60,11 @@ int lockdep_dump_locks()
   for (hash_map<pthread_t, map<int,BackTrace*> >::iterator p = held.begin();
        p != held.end();
        p++) {
-    dout(0) << "--- thread " << p->first << " ---" << dendl;
+    lockdep_dout(0) << "--- thread " << p->first << " ---" << dendl;
     for (map<int,BackTrace*>::iterator q = p->second.begin();
         q != p->second.end();
         q++) {
-      dout(0) << "  * " << lock_names[q->first] << "\n";
+      lockdep_dout(0) << "  * " << lock_names[q->first] << "\n";
       if (q->second)
        q->second->print(*_dout);
       *_dout << dendl;
@@ -90,10 +93,10 @@ int lockdep_register(const char *name)
     id = last_id++;
     lock_ids[name] = id;
     lock_names[id] = name;
-    dout(10) << "registered '" << name << "' as " << id << dendl;
+    lockdep_dout(10) << "registered '" << name << "' as " << id << dendl;
   } else {
     id = p->second;
-    dout(20) << "had '" << name << "' as " << id << dendl;
+    lockdep_dout(20) << "had '" << name << "' as " << id << dendl;
   }
 
   pthread_mutex_unlock(&lockdep_mutex);
@@ -106,7 +109,7 @@ int lockdep_register(const char *name)
 static bool does_follow(int a, int b)
 {
   if (follows[a][b]) {
-    dout(0) << "\n";
+    lockdep_dout(0) << "\n";
     *_dout << "------------------------------------" << "\n";
     *_dout << "existing dependency " << lock_names[a] << " (" << a << ") -> "
            << lock_names[b] << " (" << b << ") at:\n";
@@ -118,7 +121,7 @@ static bool does_follow(int a, int b)
   for (int i=0; i<MAX_LOCKS; i++) {
     if (follows[a][i] &&
        does_follow(i, b)) {
-      dout(0) << "existing intermediate dependency " << lock_names[a]
+      lockdep_dout(0) << "existing intermediate dependency " << lock_names[a]
           << " (" << a << ") -> " << lock_names[i] << " (" << i << ") at:\n";
       follows[a][i]->print(*_dout);
       *_dout << dendl;
@@ -135,7 +138,7 @@ int lockdep_will_lock(const char *name, int id)
   if (id < 0) id = lockdep_register(name);
 
   pthread_mutex_lock(&lockdep_mutex);
-  dout(20) << "_will_lock " << name << " (" << id << ")" << dendl;
+  lockdep_dout(20) << "_will_lock " << name << " (" << id << ")" << dendl;
 
   // check dependency graph
   map<int, BackTrace *> &m = held[p];
@@ -143,7 +146,7 @@ int lockdep_will_lock(const char *name, int id)
        p != m.end();
        p++) {
     if (p->first == id) {
-      dout(0) << "\n";
+      lockdep_dout(0) << "\n";
       *_dout << "recursive lock of " << name << " (" << id << ")\n";
       BackTrace *bt = new BackTrace(BACKTRACE_SKIP);
       bt->print(*_dout);
@@ -160,25 +163,25 @@ int lockdep_will_lock(const char *name, int id)
       // 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]
+       lockdep_dout(0) << "new dependency " << lock_names[p->first]
                << " (" << p->first << ") -> " << name << " (" << id << ")"
                << " creates a cycle at\n";
        bt->print(*_dout);
        *_dout << dendl;
 
-       dout(0) << "btw, i am holding these locks:" << dendl;
+       lockdep_dout(0) << "btw, i am holding these locks:" << dendl;
        for (map<int, BackTrace *>::iterator q = m.begin();
             q != m.end();
             q++) {
-         dout(0) << "  " << lock_names[q->first] << " (" << q->first << ")" << dendl;
+         lockdep_dout(0) << "  " << lock_names[q->first] << " (" << q->first << ")" << dendl;
          if (q->second) {
-           dout(0) << " ";
+           lockdep_dout(0) << " ";
            q->second->print(*_dout);
            *_dout << dendl;
          }
        }
 
-       dout(0) << "\n" << dendl;
+       lockdep_dout(0) << "\n" << dendl;
 
        // don't add this dependency, or we'll get aMutex. cycle in the graph, and
        // does_follow() won't terminate.
@@ -186,7 +189,7 @@ int lockdep_will_lock(const char *name, int id)
        assert(0);  // actually, we should just die here.
       } else {
        follows[p->first][id] = bt;
-       dout(10) << lock_names[p->first] << " -> " << name << " at" << dendl;
+       lockdep_dout(10) << lock_names[p->first] << " -> " << name << " at" << dendl;
        //bt->print(*_dout);
       }
     }
@@ -203,7 +206,7 @@ int lockdep_locked(const char *name, int id, bool force_backtrace)
   if (id < 0) id = lockdep_register(name);
 
   pthread_mutex_lock(&lockdep_mutex);
-  dout(20) << "_locked " << name << dendl;
+  lockdep_dout(20) << "_locked " << name << dendl;
   if (g_lockdep >= 2 || force_backtrace)
     held[p][id] = new BackTrace(BACKTRACE_SKIP);
   else
@@ -223,7 +226,7 @@ int lockdep_will_unlock(const char *name, int id)
   }
 
   pthread_mutex_lock(&lockdep_mutex);
-  dout(20) << "_will_unlock " << name << dendl;
+  lockdep_dout(20) << "_will_unlock " << name << dendl;
 
   // don't assert.. lockdep may be enabled at any point in time
   //assert(held.count(p));