mds: fix stray creation/removal notification

In MDCache::scan_stray_dir(), notify_stray_created() is called for
both null and non-null dentries. This is wrong, it should be called
only for non-null dentries.

Calling notify_stray_created() in MDCache::get_or_create_stray_dentry()
is racy too. MDS can create wrong stray dentry because target dentry
linkage can change before rmdir/rename lock the dentry lock.

Fixes: http://tracker.ceph.com/issues/19630
Signed-off-by: "Yan, Zheng" <zyan@redhat.com>

diff --git a/src/mds/CDentry.cc b/src/mds/CDentry.cc
index 032f83a4297aa50a4ccea95f7836910aa6d3f12b..8790377858ea30d4af0672caa9aea7d143c08ba8 100644 (file)
--- a/src/mds/CDentry.cc
+++ b/src/mds/CDentry.cc
@@ -249,6 +249,18 @@ void CDentry::unlink_remote(CDentry::linkage_t *dnl)
   dnl->inode = 0;
 }
 
+void CDentry::push_projected_linkage()
+{
+  _project_linkage();
+
+  if (is_auth()) {
+    CInode *diri = dir->inode;
+    if (diri->is_stray())
+      diri->mdcache->notify_stray_removed();
+  }
+}
+
+
 void CDentry::push_projected_linkage(CInode *inode)
 {
   // dirty rstat tracking is in the projected plane
@@ -261,6 +273,12 @@ void CDentry::push_projected_linkage(CInode *inode)
 
   if (dirty_rstat)
     inode->mark_dirty_rstat();
+
+  if (is_auth()) {
+    CInode *diri = dir->inode;
+    if (diri->is_stray())
+      diri->mdcache->notify_stray_created();
+  }
 }
 
 CDentry::linkage_t *CDentry::pop_projected_linkage()

diff --git a/src/mds/CDentry.h b/src/mds/CDentry.h
index 3d300237f6a7b4099e9820dee567dc600bfb6e76..f9505875e0464bb6f4842d84b4c05d99f5b2b8a2 100644 (file)
--- a/src/mds/CDentry.h
+++ b/src/mds/CDentry.h
@@ -168,9 +168,7 @@ public:
     projected.push_back(linkage_t());
     return &projected.back();
   }
-  void push_projected_linkage() {
-    _project_linkage();
-  }
+  void push_projected_linkage();
   void push_projected_linkage(inodeno_t ino, char d_type) {
     linkage_t *p = _project_linkage();
     p->remote_ino = ino;

diff --git a/src/mds/MDCache.cc b/src/mds/MDCache.cc
index 88832279d2fd060e0c29acedf7197608b60f0011..3fea6e416c240b8a534d531a12f2b74173b7417a 100644 (file)
--- a/src/mds/MDCache.cc
+++ b/src/mds/MDCache.cc
@@ -657,6 +657,7 @@ void MDCache::populate_mydir()
   }
 
   // open or create stray
+  uint64_t num_strays = 0;
   for (int i = 0; i < NUM_STRAY; ++i) {
     stringstream name;
     name << "stray" << i;
@@ -699,9 +700,14 @@ void MDCache::populate_mydir()
         dir->fetch(new C_MDS_RetryOpenRoot(this));
         return;
       }
+
+      if (dir->get_frag_size() > 0)
+       num_strays += dir->get_frag_size();
     }
   }
 
+  stray_manager.set_num_strays(num_strays);
+
   // okay!
   dout(10) << "populate_mydir done" << dendl;
   assert(!open);    
@@ -742,11 +748,6 @@ CDentry *MDCache::get_or_create_stray_dentry(CInode *in)
     assert(straydn->get_projected_linkage()->is_null());
   }
 
-  // Notify even if a null dentry already existed, because
-  // StrayManager counts the number of stray inodes, not the
-  // number of dentries in the directory.
-  stray_manager.notify_stray_created();
-
   straydn->state_set(CDentry::STATE_STRAY);
   return straydn;
 }
