From: Samuel Just Date: Mon, 10 Jun 2024 20:47:07 +0000 (+0000) Subject: crimson: eliminate lock promotion from object_context and tri_mutex X-Git-Tag: v20.0.0~1674^2~7 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=4e14b21f03c5aa1801c1694136b35071186c19e4;p=ceph.git crimson: eliminate lock promotion from object_context and tri_mutex Since we now load obc's via ObjectContext::load_then_with_lock, we no longer need to promote locks. Eliminate support for now. Signed-off-by: Samuel Just --- diff --git a/src/crimson/common/tri_mutex.cc b/src/crimson/common/tri_mutex.cc index 541ebfcf810b..8d8d44c11799 100644 --- a/src/crimson/common/tri_mutex.cc +++ b/src/crimson/common/tri_mutex.cc @@ -41,26 +41,6 @@ void excl_lock::unlock() static_cast(this)->unlock_for_excl(); } -void excl_lock_from_read::lock() -{ - static_cast(this)->promote_from_read(); -} - -void excl_lock_from_read::unlock() -{ - static_cast(this)->demote_to_read(); -} - -void excl_lock_from_write::lock() -{ - static_cast(this)->promote_from_write(); -} - -void excl_lock_from_write::unlock() -{ - static_cast(this)->demote_to_write(); -} - tri_mutex::~tri_mutex() { LOG_PREFIX(tri_mutex::~tri_mutex()); @@ -103,15 +83,6 @@ void tri_mutex::unlock_for_read() } } -void tri_mutex::promote_from_read() -{ - LOG_PREFIX(tri_mutex::promote_from_read()); - DEBUGDPP("", *this); - assert(readers == 1); - --readers; - exclusively_used = true; -} - void tri_mutex::demote_to_read() { LOG_PREFIX(tri_mutex::demote_to_read()); @@ -156,15 +127,6 @@ void tri_mutex::unlock_for_write() } } -void tri_mutex::promote_from_write() -{ - LOG_PREFIX(tri_mutex::promote_from_write()); - DEBUGDPP("", *this); - assert(writers == 1); - --writers; - exclusively_used = true; -} - void tri_mutex::demote_to_write() { LOG_PREFIX(tri_mutex::demote_to_write()); diff --git a/src/crimson/common/tri_mutex.h b/src/crimson/common/tri_mutex.h index d2e83d876437..6e265316c4af 100644 --- a/src/crimson/common/tri_mutex.h +++ b/src/crimson/common/tri_mutex.h @@ -27,20 +27,6 @@ public: void unlock(); }; -// promote from read to excl -class excl_lock_from_read { -public: - void lock(); - void unlock(); -}; - -// promote from write to excl -class excl_lock_from_write { -public: - void lock(); - void unlock(); -}; - /// shared/exclusive mutual exclusion /// /// Unlike reader/write lock, tri_mutex does not enforce the exclusive access @@ -54,14 +40,9 @@ public: /// - readers /// - writers /// - exclusive users -/// -/// For lock promotion, a read or a write lock is only allowed to be promoted -/// atomically upon the first locking. class tri_mutex : private read_lock, write_lock, - excl_lock, - excl_lock_from_read, - excl_lock_from_write + excl_lock { public: tri_mutex() = default; @@ -81,18 +62,11 @@ public: excl_lock& for_excl() { return *this; } - excl_lock_from_read& excl_from_read() { - return *this; - } - excl_lock_from_write& excl_from_write() { - return *this; - } // for shared readers std::optional> lock_for_read(); bool try_lock_for_read() noexcept; void unlock_for_read(); - void promote_from_read(); void demote_to_read(); unsigned get_readers() const { return readers; @@ -102,7 +76,6 @@ public: std::optional> lock_for_write(); bool try_lock_for_write() noexcept; void unlock_for_write(); - void promote_from_write(); void demote_to_write(); unsigned get_writers() const { return writers; @@ -156,9 +129,6 @@ private: friend class read_lock; friend class write_lock; friend class excl_lock; - friend class excl_lock_from_read; - friend class excl_lock_from_write; - friend class excl_lock_from_excl; friend std::ostream& operator<<(std::ostream &lhs, const tri_mutex &rhs); }; diff --git a/src/crimson/osd/object_context.h b/src/crimson/osd/object_context.h index ee2edaefe1dc..a19279ee5ceb 100644 --- a/src/crimson/osd/object_context.h +++ b/src/crimson/osd/object_context.h @@ -168,15 +168,6 @@ private: }); } - template - auto _with_promoted_lock(Lock& lock, Func&& func) { - Ref obc = this; - lock.lock(); - return seastar::futurize_invoke(func).finally([&lock, obc] { - lock.unlock(); - }); - } - boost::intrusive::list_member_hook<> obc_accessing_hook; uint64_t list_link_cnt = 0; bool fully_loaded = false; @@ -237,37 +228,6 @@ public: } } } - template - auto with_promoted_lock(Func&& func) { - if constexpr (!std::is_void_v) { - auto wrapper = ::crimson::interruptible::interruptor::wrap_function(std::forward(func)); - switch (Type) { - case RWState::RWWRITE: - return _with_promoted_lock(lock.excl_from_write(), std::move(wrapper)); - case RWState::RWREAD: - return _with_promoted_lock(lock.excl_from_read(), std::move(wrapper)); - case RWState::RWEXCL: - return seastar::futurize_invoke(std::move(wrapper)); - case RWState::RWNONE: - return _with_lock(lock.for_excl(), std::move(wrapper)); - default: - assert(0 == "noop"); - } - } else { - switch (Type) { - case RWState::RWWRITE: - return _with_promoted_lock(lock.excl_from_write(), std::forward(func)); - case RWState::RWREAD: - return _with_promoted_lock(lock.excl_from_read(), std::forward(func)); - case RWState::RWEXCL: - return seastar::futurize_invoke(std::forward(func)); - case RWState::RWNONE: - return _with_lock(lock.for_excl(), std::forward(func)); - default: - assert(0 == "noop"); - } - } - } /** * load_then_with_lock diff --git a/src/crimson/osd/object_context_loader.cc b/src/crimson/osd/object_context_loader.cc index d099f41e1581..f7a248f13141 100644 --- a/src/crimson/osd/object_context_loader.cc +++ b/src/crimson/osd/object_context_loader.cc @@ -162,50 +162,6 @@ using crimson::common::local_conf; }); } - template - ObjectContextLoader::load_obc_iertr::future - ObjectContextLoader::get_or_load_obc(ObjectContextRef obc, - bool existed) - { - LOG_PREFIX(ObjectContextLoader::get_or_load_obc); - DEBUGDPP("{} -- fully_loaded={}, " - "invalidated_by_interval_change={}", - dpp, obc->get_oid(), - obc->fully_loaded, - obc->invalidated_by_interval_change); - if (existed) { - // obc is already loaded - avoid loading_mutex usage - DEBUGDPP("cache hit on {}", dpp, obc->get_oid()); - return get_obc(obc, existed); - } - // See ObjectContext::_with_lock(), - // this function must be able to support atomicity before - // acquiring the lock - ceph_assert(obc->loading_mutex.try_lock()); - return _get_or_load_obc(obc, existed - ).finally([obc]{ - obc->loading_mutex.unlock(); - }); - } - - template - ObjectContextLoader::load_obc_iertr::future - ObjectContextLoader::_get_or_load_obc(ObjectContextRef obc, - bool existed) - { - LOG_PREFIX(ObjectContextLoader::_get_or_load_obc); - if (existed) { - DEBUGDPP("cache hit on {}", dpp, obc->get_oid()); - return get_obc(obc, existed); - } else { - DEBUGDPP("cache miss on {}", dpp, obc->get_oid()); - return obc->template with_promoted_lock( - [obc, this] { - return load_obc(obc); - }); - } - } - ObjectContextLoader::load_obc_iertr::future<> ObjectContextLoader::reload_obc(ObjectContext& obc) const { diff --git a/src/crimson/osd/object_context_loader.h b/src/crimson/osd/object_context_loader.h index db27e81a3e48..277708eca4f3 100644 --- a/src/crimson/osd/object_context_loader.h +++ b/src/crimson/osd/object_context_loader.h @@ -84,18 +84,6 @@ private: get_or_load_obc(ObjectContextRef obc, bool existed); - template - load_obc_iertr::future - _get_or_load_obc(ObjectContextRef obc, - bool existed); - - static inline load_obc_iertr::future - get_obc(ObjectContextRef obc, - bool existed) { - ceph_assert(existed && obc->is_valid() && obc->is_loaded()); - return load_obc_iertr::make_ready_future(obc); - } - load_obc_iertr::future<> load_obc(ObjectContextRef obc); }; }