From 2931860c4fe115a308c5d3a41d57bc3fbd416f36 Mon Sep 17 00:00:00 2001 From: Patrick Donnelly Date: Mon, 15 Apr 2024 21:20:15 -0400 Subject: [PATCH] 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 --- src/mds/Server.cc | 40 ++++++++++++++++------------------------ 1 file changed, 16 insertions(+), 24 deletions(-) diff --git a/src/mds/Server.cc b/src/mds/Server.cc index 0bb30664ac5..c66b55158ae 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) { -- 2.39.5