From 540c292f2a58eef6b8e6fe8cba11cf039e47ad11 Mon Sep 17 00:00:00 2001
From: Xiubo Li <xiubli@redhat.com>
Date: Sat, 10 Apr 2021 12:52:24 +0800
Subject: [PATCH] client: wait rename to finish

In rare case during the rename if another thread tries to lookup the
dst dentry, it may get an inconsistent result that both src dentry
and dst dentry will link to the same inode at the same time.

Will wait the rename to finish and try it again.

Fixes: https://tracker.ceph.com/issues/49912
Signed-off-by: Xiubo Li <xiubli@redhat.com>
(cherry picked from commit 11fb16df06ad61e228c17feedf00dfb85f0477d5)
---
 src/client/Client.cc | 32 +++++++++++++++++++++++++++++---
 src/client/Client.h  |  5 ++++-
 src/client/Dentry.h  |  1 +
 3 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/src/client/Client.cc b/src/client/Client.cc
index 8613f19ed94..c5173e21a65 100644
--- a/src/client/Client.cc
+++ b/src/client/Client.cc
@@ -1209,6 +1209,11 @@ Dentry *Client::insert_dentry_inode(Dir *dir, const string& dname, LeaseStat *dl
     Inode *diri = dir->parent_inode;
     clear_dir_complete_and_ordered(diri, false);
     dn = link(dir, dname, in, dn);
+
+    if (old_dentry) {
+      dn->is_renaming = false;
+      signal_cond_list(waiting_for_rename);
+    }
   }
 
   update_dentry_lease(dn, dlease, from, session);
@@ -7052,7 +7057,8 @@ bool Client::_dentry_valid(const Dentry *dn)
 }
 
 int Client::_lookup(Inode *dir, const string& dname, int mask, InodeRef *target,
-		    const UserPerm& perms, std::string* alternate_name)
+                    const UserPerm& perms, std::string* alternate_name,
+                    bool is_rename)
 {
   int r = 0;
   Dentry *dn = NULL;
@@ -7131,6 +7137,19 @@ relookup:
     } else {
       ldout(cct, 20) << " no cap on " << dn->inode->vino() << dendl;
     }
+
+    // In rare case during the rename if another thread tries to
+    // lookup the dst dentry, it may get an inconsistent result
+    // that both src dentry and dst dentry will link to the same
+    // inode at the same time.
+    // Will wait the rename to finish and try it again.
+    if (!is_rename && dn->is_renaming) {
+      ldout(cct, 1) << __func__ << " dir " << *dir
+                    << " rename is on the way, will wait for dn '"
+                    << dname << "'" << dendl;
+      wait_on_list(waiting_for_rename);
+      goto relookup;
+    }
   } else {
     // can we conclude ENOENT locally?
     if (dir->caps_issued_mask(CEPH_CAP_FILE_SHARED, true) &&
@@ -14202,12 +14221,13 @@ int Client::_rename(Inode *fromdir, const char *fromname, Inode *todir, const ch
     req->old_dentry_drop = CEPH_CAP_FILE_SHARED;
     req->old_dentry_unless = CEPH_CAP_FILE_EXCL;
 
+    de->is_renaming = true;
     req->set_dentry(de);
     req->dentry_drop = CEPH_CAP_FILE_SHARED;
     req->dentry_unless = CEPH_CAP_FILE_EXCL;
 
     InodeRef oldin, otherin;
-    res = _lookup(fromdir, fromname, 0, &oldin, perm);
+    res = _lookup(fromdir, fromname, 0, &oldin, perm, nullptr, true);
     if (res < 0)
       goto fail;
 
@@ -14216,7 +14236,7 @@ int Client::_rename(Inode *fromdir, const char *fromname, Inode *todir, const ch
     req->set_old_inode(oldinode);
     req->old_inode_drop = CEPH_CAP_LINK_SHARED;
 
-    res = _lookup(todir, toname, 0, &otherin, perm);
+    res = _lookup(todir, toname, 0, &otherin, perm, nullptr, true);
     switch (res) {
     case 0:
       {
@@ -14245,6 +14265,12 @@ int Client::_rename(Inode *fromdir, const char *fromname, Inode *todir, const ch
   res = make_request(req, perm, &target);
   ldout(cct, 10) << "rename result is " << res << dendl;
 
+  // if rename fails it will miss waking up the waiters
+  if (op == CEPH_MDS_OP_RENAME && de->is_renaming) {
+    de->is_renaming = false;
+    signal_cond_list(waiting_for_rename);
+  }
+
   // renamed item from our cache
 
   trim_cache();
diff --git a/src/client/Client.h b/src/client/Client.h
index 97c2ad73378..f71aab33f94 100644
--- a/src/client/Client.h
+++ b/src/client/Client.h
@@ -1320,7 +1320,8 @@ private:
 		 const UserPerm& perms);
 
   int _lookup(Inode *dir, const std::string& dname, int mask, InodeRef *target,
-	      const UserPerm& perm, std::string* alternate_name=nullptr);
+	      const UserPerm& perm, std::string* alternate_name=nullptr,
+              bool is_rename=false);
 
   int _link(Inode *in, Inode *dir, const char *name, const UserPerm& perm, std::string alternate_name,
 	    InodeRef *inp = 0);
@@ -1573,6 +1574,8 @@ private:
   std::map<std::pair<int64_t,std::string>, int> pool_perms;
   std::list<ceph::condition_variable*> waiting_for_pool_perm;
 
+  std::list<ceph::condition_variable*> waiting_for_rename;
+
   uint64_t retries_on_invalidate = 0;
 
   // state reclaim
diff --git a/src/client/Dentry.h b/src/client/Dentry.h
index 94722c5de70..8003dfed34b 100644
--- a/src/client/Dentry.h
+++ b/src/client/Dentry.h
@@ -91,6 +91,7 @@ public:
   ceph_seq_t lease_seq = 0;
   int cap_shared_gen = 0;
   std::string alternate_name;
+  bool is_renaming = false;
 
 private:
   xlist<Dentry *>::item inode_xlist_link;
-- 
2.39.5