From fc8315a6df634efbd99bb797507351f709a034dd Mon Sep 17 00:00:00 2001 From: Samuel Just Date: Thu, 3 Oct 2024 19:04:34 +0000 Subject: [PATCH] crimson: convert PG::with_locked_obc users to use obc manager Signed-off-by: Samuel Just --- .../osd/osd_operations/client_request.cc | 73 ++++++++----------- .../osd_operations/internal_client_request.cc | 31 ++++---- .../osd/osd_operations/snaptrim_event.cc | 21 ++++-- 3 files changed, 63 insertions(+), 62 deletions(-) diff --git a/src/crimson/osd/osd_operations/client_request.cc b/src/crimson/osd/osd_operations/client_request.cc index a40db28f05381..d69793c6919e5 100644 --- a/src/crimson/osd/osd_operations/client_request.cc +++ b/src/crimson/osd/osd_operations/client_request.cc @@ -400,51 +400,40 @@ ClientRequest::process_op( DEBUGDPP("{}.{}: past scrub blocker, getting obc", *pg, *this, this_instance_id); - // call with_locked_obc() in order, but wait concurrently for loading. + + auto obc_manager = pg->obc_loader.get_obc_manager(m->get_hobj()); + + // initiate load_and_lock in order, but wait concurrently ihref.enter_stage_sync( client_pp(*pg).lock_obc, *this); - auto process = pg->with_locked_obc( - m->get_hobj(), op_info, - [FNAME, this, pg, this_instance_id, &ihref] ( - auto head, auto obc - ) -> interruptible_future<> { - DEBUGDPP("{}.{}: got obc {}, entering process stage", - *pg, *this, this_instance_id, obc->obs); - return ihref.enter_stage( - client_pp(*pg).process, *this - ).then_interruptible( - [FNAME, this, pg, this_instance_id, obc, &ihref]() mutable { - DEBUGDPP("{}.{}: in process stage, calling do_process", - *pg, *this, this_instance_id); - return do_process( - ihref, pg, obc, this_instance_id - ); - } - ); - }).handle_error_interruptible( - PG::load_obc_ertr::all_same_way( - [FNAME, this, pg=std::move(pg), this_instance_id]( - const auto &code - ) -> interruptible_future<> { - DEBUGDPP("{}.{}: saw error code {}", - *pg, *this, this_instance_id, code); - assert(code.value() > 0); - return reply_op_error(pg, -code.value()); - }) - ); - /* The following works around gcc bug - * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98401. - * The specific symptom I observed is the pg param being - * destructed multiple times resulting in the refcount going - * rapidly to 0 destoying the PG prematurely. - * - * This bug seems to be resolved in gcc 13.2.1. - * - * Assigning the intermediate result and moving it into the co_await - * expression bypasses both bugs. - */ - co_await std::move(process); + int load_err = co_await pg->obc_loader.load_and_lock( + obc_manager, pg->get_lock_type(op_info) + ).si_then([]() -> int { + return 0; + }).handle_error_interruptible( + PG::load_obc_ertr::all_same_way( + [](const auto &code) -> int { + return -code.value(); + }) + ); + if (load_err) { + DEBUGDPP("{}.{}: saw error code loading obc {}", + *pg, *this, this_instance_id, load_err); + co_await reply_op_error(pg, load_err); + co_return; + } + + DEBUGDPP("{}.{}: got obc {}, entering process stage", + *pg, *this, this_instance_id, obc_manager.get_obc()->obs); + co_await ihref.enter_stage( + client_pp(*pg).process, *this); + + DEBUGDPP("{}.{}: in process stage, calling do_process", + *pg, *this, this_instance_id); + co_await do_process( + ihref, pg, obc_manager.get_obc(), this_instance_id + ); } ClientRequest::interruptible_future<> diff --git a/src/crimson/osd/osd_operations/internal_client_request.cc b/src/crimson/osd/osd_operations/internal_client_request.cc index 9e5867caf8067..9692556e790e4 100644 --- a/src/crimson/osd/osd_operations/internal_client_request.cc +++ b/src/crimson/osd/osd_operations/internal_client_request.cc @@ -4,6 +4,7 @@ #include #include "crimson/osd/osd_operations/internal_client_request.h" +#include "osd/object_state_fmt.h" namespace { seastar::logger& logger() { @@ -112,21 +113,25 @@ InternalClientRequest::with_interruption() [[maybe_unused]] const int ret = op_info.set_from_op( std::as_const(osd_ops), pg->get_pgid().pgid, *pg->get_osdmap()); assert(ret == 0); - // call with_locked_obc() in order, but wait concurrently for loading. + + auto obc_manager = pg->obc_loader.get_obc_manager(get_target_oid()); + + // initiate load_and_lock in order, but wait concurrently enter_stage_sync(client_pp().lock_obc); - auto fut = pg->with_locked_obc( - get_target_oid(), op_info, - [&osd_ops, this](auto, auto obc) { - return enter_stage(client_pp().process - ).then_interruptible( - [obc=std::move(obc), &osd_ops, this]() mutable { - return do_process(std::move(obc), osd_ops); - }); - }).handle_error_interruptible( - crimson::ct_error::assert_all("unexpected error") - ); - co_await std::move(fut); + co_await pg->obc_loader.load_and_lock( + obc_manager, pg->get_lock_type(op_info) + ).handle_error_interruptible( + crimson::ct_error::assert_all("unexpected error") + ); + + DEBUGDPP("{}: got obc {}, entering process stage", + *pg, *this, obc_manager.get_obc()->obs); + co_await enter_stage(client_pp().process); + + DEBUGDPP("{}: in process stage, calling do_process", + *pg, *this); + co_await do_process(obc_manager.get_obc(), osd_ops); logger().debug("{}: complete", *this); co_await interruptor::make_interruptible(handle.complete()); diff --git a/src/crimson/osd/osd_operations/snaptrim_event.cc b/src/crimson/osd/osd_operations/snaptrim_event.cc index 9ed0b73cfb458..c5bdcae47f237 100644 --- a/src/crimson/osd/osd_operations/snaptrim_event.cc +++ b/src/crimson/osd/osd_operations/snaptrim_event.cc @@ -399,13 +399,20 @@ SnapTrimObjSubEvent::start() client_pp().check_already_complete_get_obc); logger().debug("{}: getting obc for {}", *this, coid); - // end of commonality - // lock both clone's and head's obcs - co_await pg->obc_loader.with_obc( - coid, - std::bind(&SnapTrimObjSubEvent::process_and_submit, - this, std::placeholders::_1, std::placeholders::_2), - false + + + auto obc_manager = pg->obc_loader.get_obc_manager( + coid, false /* resolve_oid */); + + co_await pg->obc_loader.load_and_lock( + obc_manager, RWState::RWWRITE + ).handle_error_interruptible( + remove_or_update_iertr::pass_further{}, + crimson::ct_error::assert_all{"unexpected error in SnapTrimObjSubEvent"} + ); + + co_await process_and_submit( + obc_manager.get_head_obc(), obc_manager.get_obc() ).handle_error_interruptible( remove_or_update_iertr::pass_further{}, crimson::ct_error::assert_all{"unexpected error in SnapTrimObjSubEvent"} -- 2.39.5