mds, client: fix locking around handle_conf_change

src/CMakeLists.txt

@@ -909,7 +909,7 @@ if(WITH_LIBCEPHFS)
     $<TARGET_OBJECTS:common_util_obj>)
   target_link_libraries(cephfs PRIVATE client osdc osd os global common cls_lock_client
     ${BLKID_LIBRARIES}
-    ${CRYPTO_LIBS} ${EXTRALIBS} ${TCMALLOC_LIBS})
+    ${CRYPTO_LIBS} ${EXTRALIBS} ${ALLOC_LIBS})
 if(${ENABLE_SHARED})
   set_target_properties(cephfs PROPERTIES OUTPUT_NAME cephfs VERSION 1.0.0
     SOVERSION 1)

src/client/Client.cc

@@ -468,15 +468,13 @@ void Client::dump_status(Formatter *f)
 
 int Client::init()
 {
-  client_lock.Lock();
-  assert(!initialized);
-
   timer.init();
-
   objectcacher->start();
-
   objecter->init();
 
+  client_lock.Lock();
+  assert(!initialized);
+
   // ok!
   messenger->add_dispatcher_tail(objecter);
   messenger->add_dispatcher_tail(this);
@@ -608,9 +606,9 @@ void Client::shutdown()
   assert(initialized);
   initialized = false;
   timer.shutdown();
-  objecter->shutdown();
   client_lock.Unlock();
 
+  objecter->shutdown();
   objecter_finisher.wait_for_empty();
   objecter_finisher.stop();
 
@@ -12233,6 +12231,8 @@ const char** Client::get_tracked_conf_keys() const
 void Client::handle_conf_change(const struct md_config_t *conf,
 				const std::set <std::string> &changed)
 {
+  Mutex::Locker lock(client_lock);
+
   if (changed.count("client_cache_size") ||
       changed.count("client_cache_mid")) {
     lru.lru_set_max(cct->_conf->client_cache_size);

src/mds/MDCache.cc

@@ -11957,3 +11957,13 @@ void MDCache::notify_osdmap_changed()
   stray_manager.update_op_limit();
 }
 
+void MDCache::handle_conf_change(const struct md_config_t *conf,
+			     const std::set <std::string> &changed)
+{
+  assert(mds->mds_lock.is_locked_by_me());
+
+  if (changed.count("mds_max_purge_ops")
+      || changed.count("mds_max_purge_ops_per_pg")) {
+    stray_manager.update_op_limit();
+  }
+}

src/mds/MDCache.h

@@ -156,6 +156,9 @@ class MDCache {
     stray_manager.eval_stray(dn);
   }
 
+  void handle_conf_change(const struct md_config_t *conf,
+                          const std::set <std::string> &changed);
+
   void maybe_eval_stray(CInode *in, bool delay=false);
   bool is_readonly() { return readonly; }
   void force_readonly();

src/mds/MDSDaemon.cc

@@ -226,6 +226,7 @@ void MDSDaemon::set_up_admin_socket()
 {
   int r;
   AdminSocket *admin_socket = g_ceph_context->get_admin_socket();
+  assert(asok_hook == nullptr);
   asok_hook = new MDSSocketHook(this);
   r = admin_socket->register_command("status", "status", asok_hook,
 				     "high-level status of MDS");
@@ -352,6 +353,9 @@ const char** MDSDaemon::get_tracked_conf_keys() const
     "clog_to_syslog",
     "clog_to_syslog_facility",
     "clog_to_syslog_level",
+    // StrayManager
+    "mds_max_purge_ops",
+    "mds_max_purge_ops_per_pg",
     NULL
   };
   return KEYS;
@@ -360,7 +364,12 @@ const char** MDSDaemon::get_tracked_conf_keys() const
 void MDSDaemon::handle_conf_change(const struct md_config_t *conf,
 			     const std::set <std::string> &changed)
 {
-  Mutex::Locker l(mds_lock);
+  // We may be called within mds_lock (via `tell`) or outwith the
+  // lock (via admin socket `config set`), so handle either case.
+  const bool initially_locked = mds_lock.is_locked_by_me();
+  if (!initially_locked) {
+    mds_lock.Lock();
+  }
 
   if (changed.count("mds_op_complaint_time") ||
       changed.count("mds_op_log_threshold")) {
@@ -389,9 +398,19 @@ void MDSDaemon::handle_conf_change(const struct md_config_t *conf,
       mds_rank->update_log_config();
     }
   }
+
   if (!g_conf->mds_log_pause && changed.count("mds_log_pause")) {
-    if (mds_rank)
+    if (mds_rank) {
       mds_rank->mdlog->kick_submitter();
+    }
+  }
+
+  if (mds_rank) {
+    mds_rank->mdcache->handle_conf_change(conf, changed);
+  }
+
+  if (!initially_locked) {
+    mds_lock.Unlock();
   }
 }
 
@@ -477,6 +496,12 @@ int MDSDaemon::init(MDSMap::DaemonState wanted_state)
     sleep(10);
   }
 
+  // Set up admin socket before taking mds_lock, so that ordering
+  // is consistent (later we take mds_lock within asok callbacks)
+  set_up_admin_socket();
+
+  g_conf->add_observer(this);
+
   mds_lock.Lock();
   if (beacon.get_want_state() == MDSMap::STATE_DNE) {
     suicide();  // we could do something more graceful here
@@ -525,9 +550,6 @@ int MDSDaemon::init(MDSMap::DaemonState wanted_state)
   // schedule tick
   reset_tick();
 
-  set_up_admin_socket();
-  g_conf->add_observer(this);
-
   mds_lock.Unlock();
 
   return 0;

src/mds/StrayManager.cc

@@ -847,26 +847,6 @@ void StrayManager::_truncate_stray_logged(CDentry *dn, LogSegment *ls)
 }
 
 
-const char** StrayManager::get_tracked_conf_keys() const
-{
-  static const char* KEYS[] = {
-    "mds_max_purge_ops",
-    "mds_max_purge_ops_per_pg",
-    NULL
-  };
-  return KEYS;
-}
-
-void StrayManager::handle_conf_change(const struct md_config_t *conf,
-			  const std::set <std::string> &changed)
-{
-  if (changed.count("mds_max_purge_ops")
-      || changed.count("mds_max_purge_ops_per_pg")) {
-    update_op_limit();
-  }
-}
-
-
 void StrayManager::update_op_limit()
 {
   const OSDMap *osdmap = mds->objecter->get_osdmap_read();

src/mds/StrayManager.h

@@ -23,7 +23,7 @@ class PerfCounters;
 class CInode;
 class CDentry;
 
-class StrayManager : public md_config_obs_t
+class StrayManager
 {
   protected:
   class QueuedStray {
@@ -236,17 +236,6 @@ class StrayManager : public md_config_obs_t
    * Call this whenever one of those operands changes.
    */
   void update_op_limit();
-
-  /**
-   * Subscribe to changes on mds_max_purge_ops
-   */
-  virtual const char** get_tracked_conf_keys() const;
-
-  /**
-   * Call update_op_limit if mds_max_purge_ops changes
-   */
-  virtual void handle_conf_change(const struct md_config_t *conf,
-			  const std::set <std::string> &changed);
 };
 
 #endif  // STRAY_MANAGER_H

src/vstart.sh

@@ -457,6 +457,7 @@ if [ "$start_mon" -eq 1 ]; then
         filestore fd cache size = 32
         run dir = $CEPH_OUT_DIR
         enable experimental unrecoverable data corrupting features = *
+        lockdep = true
 EOF
 if [ "$cephx" -eq 1 ] ; then
 cat <<EOF >> $conf_fn