mds: wait reintegrate to finish when unlinking

When unlinking a remote dentry while primary dentry has been just
unlinked and was trying to reintegrate, which will finally be a
RENAME client request to current MDS, the stray dentry.

If the RENAME request is sleeping and waiting for some resources
just after authpin the CInode, so the unlinking request will mark
the remote dentry as UNLINKING state and then add itself to the
wait list just waiting for the CInode to be authpined.

Then after the RENAME request is retried later it will add to a
wait list again and waiting for the UNLINKING state to be cleared.
And finally the RENAME and UNLINK requests will be deadlock.

This fix will block the UNLINK request if the reintegrate_stray
is going on for current dentry.

Fixes: https://tracker.ceph.com/issues/58340
Signed-off-by: Xiubo Li <xiubli@redhat.com>
(cherry picked from commit c3b36723defae32b0a3ef1ac37b58ec57d356b65)

diff --git a/src/mds/CDentry.cc b/src/mds/CDentry.cc
index 9846f74856133417137b4f9258d546da9207e6b6..aac7c17eda3f693260a5c4794cbe31a6aae93f9f 100644 (file)
--- a/src/mds/CDentry.cc
+++ b/src/mds/CDentry.cc
@@ -108,6 +108,8 @@ ostream& operator<<(ostream& out, const CDentry& dn)
   out << " state=" << dn.get_state();
   if (dn.is_new()) out << "|new";
   if (dn.state_test(CDentry::STATE_BOTTOMLRU)) out << "|bottomlru";