@@ -9389,7 +9390,6 @@ void MDCache::scan_stray_dir(dirfrag_t next)
     for (CDir::map_t::iterator q = dir->items.begin(); q != dir->items.end(); ++q) {
       CDentry *dn = q->second;
       dn->state_set(CDentry::STATE_STRAY);
-      stray_manager.notify_stray_created();
       CDentry::linkage_t *dnl = dn->get_projected_linkage();
       if (dnl->is_primary()) {
        CInode *in = dnl->get_inode();
@@ -12276,6 +12276,21 @@ void MDCache::register_perfcounters()
     stray_manager.set_logger(logger.get());
 }
 
+void MDCache::activate_stray_manager()
+{
+  if (open) {
+    stray_manager.activate();
+  } else {
+    wait_for_open(
+       new MDSInternalContextWrapper(mds,
+         new FunctionContext([this](int r){
+           stray_manager.activate();
+           })
+         )
+       );
+  }
+}
+
 /**
  * Call this when putting references to an inode/dentry or
  * when attempting to trim it.

diff --git a/src/mds/MDCache.h b/src/mds/MDCache.h
index 76e779794a7455d327199ca31835b21734a86e64..cd0afbe9d09069d2a733e0d5bb140cfd03718e7b 100644 (file)
--- a/src/mds/MDCache.h
+++ b/src/mds/MDCache.h
@@ -142,9 +142,7 @@ public:
     stray_index = (stray_index+1)%NUM_STRAY;
   }
 
-  void activate_stray_manager() {
-    stray_manager.activate();
-  }
+  void activate_stray_manager();
 
   /**
    * Call this when you know that a CDentry is ready to be passed

diff --git a/src/mds/Server.cc b/src/mds/Server.cc
index 9cf926bce4b9691132d353069f4feb09b98002e6..12a677daf6451e0563d96f20793e1e761b410e92 100644 (file)
--- a/src/mds/Server.cc
+++ b/src/mds/Server.cc
@@ -7116,20 +7116,6 @@ void Server::_rename_apply(MDRequestRef& mdr, CDentry *srcdn, CDentry *destdn, C
     }
   }
 
-  if (srcdn->get_dir()->inode->is_stray() &&
-      srcdn->get_dir()->inode->get_stray_owner() == mds->get_nodeid()) {
-    // A reintegration event or a migration away from me
-    dout(20) << __func__ << ": src dentry was a stray, updating stats" << dendl;
-    mdcache->notify_stray_removed();
-  }
-
-  if (destdn->get_dir()->inode->is_stray() &&
-      destdn->get_dir()->inode->get_stray_owner() == mds->get_nodeid()) {
-    // A stray migration (to me)
-    dout(20) << __func__ << ": dst dentry was a stray, updating stats" << dendl;
-    mdcache->notify_stray_created();
-  }
-
   // src
   if (srcdn->is_auth())
     srcdn->mark_dirty(mdr->more()->pvmap[srcdn], mdr->ls);

diff --git a/src/mds/StrayManager.cc b/src/mds/StrayManager.cc
index 816f9249883cc84631f486dc5cd97716ba54c297..f4d3ac4efc7f986540a44e088e44017a56f9fc2e 100644 (file)
--- a/src/mds/StrayManager.cc
+++ b/src/mds/StrayManager.cc
@@ -226,7 +226,6 @@ void StrayManager::_purge_stray_purged(
     mds->mdlog->submit_entry(le, new C_PurgeStrayLogged(this, dn, pdv,
           mds->mdlog->get_current_segment()));
 
-    num_strays--;
     logger->set(l_mdc_num_strays, num_strays);
   }
 }
@@ -377,6 +376,13 @@ void StrayManager::advance_delayed()
   logger->set(l_mdc_num_strays_delayed, num_strays_delayed);
 }
 
+void StrayManager::set_num_strays(uint64_t num)
+{
+  assert(!started);
+  num_strays = num;
+  logger->set(l_mdc_num_strays, num_strays);
+}
+
 void StrayManager::notify_stray_created()
 {
   num_strays++;

diff --git a/src/mds/StrayManager.h b/src/mds/StrayManager.h
index eff21958ddfbbac2b127c4a0958086d832eb1853..a9bf3aa7840aca63aa98d57578790e27f59803ad 100644 (file)
--- a/src/mds/StrayManager.h
+++ b/src/mds/StrayManager.h
@@ -127,6 +127,7 @@ class StrayManager
 
   bool eval_stray(CDentry *dn, bool delay=false);
 
+  void set_num_strays(uint64_t num);
   uint64_t get_num_strays() const { return num_strays; }
 
   /**