revert: mds: provide a mechanism to authpin while freezing

This is a functional revert of a9964a7ccc4394f923fb0f1c76eb8fa03fe8733d
git revert was giving too many conflicts, as the code has changed
too much since the original commit.

The bypass freezing mechanism lead us into several deadlocks,
and when we found out that a freezing inode defers reclaiming
client caps, we realized that we needed to try a different approach.
This commit removes the bypass freezing related changes to clear way
for a different approach to resolving the conflict between quiesce
and freezing.

Signed-off-by: Leonid Usov <leonid.usov@ibm.com>

diff --git a/src/common/TrackedOp.h b/src/common/TrackedOp.h
index 1a921309ca01bd66b60fbaab728ebdaa05866afa..435e7edb028ece0d62fbcc2cf5d144426d65f2ef 100644 (file)
--- a/src/common/TrackedOp.h
+++ b/src/common/TrackedOp.h
@@ -204,13 +204,15 @@ public:
   }
   ~OpTracker();
 
-  template <typename T, typename U>
-  typename T::Ref create_request(U params)
+  // NB: P is ref-like, i.e. `params` should be dereferenced for members
+  template <typename R, typename P>
+  typename R::Ref create_request(P params)
   {
-    constexpr bool has_is_continuous = requires(U u) {
-      { u->is_continuous() } -> std::same_as<bool>;
+    constexpr bool enable_mark_continuous = requires(typename R::Ref r, P p) {
+      { p->is_continuous() } -> std::same_as<bool>;
+      r->mark_continuous();
     };
-    typename T::Ref retval(new T(params, this));
+    typename R::Ref retval(new R(params, this));
     retval->tracking_start();
     if (is_tracking()) {
       retval->mark_event("header_read", params->get_recv_stamp());
@@ -218,12 +220,11 @@ public:
       retval->mark_event("all_read", params->get_recv_complete_stamp());
       retval->mark_event("dispatched", params->get_dispatch_stamp());
     }
-    if constexpr (has_is_continuous) {
+    if constexpr (enable_mark_continuous) {
       if (params->is_continuous()) {
         retval->mark_continuous();
       }
     }
-
     return retval;
   }
 };

diff --git a/src/mds/CDentry.cc b/src/mds/CDentry.cc
index 9f5f1a49f55ec64977285bb2ab55965b512f2993..8694dfc72a2e6f8145e8b803a6e81d23e24696e6 100644 (file)
--- a/src/mds/CDentry.cc
+++ b/src/mds/CDentry.cc
@@ -368,10 +368,10 @@ int CDentry::get_num_dir_auth_pins() const
   return auth_pins;
 }
 
-bool CDentry::can_auth_pin(int *err_ret, bool bypassfreezing) const
+bool CDentry::can_auth_pin(int *err_ret) const
 {
   ceph_assert(dir);
-  return dir->can_auth_pin(err_ret, bypassfreezing);
+  return dir->can_auth_pin(err_ret);
 }
 
 void CDentry::auth_pin(void *by)

diff --git a/src/mds/CDentry.h b/src/mds/CDentry.h
index 837ce2b8818d83409b6b107b4b21c77b091816af..2566395d185617bec6c4e0bbeb6719b2e6cdcdf0 100644 (file)
--- a/src/mds/CDentry.h
+++ b/src/mds/CDentry.h
@@ -222,7 +222,7 @@ public:
   void _put() override;
 
   // auth pins
-  bool can_auth_pin(int *err_ret=nullptr, bool bypassfreezing=false) const override;
+  bool can_auth_pin(int *err_ret=nullptr) const override;
   void auth_pin(void *by) override;
   void auth_unpin(void *by) override;
   void adjust_nested_auth_pins(int diradj, void *by);

diff --git a/src/mds/CDir.cc b/src/mds/CDir.cc
index 025e4d17b81fa0989aebad570ae24b8148dba950..3828fe04d7b2a41af1fc3afeb4c243ba32a34005 100644 (file)
--- a/src/mds/CDir.cc
+++ b/src/mds/CDir.cc
@@ -3450,23 +3450,16 @@ void CDir::adjust_freeze_after_rename(CDir *dir)
   mdcache->mds->queue_waiters(unfreeze_waiters);
 }
 
