mds: fix locking around handle_conf_change

We have to be careful about holding mds_lock
while setting things up, and we also have
to handle either locked or unlocked
calls to handle_conf_change.

Signed-off-by: John Spray <john.spray@redhat.com>

diff --git a/src/mds/MDCache.cc b/src/mds/MDCache.cc
index bcf7046c86a8a933956a5d1eee8832b3a76741b6..f13453676b3a7761069eec370e57287d9cebfaef 100644 (file)
--- a/src/mds/MDCache.cc
+++ b/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();
+  }
+}

diff --git a/src/mds/MDCache.h b/src/mds/MDCache.h
index 10641ea6f95063d6d1d3e69b38ab5ba73cbe90f6..1a7f89691f8eadb5206d04ae45f7566feaf9e444 100644 (file)
--- a/src/mds/MDCache.h
+++ b/src/mds/MDCache.h
@@ -156,6 +156,9 @@ public:
     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();

diff --git a/src/mds/MDSDaemon.cc b/src/mds/MDSDaemon.cc
index 568869cc4de5f3a12241cc36b4e1a3b5dbd3f562..4b562b27dbdbe1800b70b9bfb53cd908fb9dc889 100644 (file)
--- a/src/mds/MDSDaemon.cc
+++ b/src/mds/MDSDaemon.cc
@@ -353,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;
@@ -361,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")) {
@@ -390,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();
   }
 }
 
@@ -482,6 +500,8 @@ int MDSDaemon::init(MDSMap::DaemonState wanted_state)
   // 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
@@ -530,8 +550,6 @@ int MDSDaemon::init(MDSMap::DaemonState wanted_state)
   // schedule tick
   reset_tick();
 
-  g_conf->add_observer(this);
-
   mds_lock.Unlock();
 
   return 0;

diff --git a/src/mds/StrayManager.cc b/src/mds/StrayManager.cc
index bb9a3eea8c9d80d176394d596e7e40ebc469c05b..da1a2fe2ce377442fa46b4435cb5c577722e2524 100644 (file)
--- a/src/mds/StrayManager.cc
+++ b/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();

diff --git a/src/mds/StrayManager.h b/src/mds/StrayManager.h
index 1db88b1d9eb88e06904f23e0ecbd59228ededacb..c62edd3e1b65199aef07405389730f9a2b23f17e 100644 (file)
--- a/src/mds/StrayManager.h
+++ b/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