From 75f41a95782a7ee83a243d91963e8d591402f8a6 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Thu, 14 Sep 2017 09:28:34 -0400 Subject: [PATCH] lockdep: fix races with concurrent lockdep teardown If the cct is unregistered while other threads are flogging mutexes, then we can hit all sorts of bugs. Ensure that we handle that situation sanely, by checking that g_lockdep is still set after we take the lockdep_mutex. Also, remove an assertion from lockdep_unregister, and just turn it into an immediate return. It's possible to have a call to lockdep_unregister_ceph_context, and then a call to lockdep_register_ceph_context while a mutex is being held by another task. In that case, it's possible the lock does not exist in the map when we go to unregister it. That's not a bug though, just a natural consequence of that series of actions. Tracker: http://tracker.ceph.com/issues/20988 Signed-off-by: Jeff Layton --- src/common/lockdep.cc | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/src/common/lockdep.cc b/src/common/lockdep.cc index 1fc564c9fd167..f3700c2a07faa 100644 --- a/src/common/lockdep.cc +++ b/src/common/lockdep.cc @@ -113,6 +113,8 @@ void lockdep_unregister_ceph_context(CephContext *cct) int lockdep_dump_locks() { pthread_mutex_lock(&lockdep_mutex); + if (!g_lockdep) + goto out; for (ceph::unordered_map >::iterator p = held.begin(); p != held.end(); @@ -127,7 +129,7 @@ int lockdep_dump_locks() *_dout << dendl; } } - +out: pthread_mutex_unlock(&lockdep_mutex); return 0; } @@ -165,8 +167,10 @@ int lockdep_get_free_id(void) static int _lockdep_register(const char *name) { - int id; + int id = -1; + if (!g_lockdep) + return id; ceph::unordered_map::iterator p = lock_ids.find(name); if (p == lock_ids.end()) { id = lockdep_get_free_id(); @@ -211,9 +215,16 @@ void lockdep_unregister(int id) } pthread_mutex_lock(&lockdep_mutex); + if (!g_lockdep) { + pthread_mutex_unlock(&lockdep_mutex); + return; + } map::iterator p = lock_names.find(id); - assert(p != lock_names.end()); + if (p == lock_names.end()) { + pthread_mutex_unlock(&lockdep_mutex); + return; + } int &refs = lock_refs[id]; if (--refs == 0) { @@ -279,8 +290,14 @@ int lockdep_will_lock(const char *name, int id, bool force_backtrace) pthread_t p = pthread_self(); pthread_mutex_lock(&lockdep_mutex); + if (!g_lockdep) { + pthread_mutex_unlock(&lockdep_mutex); + return id; + } + if (id < 0) id = _lockdep_register(name); + lockdep_dout(20) << "_will_lock " << name << " (" << id << ")" << dendl; // check dependency graph @@ -343,7 +360,6 @@ int lockdep_will_lock(const char *name, int id, bool force_backtrace) } } } - pthread_mutex_unlock(&lockdep_mutex); return id; } @@ -353,6 +369,8 @@ int lockdep_locked(const char *name, int id, bool force_backtrace) pthread_t p = pthread_self(); pthread_mutex_lock(&lockdep_mutex); + if (!g_lockdep) + goto out; if (id < 0) id = _lockdep_register(name); @@ -361,6 +379,7 @@ int lockdep_locked(const char *name, int id, bool force_backtrace) held[p][id] = new BackTrace(BACKTRACE_SKIP); else held[p][id] = 0; +out: pthread_mutex_unlock(&lockdep_mutex); return id; } @@ -376,6 +395,8 @@ int lockdep_will_unlock(const char *name, int id) } pthread_mutex_lock(&lockdep_mutex); + if (!g_lockdep) + goto out; lockdep_dout(20) << "_will_unlock " << name << dendl; // don't assert.. lockdep may be enabled at any point in time @@ -384,6 +405,7 @@ int lockdep_will_unlock(const char *name, int id) delete held[p][id]; held[p].erase(id); +out: pthread_mutex_unlock(&lockdep_mutex); return id; } -- 2.39.5