-bool CDir::can_auth_pin(int *err_ret, bool bypassfreezing) const
+bool CDir::can_auth_pin(int *err_ret) const
 {
   int err;
   if (!is_auth()) {
     err = ERR_NOT_AUTH;
-  } else if (is_freezing_dir()) {
-    if (bypassfreezing) {
-      dout(20) << "allowing authpin with freezing" << dendl;
-      err = 0;
-    } else {
-      err = ERR_FRAGMENTING_DIR;
-    }
-  } else if (is_frozen_dir()) {
+  } else if (is_freezing_dir() || is_frozen_dir()) {
     err = ERR_FRAGMENTING_DIR;
   } else {
     auto p = is_freezing_or_frozen_tree();
-    if (p.first && !bypassfreezing) {
+    if (p.first) {
       err = ERR_EXPORTING_TREE;
     } else if (p.second) {
       err = ERR_EXPORTING_TREE;

diff --git a/src/mds/CDir.h b/src/mds/CDir.h
index b70725f714ed7bc368706ea9d135e6bb1cc6dd8a..76ac7e21cc0902229ea7f067d29298e956bfe23d 100644 (file)
--- a/src/mds/CDir.h
+++ b/src/mds/CDir.h
@@ -527,7 +527,7 @@ public:
   void abort_import();
 
   // -- auth pins --
-  bool can_auth_pin(int *err_ret=nullptr, bool bypassfreezing=false) const override;
+  bool can_auth_pin(int *err_ret=nullptr) const override;
   int get_auth_pins() const { return auth_pins; }
   int get_dir_auth_pins() const { return dir_auth_pins; }
   void auth_pin(void *who) override;

diff --git a/src/mds/CInode.cc b/src/mds/CInode.cc
index 49a2abd83b42dc1bc248d7b03244fb42b7d7d62f..8fe15c0047fc3a3b1aefe2b5bca08211603f2109 100644 (file)
--- a/src/mds/CInode.cc
+++ b/src/mds/CInode.cc
@@ -2970,22 +2970,15 @@ void CInode::clear_ambiguous_auth()
 }
 
 // auth_pins
-bool CInode::can_auth_pin(int *err_ret, bool bypassfreezing) const {
+bool CInode::can_auth_pin(int *err_ret) const {
   int err;
   if (!is_auth()) {
     err = ERR_NOT_AUTH;
-  } else if (is_freezing_inode()) {
-    if (bypassfreezing) {
-      dout(20) << "allowing authpin with freezing" << dendl;
-      err = 0;
-    } else {
-      err = ERR_EXPORTING_INODE;
-    }
-  } else if (is_frozen_inode() || is_frozen_auth_pin()) {
+  } else if (is_freezing_inode() || is_frozen_inode() || is_frozen_auth_pin()) {
     err = ERR_EXPORTING_INODE;
   } else {
     if (parent)
-      return parent->can_auth_pin(err_ret, bypassfreezing);
+      return parent->can_auth_pin(err_ret);
     err = 0;
   }
   if (err && err_ret)

diff --git a/src/mds/CInode.h b/src/mds/CInode.h
index 45555a07d8b8cdd4e6683c984e54497b043fa001..254ca7efc65434f5003f5554475dce28b157a01d 100644 (file)
--- a/src/mds/CInode.h
+++ b/src/mds/CInode.h
@@ -930,7 +930,7 @@ class CInode : public MDSCacheObject, public InodeStoreBase, public Counter<CIno
   mds_authority_t authority() const override;
 
   // -- auth pins --
-  bool can_auth_pin(int *err_ret=nullptr, bool bypassfreezing=false) const override;
+  bool can_auth_pin(int *err_ret=nullptr) const override;
   void auth_pin(void *by) override;
   void auth_unpin(void *by) override;
 

diff --git a/src/mds/Locker.cc b/src/mds/Locker.cc
index 294683c0717fcff87411a94cf1e1f68a97a40af0..d2b9f24f2ee218006af7a555ea44bb46b3c7a2dd 100644 (file)
--- a/src/mds/Locker.cc
+++ b/src/mds/Locker.cc
@@ -424,7 +424,7 @@ bool Locker::acquire_locks(const MDRequestRef& mdr,
       continue;
     }
     int err = 0;
-    if (!object->can_auth_pin(&err, skip_quiesce)) {
+    if (!object->can_auth_pin(&err)) {
       if (mdr->lock_cache) {
        CDir *dir;
        if (CInode *in = dynamic_cast<CInode*>(object)) {
@@ -521,8 +521,6 @@ bool Locker::acquire_locks(const MDRequestRef& mdr,
       }
       if (auth_pin_nonblocking)
        req->mark_nonblocking();
-      if (skip_quiesce)
-       req->mark_bypassfreezing();
       else if (!mdr->locks.empty())
        req->mark_notify_blocking();
 

diff --git a/src/mds/MDSCacheObject.h b/src/mds/MDSCacheObject.h
index 7bc820e4234bd1e5035b7403d1408ccbf506a74b..d322a05851a56fcf8f2c7e3669cf1246a97e2a34 100644 (file)
--- a/src/mds/MDSCacheObject.h
+++ b/src/mds/MDSCacheObject.h
@@ -220,7 +220,7 @@ class MDSCacheObject {
     ERR_FRAGMENTING_DIR,
     ERR_EXPORTING_INODE,
   };
-  virtual bool can_auth_pin(int *err_code=nullptr, bool bypassfreezing=false) const = 0;
+  virtual bool can_auth_pin(int *err_code=nullptr) const = 0;
   virtual void auth_pin(void *who) = 0;
   virtual void auth_unpin(void *who) = 0;
   virtual bool is_frozen() const = 0;

diff --git a/src/mds/Mutation.cc b/src/mds/Mutation.cc
index e94a988703f05d2c3825511d4a631f50d549924c..b7769319cb1cf7c233a0e24f9dc32f3d5858643c 100644 (file)
--- a/src/mds/Mutation.cc
+++ b/src/mds/Mutation.cc
@@ -364,9 +364,9 @@ void MDRequestImpl::clear_ambiguous_auth()
   more()->is_ambiguous_auth = false;
 }
 
-bool MDRequestImpl::can_auth_pin(MDSCacheObject *object, bool bypassfreezing)
+bool MDRequestImpl::can_auth_pin(MDSCacheObject *object)
 {
-  return object->can_auth_pin(nullptr, bypassfreezing) ||
+  return object->can_auth_pin(nullptr) ||
          (is_auth_pinned(object) && has_more() &&
          more()->is_freeze_authpin &&
          more()->rename_inode == object);

diff --git a/src/mds/Mutation.h b/src/mds/Mutation.h
index d7d74de82dfd263fd603f83be501569da7a21ae5..e1b905b33d8cb62ee77fe22ce16b233bcb8110c6 100644 (file)
--- a/src/mds/Mutation.h
+++ b/src/mds/Mutation.h
@@ -405,7 +405,7 @@ struct MDRequestImpl : public MutationImpl {
   bool freeze_auth_pin(CInode *inode);
   void unfreeze_auth_pin(bool clear_inode=false);
   void set_remote_frozen_auth_pin(CInode *inode);
-  bool can_auth_pin(MDSCacheObject *object, bool bypassfreezing=false);
+  bool can_auth_pin(MDSCacheObject *object);
   void drop_local_auth_pins();
   void set_ambiguous_auth(CInode *inode);
   void clear_ambiguous_auth();

diff --git a/src/mds/Server.cc b/src/mds/Server.cc
index 589abf9e8de7b5fb71d9bfb969d3f457e607eff6..e46f69b3297caf2ef74320a8b48d9438666144d8 100644 (file)
--- a/src/mds/Server.cc
+++ b/src/mds/Server.cc
@@ -3171,13 +3171,9 @@ void Server::handle_peer_auth_pin(const MDRequestRef& mdr)
   list<MDSCacheObject*> objects;
   CInode *auth_pin_freeze = NULL;
   bool nonblocking = mdr->peer_request->is_nonblocking();
-  bool bypassfreezing = mdr->peer_request->is_bypassfreezing();
   bool fail = false, wouldblock = false, readonly = false;
   ref_t<MMDSPeerRequest> reply;
 
-  dout(15) << " nonblocking=" << nonblocking
-           << " bypassfreezing=" << bypassfreezing << dendl;
-
   if (mdcache->is_readonly()) {
     dout(10) << " read-only FS" << dendl;
     readonly = true;
@@ -3209,7 +3205,7 @@ void Server::handle_peer_auth_pin(const MDRequestRef& mdr)
       }
       if (mdr->is_auth_pinned(obj))
        continue;
-      if (!mdr->can_auth_pin(obj, bypassfreezing)) {
+      if (!mdr->can_auth_pin(obj)) {
        if (nonblocking) {
          dout(10) << " can't auth_pin (freezing?) " << *obj << " nonblocking" << dendl;
          fail = true;

diff --git a/src/messages/MMDSPeerRequest.h b/src/messages/MMDSPeerRequest.h
index 1a007364d4abbc9df0cb9cf2664c054bc43b072c..1799ab361a11587cac19e4d0c2c4bb40a06d1bb6 100644 (file)
--- a/src/messages/MMDSPeerRequest.h
+++ b/src/messages/MMDSPeerRequest.h
@@ -106,7 +106,6 @@ public:
   static constexpr unsigned FLAG_INTERRUPTED   = 1<<5;
   static constexpr unsigned FLAG_NOTIFYBLOCKING        = 1<<6;
   static constexpr unsigned FLAG_REQBLOCKED    = 1<<7;
-  static constexpr unsigned FLAG_BYPASSFREEZING = 1<<8;
 
   // for locking
   __u16 lock_type;  // lock object type
@@ -161,8 +160,6 @@ public:
   void clear_notify_blocking() const { flags &= ~FLAG_NOTIFYBLOCKING; }
   bool is_req_blocked() const { return (flags & FLAG_REQBLOCKED); }
   void mark_req_blocked() { flags |= FLAG_REQBLOCKED; }
-  bool is_bypassfreezing() const { return (flags & FLAG_BYPASSFREEZING); }
-  void mark_bypassfreezing() { flags |= FLAG_BYPASSFREEZING; }
 
   void set_lock_type(int t) { lock_type = t; }
   const ceph::buffer::list& get_lock_data() const { return inode_export; }