squid: 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.

Fixes: https://tracker.ceph.com/issues/65716
Signed-off-by: Leonid Usov <leonid.usov@ibm.com>
(cherry picked from commit bf760602a4f02cc07072db2da5cb987e3072afce)
Fixes: https://tracker.ceph.com/issues/66154

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 3a778e4d0743e8897140059ac20357f015c36c64..030a54881666c03a5d5bec920742c810366a4fd8 100644 (file)
--- a/src/mds/CInode.cc
+++ b/src/mds/CInode.cc
@@ -2969,22 +2969,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 99dbe961608072658e14f0032d3e45c5474ed772..d943b6e17d7321244f462980361b4a96e7b3d1c0 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 94b200404fdcb9136c813eb003d9c722ae8f7102..f59826216af6fd1c46a8b57a7e41fe8dfbb65ce1 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 b726f430db721edf62fa5b953c13cac49cb81f80..4f0b9940ed5e0d94aff4b336aaaebe7756bc46bd 100644 (file)
--- a/src/mds/Server.cc
+++ b/src/mds/Server.cc
@@ -3146,13 +3146,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;
@@ -3184,7 +3180,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; }