From 8b98796cb3ffe5d2793ec7632b452a06a60a839a Mon Sep 17 00:00:00 2001 From: Leonid Usov Date: Tue, 21 May 2024 01:03:15 +0300 Subject: [PATCH] mds/quiesce: don't force a remote authpin for the quiesce lock Fixes: https://tracker.ceph.com/issues/66152 Fixes: https://tracker.ceph.com/issues/66123 Signed-off-by: Leonid Usov --- qa/tasks/cephfs/test_quiesce.py | 1 - src/mds/Locker.cc | 7 +++++-- src/mds/Locker.h | 1 - src/mds/MDCache.cc | 6 +++--- src/mds/Server.cc | 2 +- 5 files changed, 9 insertions(+), 8 deletions(-) diff --git a/qa/tasks/cephfs/test_quiesce.py b/qa/tasks/cephfs/test_quiesce.py index b6ef8cde324bc..bf128d8d74f65 100644 --- a/qa/tasks/cephfs/test_quiesce.py +++ b/qa/tasks/cephfs/test_quiesce.py @@ -687,7 +687,6 @@ class TestQuiesceMultiRank(QuiesceTestCase): op = self.fs.rank_tell(["quiesce", "path", self.subvolume, '--wait'], rank=0)['op'] self.assertEqual(op['result'], -1) # EPERM - @unittest.skip("https://tracker.ceph.com/issues/66152") def test_quiesce_drops_remote_authpins_on_failure(self): """ That remote authpins are dropped when the request fails to acquire the quiesce lock diff --git a/src/mds/Locker.cc b/src/mds/Locker.cc index 02115453027d6..b84f7b59ee538 100644 --- a/src/mds/Locker.cc +++ b/src/mds/Locker.cc @@ -230,7 +230,6 @@ struct MarkEventOnDestruct { bool Locker::acquire_locks(const MDRequestRef& mdr, MutationImpl::LockOpVec& lov, CInode* auth_pin_freeze, - std::set mustpin, bool auth_pin_nonblocking, bool skip_quiesce) { @@ -245,6 +244,7 @@ bool Locker::acquire_locks(const MDRequestRef& mdr, client_t client = mdr->get_client(); + std::set mustpin; if (auth_pin_freeze) mustpin.insert(auth_pin_freeze); @@ -294,7 +294,10 @@ bool Locker::acquire_locks(const MDRequestRef& mdr, dout(20) << " must xlock " << *lock << " " << *object << dendl; - mustpin.insert(object); + // only take the authpin for the quiesce lock on the auth + if (lock->get_type() != CEPH_LOCK_IQUIESCE || object->is_auth()) { + mustpin.insert(object); + } // augment xlock with a versionlock? if (lock->get_type() == CEPH_LOCK_DN) { diff --git a/src/mds/Locker.h b/src/mds/Locker.h index a1ea4a260989c..aa037ac6abda7 100644 --- a/src/mds/Locker.h +++ b/src/mds/Locker.h @@ -54,7 +54,6 @@ public: bool acquire_locks(const MDRequestRef& mdr, MutationImpl::LockOpVec& lov, CInode *auth_pin_freeze=NULL, - std::set mustpin = {}, bool auth_pin_nonblocking=false, bool skip_quiesce=false); diff --git a/src/mds/MDCache.cc b/src/mds/MDCache.cc index 554616c333799..c6cfeb15466c1 100644 --- a/src/mds/MDCache.cc +++ b/src/mds/MDCache.cc @@ -12090,7 +12090,7 @@ void MDCache::dispatch_fragment_dir(const MDRequestRef& mdr, bool abort_if_freez // prevent a racing gather on any other scatterlocks too lov.lock_scatter_gather(&diri->nestlock); lov.lock_scatter_gather(&diri->filelock); - if (mds->locker->acquire_locks(mdr, lov, NULL, {}, true)) { + if (mds->locker->acquire_locks(mdr, lov, NULL, true)) { mdr->locking_state |= MutationImpl::ALL_LOCKED; } else { if (!mdr->aborted) { @@ -13697,7 +13697,7 @@ void MDCache::dispatch_quiesce_inode(const MDRequestRef& mdr) // as a result of the auth taking the above locks. } - if (!mds->locker->acquire_locks(mdr, lov, nullptr, {in}, false, true)) { + if (!mds->locker->acquire_locks(mdr, lov, nullptr, false, true)) { return; } mdr->locking_state |= MutationImpl::ALL_LOCKED; @@ -14030,7 +14030,7 @@ void MDCache::dispatch_lock_path(const MDRequestRef& mdr) } } - if (!mds->locker->acquire_locks(mdr, lov, lps.config.ap_freeze ? in : nullptr, {in}, lps.config.ap_dont_block, true)) { + if (!mds->locker->acquire_locks(mdr, lov, lps.config.ap_freeze ? in : nullptr, lps.config.ap_dont_block, true)) { if (lps.config.ap_dont_block && mdr->aborted) { mds->server->respond_to_request(mdr, -CEPHFS_EWOULDBLOCK); } diff --git a/src/mds/Server.cc b/src/mds/Server.cc index 681663cb65276..918cc4e613c1d 100644 --- a/src/mds/Server.cc +++ b/src/mds/Server.cc @@ -3105,7 +3105,7 @@ void Server::dispatch_peer_request(const MDRequestRef& mdr) // will want to take other locks is to prevent issuing unwanted client capabilities, // but since replicas can't issue capabilities, it should be fine allowing remote locks // without taking the quiesce lock. - if (!mds->locker->acquire_locks(mdr, lov, nullptr, {}, false, true)) + if (!mds->locker->acquire_locks(mdr, lov, nullptr, false, true)) return; // ack -- 2.39.5