From 0705a831b6fa3592caf22e9a2ed49ac959a950d2 Mon Sep 17 00:00:00 2001
From: "Yan, Zheng" <zyan@redhat.com>
Date: Fri, 31 Jan 2020 16:10:59 +0800
Subject: [PATCH] mds: fix 'can wrlock' check in Locker::acquire_locks()

the check does not match check in Locker::wrlock_start()

Fixes: https://tracker.ceph.com/issues/43908
Signed-off-by: "Yan, Zheng" <zyan@redhat.com>
---
 src/mds/Locker.cc | 58 +++++++++++++++++++++++++++--------------------
 src/mds/Locker.h  |  2 +-
 2 files changed, 34 insertions(+), 26 deletions(-)

diff --git a/src/mds/Locker.cc b/src/mds/Locker.cc
index d98f1bc6bb3..df5e129ec98 100644
--- a/src/mds/Locker.cc
+++ b/src/mds/Locker.cc
@@ -530,20 +530,23 @@ bool Locker::acquire_locks(MDRequestRef& mdr,
 	  continue;
 	}
 	client_t _client = p.is_state_pin() ? lock->get_excl_client() : client;
-	if (p.is_remote_wrlock() && !lock->can_wrlock(_client)) {
-	  marker.message = "failed to wrlock, dropping remote wrlock and waiting";
-	  // can't take the wrlock because the scatter lock is gathering. need to
-	  // release the remote wrlock, so that the gathering process can finish.
-	  ceph_assert(it != mdr->locks.end());
-	  remote_wrlock_finish(it, mdr.get());
-	  remote_wrlock_start(lock, p.wrlock_target, mdr);
-	  goto out;
-	}
-	// nowait if we have already gotten remote wrlock
-	if (!wrlock_start(lock, mdr)) {
-	  ceph_assert(!p.is_remote_wrlock());
-	  marker.message = "failed to wrlock, waiting";
-	  goto out;
+	if (p.is_remote_wrlock()) {
+	  // nowait if we have already gotten remote wrlock
+	  if (!wrlock_try(lock, mdr, _client)) {
+	    marker.message = "failed to wrlock, dropping remote wrlock and waiting";
+	    // can't take the wrlock because the scatter lock is gathering. need to
+	    // release the remote wrlock, so that the gathering process can finish.
+	    ceph_assert(it != mdr->locks.end());
+	    remote_wrlock_finish(it, mdr.get());
+	    remote_wrlock_start(lock, p.wrlock_target, mdr);
+	    goto out;
+	  }
+	} else {
+	  if (!wrlock_start(lock, mdr)) {
+	    ceph_assert(!p.is_remote_wrlock());
+	    marker.message = "failed to wrlock, waiting";
+	    goto out;
+	  }
 	}
 	dout(10) << " got wrlock on " << *lock << " " << *lock->get_parent() << dendl;
       }
@@ -1704,14 +1707,17 @@ void Locker::wrlock_force(SimpleLock *lock, MutationRef& mut)
   mut->emplace_lock(lock, MutationImpl::LockOp::WRLOCK);
 }
 
-bool Locker::wrlock_try(SimpleLock *lock, MutationRef& mut)
+bool Locker::wrlock_try(SimpleLock *lock, const MutationRef& mut, client_t client)
 {
   dout(10) << "wrlock_try " << *lock << " on " << *lock->get_parent() << dendl;
+  if (client == -1)
+    client = mut->get_client();
 
   while (1) {
-    if (lock->can_wrlock(mut->get_client())) {
+    if (lock->can_wrlock(client)) {
       lock->get_wrlock();
-      mut->emplace_lock(lock, MutationImpl::LockOp::WRLOCK);
+      auto it = mut->emplace_lock(lock, MutationImpl::LockOp::WRLOCK);
+      it->flags |= MutationImpl::LockOp::WRLOCK; // may already remote_wrlocked
       return true;
     }
     if (!lock->is_stable())
@@ -1719,16 +1725,18 @@ bool Locker::wrlock_try(SimpleLock *lock, MutationRef& mut)
     CInode *in = static_cast<CInode *>(lock->get_parent());
     if (!in->is_auth())
       break;
-    // don't do nested lock state change if we have dirty scatterdata and
-    // may scatter_writebehind or start_scatter, because nowait==true implies
-    // that the caller already has a log entry open!
+    // caller may already has a log entry open. To avoid calling
+    // scatter_writebehind or start_scatter. don't change nest lock
+    // state if it has dirty scatterdata.
     if (lock->is_dirty())
-      return false;
+      break;
+    // To avoid calling scatter_writebehind or start_scatter. don't
+    // change nest lock state to MIX.
     ScatterLock *slock = static_cast<ScatterLock*>(lock);
-    if (in->has_subtree_or_exporting_dirfrag() || slock->get_scatter_wanted())
-      scatter_mix(slock);
-    else
-      simple_lock(lock);
+    if (slock->get_scatter_wanted() || in->has_subtree_or_exporting_dirfrag())
+      break;
+
+    simple_lock(lock);
   }
   return false;
 }
diff --git a/src/mds/Locker.h b/src/mds/Locker.h
index 2a3f51cf621..3f78bf433b3 100644
--- a/src/mds/Locker.h
+++ b/src/mds/Locker.h
@@ -101,7 +101,7 @@ public:
   bool rdlock_try_set(MutationImpl::LockOpVec& lov, MutationRef& mut);
 
   void wrlock_force(SimpleLock *lock, MutationRef& mut);
-  bool wrlock_try(SimpleLock *lock, MutationRef& mut);
+  bool wrlock_try(SimpleLock *lock, const MutationRef& mut, client_t client=-1);
   bool wrlock_start(const MutationImpl::LockOp &op, MDRequestRef& mut);
   void wrlock_finish(const MutationImpl::lock_iterator& it, MutationImpl *mut, bool *pneed_issue);
 
-- 
2.39.5