+  if (dn.state_test(CDentry::STATE_UNLINKING)) out << "|unlinking";
+  if (dn.state_test(CDentry::STATE_REINTEGRATING)) out << "|reintegrating";
 
   if (dn.get_num_ref()) {
     out << " |";

diff --git a/src/mds/CDentry.h b/src/mds/CDentry.h
index 03932285cf7614dcb7851424fa516c851cc3b9a8..c7f83d1710a254aa4a4bfaae3c95329fcaa0b1d8 100644 (file)
--- a/src/mds/CDentry.h
+++ b/src/mds/CDentry.h
@@ -88,6 +88,7 @@ public:
   static const int STATE_PURGINGPINNED =  (1<<5);
   static const int STATE_BOTTOMLRU =    (1<<6);
   static const int STATE_UNLINKING =    (1<<7);
+  static const int STATE_REINTEGRATING = (1<<8);
   // stray dentry needs notification of releasing reference
   static const int STATE_STRAY =       STATE_NOTIFYREF;
   static const int MASK_STATE_IMPORT_KEPT = STATE_BOTTOMLRU;
@@ -101,8 +102,9 @@ public:
 
   static const unsigned EXPORT_NONCE = 1;
 
-  const static uint64_t WAIT_UNLINK_STATE  = (1<<0);
-  const static uint64_t WAIT_UNLINK_FINISH = (1<<1);
+  const static uint64_t WAIT_UNLINK_STATE       = (1<<0);
+  const static uint64_t WAIT_UNLINK_FINISH      = (1<<1);
+  const static uint64_t WAIT_REINTEGRATE_FINISH = (1<<2);
   uint32_t replica_unlinking_ref = 0;
 
   CDentry(std::string_view n, __u32 h,

diff --git a/src/mds/MDSRank.cc b/src/mds/MDSRank.cc
index 6eb581723b2e7ed83e47600124435b4fd69458e7..77eee2e02c4216d46b004e9a000cc07533c92136 100644 (file)
--- a/src/mds/MDSRank.cc
+++ b/src/mds/MDSRank.cc
@@ -1178,6 +1178,7 @@ bool MDSRank::is_valid_message(const cref_t<Message> &m) {
       type == CEPH_MSG_CLIENT_RECONNECT ||
       type == CEPH_MSG_CLIENT_RECLAIM ||
       type == CEPH_MSG_CLIENT_REQUEST ||
+      type == CEPH_MSG_CLIENT_REPLY ||
       type == MSG_MDS_PEER_REQUEST ||
       type == MSG_MDS_HEARTBEAT ||
       type == MSG_MDS_TABLE_REQUEST ||
@@ -1231,6 +1232,7 @@ void MDSRank::handle_message(const cref_t<Message> &m)
       ALLOW_MESSAGES_FROM(CEPH_ENTITY_TYPE_CLIENT);
       // fall-thru
     case CEPH_MSG_CLIENT_REQUEST:
+    case CEPH_MSG_CLIENT_REPLY:
       server->dispatch(m);
       break;
     case MSG_MDS_PEER_REQUEST:

diff --git a/src/mds/MDSRank.h b/src/mds/MDSRank.h
index cef9ccc6cfca0bf0a2f90976d894aae59249199e..e4bb0c7c9bbc50333c5e93b0ceaf67a014277b8a 100644 (file)
--- a/src/mds/MDSRank.h
+++ b/src/mds/MDSRank.h
@@ -150,6 +150,29 @@ class Finisher;
 class ScrubStack;
 class C_ExecAndReply;
 
+struct MDSMetaRequest {
+private:
+  int _op;
+  CDentry *_dentry;
+  ceph_tid_t _tid;
+public:
+  explicit MDSMetaRequest(int op, CDentry *dn, ceph_tid_t tid) :
+    _op(op), _dentry(dn), _tid(tid) {
+    if (_dentry) {
+      _dentry->get(CDentry::PIN_PURGING);
+    }
+  }
+  ~MDSMetaRequest() {
+    if (_dentry) {
+      _dentry->put(CDentry::PIN_PURGING);
+    }
+  }
+
+  CDentry *get_dentry() { return _dentry; }
+  int get_op() { return _op; }
+  ceph_tid_t get_tid() { return _tid; }
+};
+
 /**
  * The public part of this class's interface is what's exposed to all
  * the various subsystems (server, mdcache, etc), such as pointers
@@ -414,6 +437,8 @@ class MDSRank {
     PerfCounters *logger = nullptr, *mlogger = nullptr;
     OpTracker op_tracker;
 
+    std::map<ceph_tid_t, MDSMetaRequest> internal_client_requests;
+
     // The last different state I held before current
     MDSMap::DaemonState last_state = MDSMap::STATE_BOOT;
     // The state assigned to me by the MDSMap

diff --git a/src/mds/Server.cc b/src/mds/Server.cc
index a4b3ef60904b7e7b368c364ee683c3950e9a13e3..a63cbd1703aefc956720794fb63bbfd470cd96fd 100644 (file)
--- a/src/mds/Server.cc
+++ b/src/mds/Server.cc
@@ -31,6 +31,7 @@
 #include "Mutation.h"
 #include "MetricsHandler.h"
 #include "cephfs_features.h"
+#include "MDSContext.h"
 
 #include "msg/Messenger.h"
 
@@ -357,6 +358,9 @@ void Server::dispatch(const cref_t<Message> &m)
   case CEPH_MSG_CLIENT_REQUEST:
     handle_client_request(ref_cast<MClientRequest>(m));
     return;
+  case CEPH_MSG_CLIENT_REPLY:
+    handle_client_reply(ref_cast<MClientReply>(m));
+    return;
   case CEPH_MSG_CLIENT_RECLAIM:
     handle_client_reclaim(ref_cast<MClientReclaim>(m));
     return;
@@ -2265,6 +2269,10 @@ void Server::reply_client_request(MDRequestRef& mdr, const ref_t<MClientReply> &
     mds->send_message_client(reply, session);
   }
 
+  if (client_inst.name.is_mds() && reply->get_op() == CEPH_MDS_OP_RENAME) {
+    mds->send_message(reply, mdr->client_request->get_connection());
+  }
+
   if (req->is_queued_for_replay() &&
       (mdr->has_completed || reply->get_result() < 0)) {
     if (reply->get_result() < 0) {
@@ -2496,6 +2504,38 @@ void Server::handle_client_request(const cref_t<MClientRequest> &req)
   return;
 }
 
+void Server::handle_client_reply(const cref_t<MClientReply> &reply)
+{
+  dout(4) << "handle_client_reply " << *reply << dendl;
+
+  ceph_assert(reply->is_safe());
+  ceph_tid_t tid = reply->get_tid();
+
+  if (mds->internal_client_requests.count(tid) == 0) {
+    dout(1) << " no pending request on tid " << tid << dendl;
+    return;
+  }
+
+  auto &req = mds->internal_client_requests.at(tid);
+  CDentry *dn = req.get_dentry();
+
+  switch (reply->get_op()) {
+  case CEPH_MDS_OP_RENAME:
+    if (dn) {
+      dn->state_clear(CDentry::STATE_REINTEGRATING);
+
+      MDSContext::vec finished;
+      dn->take_waiting(CDentry::WAIT_REINTEGRATE_FINISH, finished);
+      mds->queue_waiters(finished);
+    }
+    break;
+  default:
+    dout(5) << " unknown client op " << reply->get_op() << dendl;
+  }
+
+  mds->internal_client_requests.erase(tid);
+}
+
 void Server::handle_osd_map()
 {
   /* Note that we check the OSDMAP_FULL flag directly rather than
@@ -6681,6 +6721,45 @@ void Server::wait_for_pending_unlink(CDentry *dn, MDRequestRef& mdr)
   dn->add_waiter(CDentry::WAIT_UNLINK_FINISH, new C_WaitUnlinkToFinish(mdcache, dn, fin));
 }
 
+struct C_WaitReintegrateToFinish : public MDSContext {
+protected:
+  MDCache *mdcache;
+  CDentry *dn;
+  MDSContext *fin;
+
+  MDSRank *get_mds() override
+  {
+    ceph_assert(mdcache != NULL);
+    return mdcache->mds;
+  }
+
+public:
+  C_WaitReintegrateToFinish(MDCache *m, CDentry *d, MDSContext *f) :
+    mdcache(m), dn(d), fin(f) {}
+  void finish(int r) override {
+    fin->complete(r);
+    dn->put(CDentry::PIN_PURGING);
+  }
+};
+
+bool Server::is_reintegrate_pending(CDentry *dn)
+{
+  CDentry::linkage_t *dnl = dn->get_projected_linkage();
+  if (!dnl->is_null() && dn->state_test(CDentry::STATE_REINTEGRATING)) {
+      return true;
+  }
+  return false;
+}
+
+void Server::wait_for_pending_reintegrate(CDentry *dn, MDRequestRef& mdr)
+{
+  dout(20) << __func__ << " dn " << *dn << dendl;
+  mds->locker->drop_locks(mdr.get());
+  auto fin = new C_MDS_RetryRequest(mdcache, mdr);
+  dn->get(CDentry::PIN_PURGING);
+  dn->add_waiter(CDentry::WAIT_REINTEGRATE_FINISH, new C_WaitReintegrateToFinish(mdcache, dn, fin));
+}
+
 // MKNOD
 
 class C_MDS_mknod_finish : public ServerLogContext {
@@ -7736,6 +7815,11 @@ void Server::handle_client_unlink(MDRequestRef& mdr)
   if (!dn)
     return;
 
+  if (is_reintegrate_pending(dn)) {
+    wait_for_pending_reintegrate(dn, mdr);
+    return;
+  }
+
   // notify replica MDSes the dentry is under unlink
   if (!dn->state_test(CDentry::STATE_UNLINKING)) {
     dn->state_set(CDentry::STATE_UNLINKING);

diff --git a/src/mds/Server.h b/src/mds/Server.h
index 5e3318e5a5e37e28c4c8a63cfe8f65cb60307cd6..c434fee9f2915b8b70028448382972079e368c18 100644 (file)
--- a/src/mds/Server.h
+++ b/src/mds/Server.h
@@ -158,6 +158,7 @@ public:
 
   // -- requests --
   void handle_client_request(const cref_t<MClientRequest> &m);
+  void handle_client_reply(const cref_t<MClientReply> &m);
 
   void journal_and_reply(MDRequestRef& mdr, CInode *tracei, CDentry *tracedn,
                         LogEvent *le, MDSLogContextBase *fin);
@@ -237,6 +238,9 @@ public:
   bool is_unlink_pending(CDentry *dn);
   void wait_for_pending_unlink(CDentry *dn, MDRequestRef& mdr);
 
+  bool is_reintegrate_pending(CDentry *dn);
+  void wait_for_pending_reintegrate(CDentry *dn, MDRequestRef& mdr);
+
   // open
   void handle_client_open(MDRequestRef& mdr);
   void handle_client_openc(MDRequestRef& mdr);  // O_CREAT variant.

diff --git a/src/mds/StrayManager.cc b/src/mds/StrayManager.cc
index 61e4f23fd7a3b42fe163e08cecfb401ca7db734b..412192b3a3acf991208bc37c3c95b4f9872fc2df 100644 (file)
--- a/src/mds/StrayManager.cc
+++ b/src/mds/StrayManager.cc
@@ -670,19 +670,27 @@ void StrayManager::reintegrate_stray(CDentry *straydn, CDentry *rdn)
   dout(10) << __func__ << " " << *straydn << " to " << *rdn << dendl;
 
   logger->inc(l_mdc_strays_reintegrated);
-  
+
   // rename it to remote linkage .
   filepath src(straydn->get_name(), straydn->get_dir()->ino());
   filepath dst(rdn->get_name(), rdn->get_dir()->ino());
 
+  ceph_tid_t tid = mds->issue_tid();
+
   auto req = make_message<MClientRequest>(CEPH_MDS_OP_RENAME);
   req->set_filepath(dst);
   req->set_filepath2(src);
-  req->set_tid(mds->issue_tid());
+  req->set_tid(tid);
+
+  rdn->state_set(CDentry::STATE_REINTEGRATING);
+  mds->internal_client_requests.emplace(std::piecewise_construct,
+                                        std::make_tuple(tid),
+                                        std::make_tuple(CEPH_MDS_OP_RENAME,
+                                                        rdn, tid));
 
   mds->send_message_mds(req, rdn->authority().first);
 }
- 
+
 void StrayManager::migrate_stray(CDentry *dn, mds_rank_t to)
 {
   dout(10) << __func__ << " " << *dn << " to mds." << to << dendl;
@@ -696,10 +704,17 @@ void StrayManager::migrate_stray(CDentry *dn, mds_rank_t to)
   filepath src(dn->get_name(), dirino);
   filepath dst(dn->get_name(), MDS_INO_STRAY(to, MDS_INO_STRAY_INDEX(dirino)));
 
+  ceph_tid_t tid = mds->issue_tid();
+
   auto req = make_message<MClientRequest>(CEPH_MDS_OP_RENAME);
   req->set_filepath(dst);
   req->set_filepath2(src);
-  req->set_tid(mds->issue_tid());
+  req->set_tid(tid);
+
+  mds->internal_client_requests.emplace(std::piecewise_construct,
+                                        std::make_tuple(tid),
+                                        std::make_tuple(CEPH_MDS_OP_RENAME,
+                                                        nullptr, tid));
 
   mds->send_message_mds(req, to);
 }