From: Patrick Donnelly <pdonnell@redhat.com>
Date: Tue, 16 Apr 2024 01:20:15 +0000 (-0400)
Subject: mds: move drop_locks to directly after rdonly check
X-Git-Tag: v20.0.0~2054^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=2931860c4fe115a308c5d3a41d57bc3fbd416f36;p=ceph.git

mds: move drop_locks to directly after rdonly check

The code logic had a serious defect that it would only execute xlock_policylock
once such that a retry would then proceed to executing the vxattr setting.

Fixes: https://tracker.ceph.com/issues/65496
Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>
---

diff --git a/src/mds/Server.cc b/src/mds/Server.cc
index 0bb30664ac5f..c66b55158aef 100644
--- a/src/mds/Server.cc
+++ b/src/mds/Server.cc
@@ -6188,29 +6188,25 @@ void Server::handle_set_vxattr(const MDRequestRef& mdr, CInode *cur)
      * rdlock.
      */
     if (!mdr->more()->rdonly_checks) {
-      if (!(mdr->locking_state & MutationImpl::ALL_LOCKED)) {
-        lov.add_rdlock(&cur->policylock);
-        if (!mds->locker->acquire_locks(mdr, lov))
-          return;
-        mdr->locking_state |= MutationImpl::ALL_LOCKED;
-      }
+      lov.add_rdlock(&cur->policylock);
+      if (!mds->locker->acquire_locks(mdr, lov))
+        return;
+
       bool is_blocked = cur->get_projected_inode()->get_quiesce_block();
       if (is_blocked == val) {
         dout(20) << "already F_QUIESCE_BLOCK set" << dendl;
         respond_to_request(mdr, 0);
         return;
       }
-      mdr->more()->rdonly_checks = true;
-    }
 
-    if ((mdr->locking_state & MutationImpl::ALL_LOCKED) && !mdr->is_xlocked(&cur->policylock)) {
-      /* drop the rdlock and acquire xlocks */
+      mdr->more()->rdonly_checks = true;
       dout(20) << "dropping rdlocks" << dendl;
       mds->locker->drop_locks(mdr.get());
-      if (!xlock_policylock(mdr, cur, false, true))
-        return;
     }
 
+    if (!xlock_policylock(mdr, cur, false, true))
+      return;
+
     /* repeat rdonly checks in case changed between rdlock -> xlock */
     bool is_blocked = cur->get_projected_inode()->get_quiesce_block();
     if (is_blocked == val) {
@@ -6244,29 +6240,25 @@ void Server::handle_set_vxattr(const MDRequestRef& mdr, CInode *cur)
      * rdlock.
      */
     if (!mdr->more()->rdonly_checks) {
-      if (!(mdr->locking_state & MutationImpl::ALL_LOCKED)) {
-        lov.add_rdlock(&cur->snaplock);
-        if (!mds->locker->acquire_locks(mdr, lov))
-          return;
-        mdr->locking_state |= MutationImpl::ALL_LOCKED;
-      }
+      lov.add_rdlock(&cur->snaplock);
+      if (!mds->locker->acquire_locks(mdr, lov))
+        return;
+
       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 */
+      mdr->more()->rdonly_checks = true;
       dout(20) << "dropping rdlocks" << dendl;
       mds->locker->drop_locks(mdr.get());
-      if (!xlock_policylock(mdr, cur, false, true))
-        return;
     }
 
+    if (!xlock_policylock(mdr, cur, false, true))
+      return;
+
     /* repeat rdonly checks in case changed between rdlock -> xlock */
     SnapRealm *realm = cur->find_snaprealm();
     if (val) {