From 20d21cff89e1bb2bd6d7af311b4cc3272ce5fe65 Mon Sep 17 00:00:00 2001 From: "Yan, Zheng" Date: Thu, 5 Mar 2020 22:54:50 +0800 Subject: [PATCH] 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" --- src/mds/Locker.cc | 10 +++++++--- src/mds/Mutation.cc | 7 +++++-- src/mds/Mutation.h | 3 ++- src/mds/SimpleLock.cc | 12 +++++++----- src/mds/SimpleLock.h | 2 +- 5 files changed, 22 insertions(+), 12 deletions(-) diff --git a/src/mds/Locker.cc b/src/mds/Locker.cc index f8f99342c47..fc8007db2bb 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 03555b4f098..d9f3f7cadcc 100644 --- a/src/mds/Mutation.cc +++ b/src/mds/Mutation.cc @@ -578,7 +578,7 @@ void MDLockCache::attach_dirfrags(std::vector&& 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 d79b430de66..0b4070420d3 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&& 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 1f3b732169b..7c447419bb6 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 SimpleLock::get_active_caches() { + std::vector 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 1ad4cd91300..37bdc8d20d9 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 get_active_caches(); // state int get_state() const { return state; }