lockdep: free_ids and lock_ref hashes must be truly global

It's possible for the teardown of g_lockdep_ceph_ctx to occur, followed
by a new context being registered as the lockdep context. When that
occurs, we can end up reusing lock id's that were previously handed out
to consumers. We need for those IDs to be persistent across lockdep
enablement and disablement.

Make both the free_ids table, and the lock_refs map persistent across
lockdep_unregister_ceph_context and lockdep_register_ceph_context cycles.
Entries in those tables will only be deleted by the destruction of the
associated mutex.

When lockdep_unregister is called, do the refcounting like we normally
would, but only clear out the state when the lockid is registered
in the lock_names hash.

Finally, we do still need to handle the case where g_lockdep has gone
false even when there are outstanding references after the decrement.
Only log the message if that's not the case.

With this, we can deal with the case of multiple clients enabling and
disabling lockdep in an unsynchronized way.

Tracker: http://tracker.ceph.com/issues/21512
Signed-off-by: Jeff Layton <jlayton@redhat.com>
(cherry picked from commit 82fe4e94bcb706434f9215bc8405ff60770cf14f)

diff --git a/src/common/lockdep.cc b/src/common/lockdep.cc
index d7a165ff9cf1beb73ab6c0fdb79864acc3649461..ee7f7ece74e866c6946a158fcac10e2d40a01260 100644 (file)
--- a/src/common/lockdep.cc
+++ b/src/common/lockdep.cc
@@ -51,7 +51,8 @@ static ceph::unordered_map<pthread_t, map<int,BackTrace*> > held;
 static char follows[MAX_LOCKS][MAX_LOCKS/8]; // follows[a][b] means b taken after a
 static BackTrace *follows_bt[MAX_LOCKS][MAX_LOCKS];
 unsigned current_maxid;
-int last_freed_id;
+int last_freed_id = -1;
+static bool free_ids_inited;
 
 static bool lockdep_force_backtrace()
 {
@@ -73,10 +74,10 @@ void lockdep_register_ceph_context(CephContext *cct)
     g_lockdep = true;
     g_lockdep_ceph_ctx = cct;
     lockdep_dout(1) << "lockdep start" << dendl;
-    current_maxid = 0;
-       last_freed_id = -1;
-
-    memset((void*) &free_ids[0], 255, sizeof(free_ids));
+    if (!free_ids_inited) {
+      free_ids_inited = true;
+      memset((void*) &free_ids[0], 255, sizeof(free_ids));
+    }
   }
   pthread_mutex_unlock(&lockdep_mutex);
 }
@@ -100,12 +101,8 @@ void lockdep_unregister_ceph_context(CephContext *cct)
     held.clear();
     lock_names.clear();
     lock_ids.clear();
-    lock_refs.clear();
-    memset((void*)&free_ids[0], 0, sizeof(free_ids));
     memset((void*)&follows[0][0], 0, current_maxid * MAX_LOCKS/8);
     memset((void*)&follows_bt[0][0], 0, sizeof(BackTrace*) * current_maxid * MAX_LOCKS);
-    current_maxid = 0;
-    last_freed_id = -1;
   }
   pthread_mutex_unlock(&lockdep_mutex);
 }
@@ -215,40 +212,38 @@ void lockdep_unregister(int id)
   }
 
   pthread_mutex_lock(&lockdep_mutex);
-  if (!g_lockdep) {
-    pthread_mutex_unlock(&lockdep_mutex);
-    return;
-  }
 
+  std::string name;
   map<int, std::string>::iterator p = lock_names.find(id);
-  if (p == lock_names.end()) {
-    pthread_mutex_unlock(&lockdep_mutex);
-    return;
-  }
+  if (p == lock_names.end())
+    name = { "unknown" };
+  else
+    name = p->second;
 
   int &refs = lock_refs[id];
   if (--refs == 0) {
-    // reset dependency ordering
-    memset((void*)&follows[id][0], 0, MAX_LOCKS/8);
-    for (unsigned i=0; i<current_maxid; ++i) {
-      delete follows_bt[id][i];
-      follows_bt[id][i] = NULL;
-
-      delete follows_bt[i][id];
-      follows_bt[i][id] = NULL;
-      follows[i][id / 8] &= 255 - (1 << (id % 8));
-    }
+    if (p != lock_names.end()) {
+      // reset dependency ordering
+      memset((void*)&follows[id][0], 0, MAX_LOCKS/8);
+      for (unsigned i=0; i<current_maxid; ++i) {
+        delete follows_bt[id][i];
+        follows_bt[id][i] = NULL;
+
+        delete follows_bt[i][id];
+        follows_bt[i][id] = NULL;
+        follows[i][id / 8] &= 255 - (1 << (id % 8));
+      }
 
-    lockdep_dout(10) << "unregistered '" << p->second << "' from " << id
-                     << dendl;
-    lock_ids.erase(p->second);
-    lock_names.erase(id);
+      lockdep_dout(10) << "unregistered '" << name << "' from " << id << dendl;
+      lock_ids.erase(p->second);
+      lock_names.erase(id);
+    }
     lock_refs.erase(id);
     free_ids[id/8] |= (1 << (id % 8));
-       last_freed_id = id;
-  } else {
-    lockdep_dout(20) << "have " << refs << " of '" << p->second << "' "
-                     << "from " << id << dendl;
+    last_freed_id = id;
+  } else if (g_lockdep) {
+    lockdep_dout(20) << "have " << refs << " of '" << name << "' " <<
+                       "from " << id << dendl;
   }
   pthread_mutex_unlock(&lockdep_mutex);
 }