From c55a5ceb197cfb8f8bd9528d493420ab654f0803 Mon Sep 17 00:00:00 2001 From: Patrick Donnelly Date: Fri, 18 Jun 2021 09:27:54 -0700 Subject: [PATCH] mds: avoid journaling overhead for ceph.dir.subvolume for no-op case In preparation for acquiring the xlock on the directory inode, the MDS must journal a few events before continuing on with the setvxattr. This can cause significant delays in the volumes ceph-mgr module which needs to regularly enable this vxattr from multiple code paths. We could cache in that module whether the vxattr is set but it's also pretty easy to adjust the MDS to acquire a rdlock on the directory to check if the subvolume flag is already set. That is much lighter weight and the lock is generally readily available. Fixes: https://tracker.ceph.com/issues/51276 Signed-off-by: Patrick Donnelly (cherry picked from commit b5f736eee408c220ffdfb67b10667a7b553dac25) --- src/mds/Mutation.h | 1 + src/mds/Server.cc | 31 +++++++++++++++++++++++++++++-- 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/src/mds/Mutation.h b/src/mds/Mutation.h index 4b2ea17114a97..536f6e806f519 100644 --- a/src/mds/Mutation.h +++ b/src/mds/Mutation.h @@ -304,6 +304,7 @@ struct MDRequestImpl : public MutationImpl { bool is_ambiguous_auth = false; bool is_remote_frozen_authpin = false; bool is_inode_exporter = false; + bool rdonly_checks = false; map > imported_session_map; map > cap_imports; diff --git a/src/mds/Server.cc b/src/mds/Server.cc index e4b9c28e3fe1e..35c9005b19477 100644 --- a/src/mds/Server.cc +++ b/src/mds/Server.cc @@ -5637,9 +5637,36 @@ void Server::handle_set_vxattr(MDRequestRef& mdr, CInode *cur) return; } - if (!xlock_policylock(mdr, cur, false, true)) - return; + /* Verify it's not already a subvolume with lighter weight + * rdlock. + */ + if (!mdr->more()->rdonly_checks) { + if (!(mdr->locking_state & MutationImpl::ALL_LOCKED)) { + MutationImpl::LockOpVec lov; + lov.add_rdlock(&cur->snaplock); + if (!mds->locker->acquire_locks(mdr, lov)) + return; + mdr->locking_state |= MutationImpl::ALL_LOCKED; + } + SnapRealm *realm = cur->find_snaprealm(); + const auto srnode = cur->get_projected_srnode(); + if (val == (srnode && srnode->is_subvolume())) { + dout(20) << "already marked subvolume" << dendl; + respond_to_request(mdr, 0); + return; + } + mdr->more()->rdonly_checks = true; + } + + if ((mdr->locking_state & MutationImpl::ALL_LOCKED) && !mdr->is_xlocked(&cur->snaplock)) { + /* drop the rdlock and acquire xlocks */ + dout(20) << "dropping rdlocks" << dendl; + mds->locker->drop_locks(mdr.get()); + if (!xlock_policylock(mdr, cur, false, true)) + return; + } + /* repeat rdonly checks in case changed between rdlock -> xlock */ SnapRealm *realm = cur->find_snaprealm(); if (val) { inodeno_t subvol_ino = realm->get_subvolume_ino(); -- 2.39.5