From: Patrick Donnelly <pdonnell@redhat.com>
Date: Fri, 18 Jun 2021 16:27:54 +0000 (-0700)
Subject: mds: avoid journaling overhead for ceph.dir.subvolume for no-op case
X-Git-Tag: v17.1.0~1573^2
X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=b5f736eee408c220ffdfb67b10667a7b553dac25;p=ceph.git

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 <pdonnell@redhat.com>
---

diff --git a/src/mds/Mutation.h b/src/mds/Mutation.h
index 9ce55270e5993..09ebe7052a90f 100644
--- a/src/mds/Mutation.h
+++ b/src/mds/Mutation.h
@@ -308,6 +308,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;
 
     std::map<client_t, std::pair<Session*, uint64_t> > imported_session_map;
     std::map<CInode*, std::map<client_t,Capability::Export> > cap_imports;
diff --git a/src/mds/Server.cc b/src/mds/Server.cc
index 63ebc303a044e..856d06130f860 100644
--- a/src/mds/Server.cc
+++ b/src/mds/Server.cc
@@ -5689,9 +5689,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();