From: Leonid Usov Date: Sun, 12 May 2024 16:19:34 +0000 (+0300) Subject: squid: revert: mds: provide a mechanism to authpin while freezing X-Git-Tag: v19.1.1~299^2~18 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=b9ce3ca25af58c8be4dca7668fe10a4a1d5f4ff3;p=ceph.git 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 (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 1a921309ca01..435e7edb028e 100644 --- a/src/common/TrackedOp.h +++ b/src/common/TrackedOp.h @@ -204,13 +204,15 @@ public: } ~OpTracker(); - template - typename T::Ref create_request(U params) + // NB: P is ref-like, i.e. `params` should be dereferenced for members + template + typename R::Ref create_request(P params) { - constexpr bool has_is_continuous = requires(U u) { - { u->is_continuous() } -> std::same_as; + constexpr bool enable_mark_continuous = requires(typename R::Ref r, P p) { + { p->is_continuous() } -> std::same_as; + 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 9f5f1a49f55e..8694dfc72a2e 100644 --- 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 837ce2b8818d..2566395d1856 100644 --- 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 025e4d17b81f..3828fe04d7b2 100644 --- 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 b70725f714ed..76ac7e21cc09 100644 --- 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 3a778e4d0743..030a54881666 100644 --- 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 99dbe9616080..d943b6e17d73 100644 --- a/src/mds/CInode.h +++ b/src/mds/CInode.h @@ -930,7 +930,7 @@ class CInode : public MDSCacheObject, public InodeStoreBase, public Countercan_auth_pin(&err, skip_quiesce)) { + if (!object->can_auth_pin(&err)) { if (mdr->lock_cache) { CDir *dir; if (CInode *in = dynamic_cast(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 7bc820e4234b..d322a05851a5 100644 --- 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 e94a988703f0..b7769319cb1c 100644 --- 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 d7d74de82dfd..e1b905b33d8c 100644 --- 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 b726f430db72..4f0b9940ed5e 100644 --- a/src/mds/Server.cc +++ b/src/mds/Server.cc @@ -3146,13 +3146,9 @@ void Server::handle_peer_auth_pin(const MDRequestRef& mdr) list 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 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 1a007364d4ab..1799ab361a11 100644 --- 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; }