From: Yan, Zheng <zyan@redhat.com>
Date: Thu, 5 Mar 2020 14:54:50 +0000 (+0800)
Subject: mds: fix 'if there is lock cache on dir' check
X-Git-Tag: v16.1.0~2829^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=20d21cff89e1bb2bd6d7af311b4cc3272ce5fe65;p=ceph.git

mds: fix 'if there is lock cache on dir' check

When invalidating lock cache, current code detach lock cache from all
its locks immediately. So diri->filelock.is_cached() only tells us if
there is active (not being invalidated) lock cache on a dir.

But MDCache::path_traverse() and Server::_dir_is_nonempty_unlocked() use
diri->filelock.is_cached() to check if there is any lock cache on a dir.
This bug can result in processing async requests and normal requests out
of order.

The fix is detaching lock cache from its locks when lock cache is
completely invalidated .

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

diff --git a/src/mds/Locker.cc b/src/mds/Locker.cc
index f8f99342c477..fc8007db2bbd 100644
--- a/src/mds/Locker.cc
+++ b/src/mds/Locker.cc
@@ -802,6 +802,8 @@ void Locker::put_lock_cache(MDLockCache* lock_cache)
 
   ceph_assert(lock_cache->invalidating);
 
+  lock_cache->detach_locks();
+
   CInode *diri = lock_cache->get_dir_inode();
   for (auto dir : lock_cache->auth_pinned_dirfrags) {
     if (dir->get_inode() != diri)
@@ -832,7 +834,7 @@ void Locker::invalidate_lock_cache(MDLockCache *lock_cache)
     ceph_assert(!lock_cache->client_cap);
   } else {
     lock_cache->invalidating = true;
-    lock_cache->detach_all();
+    lock_cache->detach_dirfrags();
   }
 
   Capability *cap = lock_cache->client_cap;
@@ -880,8 +882,10 @@ void Locker::invalidate_lock_caches(CDir *dir)
 void Locker::invalidate_lock_caches(SimpleLock *lock)
 {
   dout(10) << "invalidate_lock_caches " << *lock << " on " << *lock->get_parent() << dendl;
-  while (lock->is_cached()) {
-    invalidate_lock_cache(lock->get_first_cache());
+  if (lock->is_cached()) {
+    auto&& lock_caches = lock->get_active_caches();
+    for (auto& lc : lock_caches)
+      invalidate_lock_cache(lc);
   }
 }
 
diff --git a/src/mds/Mutation.cc b/src/mds/Mutation.cc
index 03555b4f0980..d9f3f7cadcc6 100644
--- a/src/mds/Mutation.cc
+++ b/src/mds/Mutation.cc
@@ -578,7 +578,7 @@ void MDLockCache::attach_dirfrags(std::vector<CDir*>&& dfv)
   }
 }
 
-void MDLockCache::detach_all()
+void MDLockCache::detach_locks()
 {
   ceph_assert(items_lock);
   int i = 0;
@@ -588,9 +588,12 @@ void MDLockCache::detach_all()
     ++i;
   }
   items_lock.reset();
+}
 
+void MDLockCache::detach_dirfrags()
+{
   ceph_assert(items_dir);
-  i = 0;
+  int i = 0;
   for (auto dir : auth_pinned_dirfrags) {
     (void)dir;
     items_dir[i].item_dir.remove_myself();
diff --git a/src/mds/Mutation.h b/src/mds/Mutation.h
index d79b430de662..0b4070420d37 100644
--- a/src/mds/Mutation.h
+++ b/src/mds/Mutation.h
@@ -496,7 +496,8 @@ struct MDLockCache : public MutationImpl {
 
   void attach_locks();
   void attach_dirfrags(std::vector<CDir*>&& dfv);
-  void detach_all();
+  void detach_locks();
+  void detach_dirfrags();
 
   CInode *diri;
   Capability *client_cap;
diff --git a/src/mds/SimpleLock.cc b/src/mds/SimpleLock.cc
index 1f3b732169b7..7c447419bb6e 100644
--- a/src/mds/SimpleLock.cc
+++ b/src/mds/SimpleLock.cc
@@ -95,12 +95,14 @@ void SimpleLock::remove_cache(MDLockCacheItem& item) {
   }
 }
 
-MDLockCache* SimpleLock::get_first_cache() {
+std::vector<MDLockCache*> SimpleLock::get_active_caches() {
+  std::vector<MDLockCache*> result;
   if (have_more()) {
-    auto& lock_caches = more()->lock_caches;
-    if (!lock_caches.empty()) {
-      return lock_caches.front()->parent;
+    for (auto it = more()->lock_caches.begin_use_current(); !it.end(); ++it) {
+      auto lock_cache = (*it)->parent;
+      if (!lock_cache->invalidating)
+	result.push_back(lock_cache);
     }
   }
-  return nullptr;
+  return result;
 }
diff --git a/src/mds/SimpleLock.h b/src/mds/SimpleLock.h
index 1ad4cd913004..37bdc8d20d94 100644
--- a/src/mds/SimpleLock.h
+++ b/src/mds/SimpleLock.h
@@ -228,7 +228,7 @@ public:
   }
   void add_cache(MDLockCacheItem& item);
   void remove_cache(MDLockCacheItem& item);
-  MDLockCache* get_first_cache();
+  std::vector<MDLockCache*> get_active_caches();
 
   // state
   int get_state() const { return state; }