From: Kefu Chai <kchai@redhat.com>
Date: Wed, 21 Nov 2018 08:04:36 +0000 (+0800)
Subject: common/mutex_debug: disable lockdep for recursive_mutex
X-Git-Tag: v14.1.0~820^2~2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=a9ce443d8253b3e1e8818612a160abbef35eb203;p=ceph.git

common/mutex_debug: disable lockdep for recursive_mutex

without this change, when using recursive_mutex on Debug build, we will
have

 1: (ceph::mutex_debug_detail::mutex_debugging_base::_will_lock(bool)+0x4a)
[0x7fb751e36534]
 2: (ceph::mutex_debug_detail::mutex_debug_impl<true>::lock(bool)+0x3b)
[0x7fb751ca33b5]
 3: (std::lock_guard<ceph::mutex_debug_detail::mutex_debug_impl<true>
>::lock_guard(ceph::mutex_debug_detail::mutex_debug_impl<true>&)+0x2f)
[0x7fb751ca2e79]
 4: (ConfigProxy::get_val(std::__cxx11::basic_string<char,
std::char_traits<char>, std::allocator<char> > const&, char**, int)
const+0x41) [0x7fb751ce65fb]
 5: (TracepointProvider::verify_config(ConfigProxy const&)+0xbf)
[0x7fb751ce63f3]
 6: (TracepointProvider::handle_conf_change(ConfigProxy const&,
std::set<std::__cxx11::basic_string<char, std::char_traits<char>,
std::allocator<char> >, std::less<std::__cx
x11::basic_string<char, std::char_traits<char>, std::allocator<char> >
>, std::allocator<std::__cxx11::basic_string<char,
std::char_traits<char>, std::allocator<char> > > >
const&)+0x93) [0x7fb751ce62eb]
 7: (void ObserverMgr<ceph::md_config_obs_impl<ConfigProxy>
>::call_all_observers<ConfigProxy>(ConfigProxy const&)+0x1ab)
[0x7fb751d4f1e1]
 8: (ConfigProxy::call_all_observers()+0x50) [0x7fb751d4c570]
 9: (CephContext::start_service_thread()+0x121) [0x7fb751d47eff]

because we have following sequence:

1. acquire ConfigProxy::lock
2. acquire TracepointProvider::m_lock
3. acquire ConfigProxy::lock

which annoys lockdep, because it's a cyclic graph.

after this change,

* we will not ignore lockdep when locking/unlocking
  recursive_mutex. this basically disables lockdep checks for
  mutex_recursive_debug, but it still has nlock checks.
* also, enable_lockdep() is extracted. as we are repeating it
  for multiple times in this header file.

see 41522699, and see http://tracker.ceph.com/issues/12614

Signed-off-by: Kefu Chai <kchai@redhat.com>
---

diff --git a/src/common/mutex_debug.h b/src/common/mutex_debug.h
index 43b52eb738f..247233fd999 100644
--- a/src/common/mutex_debug.h
+++ b/src/common/mutex_debug.h
@@ -85,6 +85,16 @@ private:
     ceph_assert(r == 0);
   }
 
+  bool enable_lockdep(bool no_lockdep) const {
+    if (recursive) {
+      return false;
+    } else if (no_lockdep) {
+      return false;
+    } else {
+      return g_lockdep;
+    }
+  }
+
 public:
   static constexpr bool recursive = Recursive;
 
@@ -163,7 +173,7 @@ public:
   bool try_lock(bool no_lockdep = false) {
     bool locked = try_lock_impl();
     if (locked) {
-      if (g_lockdep && !no_lockdep)
+      if (enable_lockdep(no_lockdep))
 	_locked();
       _post_lock();
     }
@@ -171,21 +181,21 @@ public:
   }
 
   void lock(bool no_lockdep = false) {
-    if (g_lockdep && !no_lockdep)
+    if (enable_lockdep(no_lockdep))
       _will_lock(recursive);
 
     if (try_lock())
       return;
 
     lock_impl();
-    if (!no_lockdep && g_lockdep)
+    if (enable_lockdep(no_lockdep))
       _locked();
     _post_lock();
   }
 
   void unlock(bool no_lockdep = false) {
     _pre_unlock();
-    if (!no_lockdep && g_lockdep)
+    if (enable_lockdep(no_lockdep))
       _will_unlock();
     unlock_impl();
   }