From de2e92bc93c1b5bbaf64baf34c1115db2bf26342 Mon Sep 17 00:00:00 2001 From: Samuel Just Date: Sat, 26 Oct 2024 15:40:56 -0700 Subject: [PATCH] crimson: rewrite with_[clone_]obc[_only] via manager, remove other helpers Signed-off-by: Samuel Just --- src/crimson/osd/object_context_loader.cc | 142 ----------------------- src/crimson/osd/object_context_loader.h | 48 ++++---- 2 files changed, 26 insertions(+), 164 deletions(-) diff --git a/src/crimson/osd/object_context_loader.cc b/src/crimson/osd/object_context_loader.cc index 4a6fc854bff..d0bdd441023 100644 --- a/src/crimson/osd/object_context_loader.cc +++ b/src/crimson/osd/object_context_loader.cc @@ -123,127 +123,6 @@ ObjectContextLoader::load_and_lock(Manager &manager, RWState::State lock_type) } } - template - ObjectContextLoader::load_obc_iertr::future<> - ObjectContextLoader::with_head_obc(const hobject_t& oid, - with_obc_func_t&& func) - { - return with_locked_obc( - oid, - [func=std::move(func)](auto obc) { - // The template with_obc_func_t wrapper supports two obcs (head and clone). - // In the 'with_head_obc' case, however, only the head is in use. - // Pass the same head obc twice in order to - // to support the generic with_obc sturcture. - return std::invoke(std::move(func), obc, obc); - }); - } - - template - ObjectContextLoader::load_obc_iertr::future<> - ObjectContextLoader::with_clone_obc(const hobject_t& oid, - with_obc_func_t&& func, - bool resolve_clone) - { - LOG_PREFIX(ObjectContextLoader::with_clone_obc); - assert(!oid.is_head()); - return with_head_obc( - oid.get_head(), - [FNAME, oid, func=std::move(func), resolve_clone, this] - (auto head, auto) mutable -> load_obc_iertr::future<> { - if (!head->obs.exists) { - ERRORDPP("head doesn't exist for object {}", dpp, head->obs.oi.soid); - return load_obc_iertr::future<>{ - crimson::ct_error::enoent::make() - }; - } - return this->with_clone_obc_only(std::move(head), - oid, - std::move(func), - resolve_clone); - }); - } - - template - ObjectContextLoader::load_obc_iertr::future<> - ObjectContextLoader::with_clone_obc_only(ObjectContextRef head, - hobject_t clone_oid, - with_obc_func_t&& func, - bool resolve_clone) - { - LOG_PREFIX(ObjectContextLoader::with_clone_obc_only); - DEBUGDPP("{}", dpp, clone_oid); - assert(!clone_oid.is_head()); - if (resolve_clone) { - auto resolved_oid = resolve_oid(head->get_head_ss(), clone_oid); - if (!resolved_oid) { - ERRORDPP("clone {} not found", dpp, clone_oid); - return load_obc_iertr::future<>{ - crimson::ct_error::enoent::make() - }; - } - if (resolved_oid->is_head()) { - // See resolve_oid - return std::move(func)(head, head); - } - clone_oid = *resolved_oid; - } - return with_locked_obc( - clone_oid, - [head=std::move(head), func=std::move(func)](auto clone) { - clone->set_clone_ssc(head->ssc); - return std::move(func)(std::move(head), std::move(clone)); - }); - } - - template - ObjectContextLoader::load_obc_iertr::future<> - ObjectContextLoader::with_obc(hobject_t oid, - with_obc_func_t&& func, - bool resolve_clone) - { - if (oid.is_head()) { - return with_head_obc(oid, std::move(func)); - } else { - return with_clone_obc(oid, std::move(func), resolve_clone); - } - } - - template - ObjectContextLoader::load_obc_iertr::future<> - ObjectContextLoader::with_locked_obc(const hobject_t& oid, - Func&& func) - { - LOG_PREFIX(ObjectContextLoader::with_locked_obc); - auto [obc, existed] = obc_registry.get_cached_obc(oid); - DEBUGDPP("object {} existed {}", - dpp, obc->get_oid(), existed); - obc->append_to(obc_set_accessing); - if (existed) { - return obc->with_lock( - [func=std::move(func), obc=ObjectContextRef(obc)] { - return std::invoke(std::move(func), obc); - } - ).finally([FNAME, this, obc=ObjectContextRef(obc)] { - DEBUGDPP("released object {}, {}", dpp, obc->get_oid(), obc->obs); - obc->remove_from(obc_set_accessing); - }); - } else { - return obc->load_then_with_lock ( - [this, obc=ObjectContextRef(obc)] { - return load_obc(obc); - }, - [func=std::move(func), obc=ObjectContextRef(obc)] { - return std::invoke(std::move(func), obc); - } - ).finally([FNAME, this, obc=ObjectContextRef(obc)] { - DEBUGDPP("released object {}, {}", dpp, obc->get_oid(), obc->obs); - obc->remove_from(obc_set_accessing); - }); - } - } - - ObjectContextLoader::load_obc_iertr::future<> ObjectContextLoader::load_obc(ObjectContextRef obc) { @@ -281,25 +160,4 @@ ObjectContextLoader::load_and_lock(Manager &manager, RWState::State lock_type) obc.interrupt(::crimson::common::actingset_changed(is_primary)); } } - - // explicitly instantiate the used instantiations - template ObjectContextLoader::load_obc_iertr::future<> - ObjectContextLoader::with_obc(hobject_t, - with_obc_func_t&&, - bool resolve_clone); - - template ObjectContextLoader::load_obc_iertr::future<> - ObjectContextLoader::with_obc(hobject_t, - with_obc_func_t&&, - bool resolve_clone); - - template ObjectContextLoader::load_obc_iertr::future<> - ObjectContextLoader::with_obc(hobject_t, - with_obc_func_t&&, - bool resolve_clone); - - template ObjectContextLoader::load_obc_iertr::future<> - ObjectContextLoader::with_obc(hobject_t, - with_obc_func_t&&, - bool resolve_clone); } diff --git a/src/crimson/osd/object_context_loader.h b/src/crimson/osd/object_context_loader.h index e5aee8fcb27..6d007d65176 100644 --- a/src/crimson/osd/object_context_loader.h +++ b/src/crimson/osd/object_context_loader.h @@ -168,6 +168,11 @@ public: return target_state.obc; } + ObjectContextRef &get_head_obc() { + ceph_assert(!head_state.is_empty()); + return head_state.obc; + } + void release() { release_state(head_state); release_state(target_state); @@ -213,8 +218,13 @@ public: // See SnapTrimObjSubEvent::remove_or_update - in_removed_snaps_queue usage. template load_obc_iertr::future<> with_obc(hobject_t oid, - with_obc_func_t&& func, - bool resolve_clone = true); + with_obc_func_t func, + bool resolve_clone = true) { + auto manager = get_obc_manager(oid, resolve_clone); + co_await load_and_lock(manager, State); + co_await std::invoke( + func, manager.get_head_obc(), manager.get_obc()); + } // Use this variant in the case where the head object // obc is already locked and only the clone obc is needed. @@ -223,8 +233,20 @@ public: template load_obc_iertr::future<> with_clone_obc_only(ObjectContextRef head, hobject_t clone_oid, - with_obc_func_t&& func, - bool resolve_clone = true); + with_obc_func_t func, + bool resolve_clone = true) { + LOG_PREFIX(ObjectContextLoader::with_clone_obc_only); + SUBDEBUGDPP(osd, "{}", dpp, clone_oid); + auto manager = get_obc_manager(clone_oid, resolve_clone); + // We populate head_state here with the passed obc assuming that + // it has been loaded and locked appropriately. We do not populate + // head_state.state because we won't be taking or releasing any + // locks on head as part of this call. + manager.head_state.obc = head; + manager.head_state.obc->append_to(obc_set_accessing); + co_await load_and_lock(manager, State); + co_await std::invoke(func, head, manager.get_obc()); + } void notify_on_change(bool is_primary); @@ -234,24 +256,6 @@ private: DoutPrefixProvider& dpp; obc_accessing_list_t obc_set_accessing; - template - load_obc_iertr::future<> with_clone_obc(const hobject_t& oid, - with_obc_func_t&& func, - bool resolve_clone); - - template - load_obc_iertr::future<> with_head_obc(const hobject_t& oid, - with_obc_func_t&& func); - - template - load_obc_iertr::future<> with_locked_obc(const hobject_t& oid, - Func&& func); - - template - load_obc_iertr::future - get_or_load_obc(ObjectContextRef obc, - bool existed); - load_obc_iertr::future<> load_obc(ObjectContextRef obc); }; -- 2.39.5