From 9c8e0a0634936e16b39f00075c9da4aaf92b042c Mon Sep 17 00:00:00 2001 From: Zhang Song Date: Thu, 24 Apr 2025 17:33:17 +0800 Subject: [PATCH] crimson/os/seastore: fix LBAMapping/BackrefMapping usages Signed-off-by: Zhang Song --- src/crimson/os/seastore/async_cleaner.cc | 14 +- .../os/seastore/object_data_handler.cc | 180 +++++++++-------- src/crimson/os/seastore/object_data_handler.h | 4 +- .../os/seastore/transaction_manager.cc | 12 +- src/crimson/os/seastore/transaction_manager.h | 191 +++++++----------- src/crimson/tools/store_nbd/tm_driver.cc | 6 +- .../seastore/test_btree_lba_manager.cc | 24 +-- .../seastore/test_object_data_handler.cc | 31 +-- .../seastore/test_transaction_manager.cc | 190 +++++++++-------- 9 files changed, 307 insertions(+), 345 deletions(-) diff --git a/src/crimson/os/seastore/async_cleaner.cc b/src/crimson/os/seastore/async_cleaner.cc index d3880a7ce1e9c..1ccdfea40c62e 100644 --- a/src/crimson/os/seastore/async_cleaner.cc +++ b/src/crimson/os/seastore/async_cleaner.cc @@ -1091,7 +1091,7 @@ double SegmentCleaner::calc_gc_benefit_cost( SegmentCleaner::do_reclaim_space_ret SegmentCleaner::do_reclaim_space( const std::vector &backref_extents, - const backref_pin_list_t &pin_list, + const backref_mapping_list_t &pin_list, std::size_t &reclaimed, std::size_t &runs) { @@ -1142,10 +1142,10 @@ SegmentCleaner::do_reclaim_space( backref_entry_query_set_t backref_entries; for (auto &pin : pin_list) { backref_entries.emplace( - pin->get_key(), - pin->get_val(), - pin->get_length(), - pin->get_type()); + pin.get_key(), + pin.get_val(), + pin.get_length(), + pin.get_type()); } for (auto &cached_backref : cached_backref_entries) { if (cached_backref.laddr == L_ADDR_NULL) { @@ -1236,7 +1236,7 @@ SegmentCleaner::clean_space_ret SegmentCleaner::clean_space() // transactions. So, concurrent transactions between trim and reclaim are // not allowed right now. return seastar::do_with( - std::pair, backref_pin_list_t>(), + std::pair, backref_mapping_list_t>(), [this](auto &weak_read_ret) { return repeat_eagain([this, &weak_read_ret] { // Note: not tracked by shard_stats_t intentionally. @@ -1253,7 +1253,7 @@ SegmentCleaner::clean_space_ret SegmentCleaner::clean_space() if (!pin_list.empty()) { auto it = pin_list.begin(); auto &first_pin = *it; - if (first_pin->get_key() < reclaim_state->start_pos) { + if (first_pin.get_key() < reclaim_state->start_pos) { // BackrefManager::get_mappings may include a entry before // reclaim_state->start_pos, which is semantically inconsistent // with the requirements of the cleaner diff --git a/src/crimson/os/seastore/object_data_handler.cc b/src/crimson/os/seastore/object_data_handler.cc index e1ecced2720dd..386ee317051ad 100644 --- a/src/crimson/os/seastore/object_data_handler.cc +++ b/src/crimson/os/seastore/object_data_handler.cc @@ -39,8 +39,8 @@ struct extent_to_write_t { }; type_t type; - /// pin of original extent, not nullptr if type == EXISTING - LBAMappingRef pin; + /// pin of original extent, not std::nullopt if type == EXISTING + std::optional pin; laddr_t addr; extent_len_t len; @@ -80,8 +80,7 @@ struct extent_to_write_t { } static extent_to_write_t create_existing( - LBAMappingRef &&pin, laddr_t addr, extent_len_t len) { - assert(pin); + LBAMapping &&pin, laddr_t addr, extent_len_t len) { return extent_to_write_t(std::move(pin), addr, len); } @@ -93,7 +92,7 @@ private: extent_to_write_t(laddr_t addr, extent_len_t len) : type(type_t::ZERO), addr(addr), len(len) {} - extent_to_write_t(LBAMappingRef &&pin, laddr_t addr, extent_len_t len) + extent_to_write_t(LBAMapping &&pin, laddr_t addr, extent_len_t len) : type(type_t::EXISTING), pin(std::move(pin)), addr(addr), len(len) {} }; using extent_to_write_list_t = std::list; @@ -107,7 +106,7 @@ struct extent_to_remap_t { }; type_t type; /// pin of original extent - LBAMappingRef pin; + LBAMapping pin; /// offset of remapped extent or overwrite part of overwrite extent. /// overwrite part of overwrite extent might correspond to mutiple /// fresh write extent. @@ -123,7 +122,7 @@ struct extent_to_remap_t { } bool is_remap2() const { - assert((new_offset != 0) && (pin->get_length() != new_offset + new_len)); + assert((new_offset != 0) && (pin.get_length() != new_offset + new_len)); return type == type_t::REMAP2; } @@ -150,26 +149,28 @@ struct extent_to_remap_t { assert(is_remap2()); return remap_entry_t( new_offset + new_len, - pin->get_length() - new_offset - new_len); + pin.get_length() - new_offset - new_len); } static extent_to_remap_t create_remap1( - LBAMappingRef &&pin, extent_len_t new_offset, extent_len_t new_len) { + LBAMapping &&pin, extent_len_t new_offset, extent_len_t new_len) { return extent_to_remap_t(type_t::REMAP1, std::move(pin), new_offset, new_len); } static extent_to_remap_t create_remap2( - LBAMappingRef &&pin, extent_len_t new_offset, extent_len_t new_len) { + LBAMapping &&pin, extent_len_t new_offset, extent_len_t new_len) { return extent_to_remap_t(type_t::REMAP2, std::move(pin), new_offset, new_len); } static extent_to_remap_t create_overwrite( - extent_len_t new_offset, extent_len_t new_len, LBAMappingRef p, + extent_len_t new_offset, extent_len_t new_len, LBAMapping p, bufferlist b) { - return extent_to_remap_t(type_t::OVERWRITE, - nullptr, new_offset, new_len, p->get_key(), p->get_length(), b); + auto key = p.get_key(); + auto len = p.get_length(); + return extent_to_remap_t(type_t::OVERWRITE, std::move(p), + new_offset, new_len, key, len, b); } laddr_t laddr_start; @@ -178,14 +179,14 @@ struct extent_to_remap_t { private: extent_to_remap_t(type_t type, - LBAMappingRef &&pin, extent_len_t new_offset, extent_len_t new_len) + LBAMapping &&pin, extent_len_t new_offset, extent_len_t new_len) : type(type), pin(std::move(pin)), new_offset(new_offset), new_len(new_len) {} - extent_to_remap_t(type_t type, - LBAMappingRef &&pin, extent_len_t new_offset, extent_len_t new_len, + extent_to_remap_t(type_t type, LBAMapping &&pin, + extent_len_t new_offset, extent_len_t new_len, laddr_t ori_laddr, extent_len_t ori_len, std::optional b) - : type(type), - pin(std::move(pin)), new_offset(new_offset), new_len(new_len), + : type(type), pin(std::move(pin)), + new_offset(new_offset), new_len(new_len), laddr_start(ori_laddr), length(ori_len), bl(b) {} }; using extent_to_remap_list_t = std::list; @@ -236,7 +237,7 @@ private: using extent_to_insert_list_t = std::list; // Encapsulates extents to be retired in do_removals. -using extent_to_remove_list_t = std::list; +using extent_to_remove_list_t = std::list; struct overwrite_ops_t { extent_to_remap_list_t to_remap; @@ -246,7 +247,7 @@ struct overwrite_ops_t { // prepare to_remap, to_retire, to_insert list overwrite_ops_t prepare_ops_list( - lba_pin_list_t &pins_to_remove, + lba_mapping_list_t &pins_to_remove, extent_to_write_list_t &to_write, size_t delta_based_overwrite_max_extent_size) { assert(pins_to_remove.size() != 0); @@ -265,10 +266,11 @@ overwrite_ops_t prepare_ops_list( front.is_existing() && back.is_existing()) { visitted += 2; assert(to_write.size() > 2); + assert(front.pin); assert(front.addr == front.pin->get_key()); assert(back.addr > back.pin->get_key()); ops.to_remap.push_back(extent_to_remap_t::create_remap2( - std::move(front.pin), + std::move(*front.pin), front.len, back.addr.get_byte_distance(front.addr) - front.len)); ops.to_remove.pop_front(); @@ -277,9 +279,10 @@ overwrite_ops_t prepare_ops_list( if (front.is_existing()) { visitted++; assert(to_write.size() > 1); + assert(front.pin); assert(front.addr == front.pin->get_key()); ops.to_remap.push_back(extent_to_remap_t::create_remap1( - std::move(front.pin), + std::move(*front.pin), 0, front.len)); ops.to_remove.pop_front(); @@ -287,11 +290,13 @@ overwrite_ops_t prepare_ops_list( if (back.is_existing()) { visitted++; assert(to_write.size() > 1); + assert(back.pin); assert(back.addr + back.len == back.pin->get_key() + back.pin->get_length()); + auto key = back.pin->get_key(); ops.to_remap.push_back(extent_to_remap_t::create_remap1( - std::move(back.pin), - back.addr.get_byte_distance(back.pin->get_key()), + std::move(*back.pin), + back.addr.get_byte_distance(key), back.len)); ops.to_remove.pop_back(); } @@ -300,14 +305,14 @@ overwrite_ops_t prepare_ops_list( laddr_interval_set_t pre_alloc_addr_removed, pre_alloc_addr_remapped; if (delta_based_overwrite_max_extent_size) { for (auto &r : ops.to_remove) { - if (r->is_data_stable() && !r->is_zero_reserved()) { - pre_alloc_addr_removed.insert(r->get_key(), r->get_length()); + if (r.is_data_stable() && !r.is_zero_reserved()) { + pre_alloc_addr_removed.insert(r.get_key(), r.get_length()); } } for (auto &r : ops.to_remap) { - if (r.pin && r.pin->is_data_stable() && !r.pin->is_zero_reserved()) { - pre_alloc_addr_remapped.insert(r.pin->get_key(), r.pin->get_length()); + if (r.pin.is_data_stable() && !r.pin.is_zero_reserved()) { + pre_alloc_addr_remapped.insert(r.pin.get_key(), r.pin.get_length()); } } } @@ -325,8 +330,8 @@ overwrite_ops_t prepare_ops_list( ops.to_remove, [®ion, &to_remap](auto &r) { laddr_interval_set_t range; - range.insert(r->get_key(), r->get_length()); - if (range.contains(region.addr, region.len) && !r->is_clone()) { + range.insert(r.get_key(), r.get_length()); + if (range.contains(region.addr, region.len) && !r.is_clone()) { to_remap.push_back(extent_to_remap_t::create_overwrite( 0, region.len, std::move(r), *region.to_write)); return true; @@ -341,8 +346,8 @@ overwrite_ops_t prepare_ops_list( ops.to_remap, [®ion, &to_remap](auto &r) { laddr_interval_set_t range; - range.insert(r.pin->get_key(), r.pin->get_length()); - if (range.contains(region.addr, region.len) && !r.pin->is_clone()) { + range.insert(r.pin.get_key(), r.pin.get_length()); + if (range.contains(region.addr, region.len) && !r.pin.is_clone()) { to_remap.push_back(extent_to_remap_t::create_overwrite( region.addr.get_byte_distance< extent_len_t> (range.begin().get_start()), @@ -448,7 +453,7 @@ ObjectDataHandler::write_ret do_remappings( } ).si_then([®ion](auto pins) { ceph_assert(pins.size() == 1); - ceph_assert(region.new_len == pins[0]->get_length()); + ceph_assert(region.new_len == pins[0].get_length()); return ObjectDataHandler::write_iertr::now(); }); } else if (region.is_overwrite()) { @@ -468,7 +473,7 @@ ObjectDataHandler::write_ret do_remappings( return ObjectDataHandler::write_iertr::now(); }); } else if (region.is_remap2()) { - auto pin_key = region.pin->get_key(); + auto pin_key = region.pin.get_key(); return ctx.tm.remap_pin( ctx.t, std::move(region.pin), @@ -478,9 +483,9 @@ ObjectDataHandler::write_ret do_remappings( } ).si_then([®ion, pin_key](auto pins) { ceph_assert(pins.size() == 2); - ceph_assert(pin_key == pins[0]->get_key()); - ceph_assert(pin_key + pins[0]->get_length() + - region.new_len == pins[1]->get_key()); + ceph_assert(pin_key == pins[0].get_key()); + ceph_assert(pin_key + pins[0].get_length() + + region.new_len == pins[1].get_key()); return ObjectDataHandler::write_iertr::now(); }); } else { @@ -492,7 +497,7 @@ ObjectDataHandler::write_ret do_remappings( ObjectDataHandler::write_ret do_removals( context_t ctx, - lba_pin_list_t &to_remove) + lba_mapping_list_t &to_remove) { return trans_intr::do_for_each( to_remove, @@ -500,10 +505,10 @@ ObjectDataHandler::write_ret do_removals( LOG_PREFIX(object_data_handler.cc::do_removals); DEBUGT("decreasing ref: {}", ctx.t, - pin->get_key()); + pin.get_key()); return ctx.tm.remove( ctx.t, - pin->get_key() + pin.get_key() ).discard_result().handle_error_interruptible( ObjectDataHandler::write_iertr::pass_further{}, crimson::ct_error::assert_all{ @@ -565,15 +570,15 @@ ObjectDataHandler::write_ret do_insertions( region.addr, region.len ).si_then([FNAME, ctx, ®ion](auto pin) { - ceph_assert(pin->get_length() == region.len); - if (pin->get_key() != region.addr) { + ceph_assert(pin.get_length() == region.len); + if (pin.get_key() != region.addr) { ERRORT( "inconsistent laddr: pin: {} region {}", ctx.t, - pin->get_key(), + pin.get_key(), region.addr); } - ceph_assert(pin->get_key() == region.addr); + ceph_assert(pin.get_key() == region.addr); return ObjectDataHandler::write_iertr::now(); }).handle_error_interruptible( crimson::ct_error::enospc::assert_failure{"unexpected enospc"}, @@ -707,13 +712,13 @@ public: overwrite_plan_t(laddr_t data_base, objaddr_t offset, extent_len_t len, - const lba_pin_list_t& pins, + const lba_mapping_list_t& pins, extent_len_t block_size) : data_base(data_base), - pin_begin(pins.front()->get_key()), - pin_end((pins.back()->get_key() + pins.back()->get_length()).checked_to_laddr()), - left_paddr(pins.front()->get_val()), - right_paddr(pins.back()->get_val()), + pin_begin(pins.front().get_key()), + pin_end((pins.back().get_key() + pins.back().get_length()).checked_to_laddr()), + left_paddr(pins.front().get_val()), + right_paddr(pins.back().get_val()), data_begin(data_base + offset), data_end(data_base + offset + len), aligned_data_begin(data_begin.get_aligned_laddr()), @@ -723,8 +728,8 @@ public: block_size(block_size), // TODO: introduce LBAMapping::is_fresh() // Note: fresh write can be merged with overwrite if they overlap. - is_left_fresh(!pins.front()->is_stable()), - is_right_fresh(!pins.back()->is_stable()) { + is_left_fresh(!pins.front().is_stable()), + is_right_fresh(!pins.back().is_stable()) { validate(); evaluate_operations(); assert(left_operation != overwrite_operation_t::UNKNOWN); @@ -831,7 +836,7 @@ using operate_ret_bare = std::pair< std::optional, std::optional>; using operate_ret = get_iertr::future; -operate_ret operate_left(context_t ctx, LBAMappingRef &pin, const overwrite_plan_t &overwrite_plan) +operate_ret operate_left(context_t ctx, LBAMapping &pin, const overwrite_plan_t &overwrite_plan) { if (overwrite_plan.get_left_size() == 0) { return get_iertr::make_ready_future( @@ -840,7 +845,7 @@ operate_ret operate_left(context_t ctx, LBAMappingRef &pin, const overwrite_plan } if (overwrite_plan.left_operation == overwrite_operation_t::OVERWRITE_ZERO) { - assert(pin->get_val().is_zero()); + assert(pin.get_val().is_zero()); auto zero_extent_len = overwrite_plan.get_left_extent_size(); assert_aligned(zero_extent_len); @@ -869,7 +874,7 @@ operate_ret operate_left(context_t ctx, LBAMappingRef &pin, const overwrite_plan std::nullopt); } else { return ctx.tm.read_pin( - ctx.t, pin->duplicate() + ctx.t, pin.duplicate() ).si_then([prepend_len](auto maybe_indirect_left_extent) { auto read_bl = maybe_indirect_left_extent.get_bl(); ceph::bufferlist prepend_bl; @@ -886,8 +891,8 @@ operate_ret operate_left(context_t ctx, LBAMappingRef &pin, const overwrite_plan assert(extent_len); std::optional left_to_write_extent = std::make_optional(extent_to_write_t::create_existing( - pin->duplicate(), - pin->get_key(), + pin.duplicate(), + pin.get_key(), extent_len)); auto prepend_len = overwrite_plan.get_left_alignment_size(); @@ -897,7 +902,7 @@ operate_ret operate_left(context_t ctx, LBAMappingRef &pin, const overwrite_plan std::nullopt); } else { return ctx.tm.read_pin( - ctx.t, pin->duplicate() + ctx.t, pin.duplicate() ).si_then([prepend_offset=extent_len, prepend_len, left_to_write_extent=std::move(left_to_write_extent)] (auto left_maybe_indirect_extent) mutable { @@ -917,7 +922,7 @@ operate_ret operate_left(context_t ctx, LBAMappingRef &pin, const overwrite_plan * * Proceed overwrite_plan.right_operation. */ -operate_ret operate_right(context_t ctx, LBAMappingRef &pin, const overwrite_plan_t &overwrite_plan) +operate_ret operate_right(context_t ctx, LBAMapping &pin, const overwrite_plan_t &overwrite_plan) { if (overwrite_plan.get_right_size() == 0) { return get_iertr::make_ready_future( @@ -925,10 +930,10 @@ operate_ret operate_right(context_t ctx, LBAMappingRef &pin, const overwrite_pla std::nullopt); } - auto right_pin_begin = pin->get_key(); + auto right_pin_begin = pin.get_key(); assert(overwrite_plan.data_end >= right_pin_begin); if (overwrite_plan.right_operation == overwrite_operation_t::OVERWRITE_ZERO) { - assert(pin->get_val().is_zero()); + assert(pin.get_val().is_zero()); auto zero_suffix_len = overwrite_plan.get_right_alignment_size(); std::optional suffix_bl; @@ -960,7 +965,7 @@ operate_ret operate_right(context_t ctx, LBAMappingRef &pin, const overwrite_pla overwrite_plan.data_end.get_byte_distance< extent_len_t>(right_pin_begin); return ctx.tm.read_pin( - ctx.t, pin->duplicate() + ctx.t, pin.duplicate() ).si_then([append_offset, append_len] (auto right_maybe_indirect_extent) { auto read_bl = right_maybe_indirect_extent.get_bl(); @@ -978,7 +983,7 @@ operate_ret operate_right(context_t ctx, LBAMappingRef &pin, const overwrite_pla assert(extent_len); std::optional right_to_write_extent = std::make_optional(extent_to_write_t::create_existing( - pin->duplicate(), + pin.duplicate(), overwrite_plan.aligned_data_end, extent_len)); @@ -992,7 +997,7 @@ operate_ret operate_right(context_t ctx, LBAMappingRef &pin, const overwrite_pla overwrite_plan.data_end.get_byte_distance< extent_len_t>(right_pin_begin); return ctx.tm.read_pin( - ctx.t, pin->duplicate() + ctx.t, pin.duplicate() ).si_then([append_offset, append_len, right_to_write_extent=std::move(right_to_write_extent)] (auto maybe_indirect_right_extent) mutable { @@ -1074,10 +1079,10 @@ ObjectDataHandler::write_ret ObjectDataHandler::prepare_data_reservation( ctx.onode.get_data_hint(), max_object_size ).si_then([max_object_size=max_object_size, &object_data](auto pin) { - ceph_assert(pin->get_length() == max_object_size); + ceph_assert(pin.get_length() == max_object_size); object_data.update_reserved( - pin->get_key(), - pin->get_length()); + pin.get_key(), + pin.get_length()); return write_iertr::now(); }).handle_error_interruptible( crimson::ct_error::enospc::assert_failure{"unexpected enospc"}, @@ -1092,7 +1097,7 @@ ObjectDataHandler::clear_ret ObjectDataHandler::trim_data_reservation( ceph_assert(!object_data.is_null()); ceph_assert(size <= object_data.get_reserved_data_len()); return seastar::do_with( - lba_pin_list_t(), + lba_mapping_list_t(), extent_to_write_list_t(), [ctx, size, &object_data, this](auto &pins, auto &to_write) { LOG_PREFIX(ObjectDataHandler::trim_data_reservation); @@ -1112,7 +1117,7 @@ ObjectDataHandler::clear_ret ObjectDataHandler::trim_data_reservation( // size to 0 return clear_iertr::now(); } - auto &pin = *pins.front(); + auto &pin = pins.front(); ceph_assert(pin.get_key() >= object_data.get_reserved_data_base()); ceph_assert( pin.get_key() <= object_data.get_reserved_data_base() + size); @@ -1290,7 +1295,7 @@ ObjectDataHandler::write_ret ObjectDataHandler::overwrite( objaddr_t offset, extent_len_t len, std::optional &&bl, - lba_pin_list_t &&_pins) + lba_mapping_list_t &&_pins) { if (bl.has_value()) { assert(bl->length() == len); @@ -1502,7 +1507,7 @@ ObjectDataHandler::read_ret ObjectDataHandler::read( ).si_then([FNAME, ctx, l_start, l_end, &ret](auto _pins) { // offset~len falls within reserved region and len > 0 ceph_assert(_pins.size() >= 1); - ceph_assert((*_pins.begin())->get_key() <= l_start); + ceph_assert(_pins.front().get_key() <= l_start); return seastar::do_with( std::move(_pins), l_start, @@ -1511,7 +1516,7 @@ ObjectDataHandler::read_ret ObjectDataHandler::read( pins, [FNAME, ctx, l_start, l_end, &l_current, &ret](auto &pin) -> read_iertr::future<> { - auto pin_start = pin->get_key(); + auto pin_start = pin.get_key(); extent_len_t read_start; extent_len_t read_start_aligned; if (l_current == l_start) { // first pin may skip head @@ -1527,7 +1532,7 @@ ObjectDataHandler::read_ret ObjectDataHandler::read( } ceph_assert(l_current < l_end); - auto pin_len = pin->get_length(); + auto pin_len = pin.get_length(); assert(pin_len > 0); laddr_offset_t pin_end = pin_start + pin_len; assert(l_current < pin_end); @@ -1535,7 +1540,7 @@ ObjectDataHandler::read_ret ObjectDataHandler::read( extent_len_t read_len = l_current_end.get_byte_distance(l_current); - if (pin->get_val().is_zero()) { + if (pin.get_val().is_zero()) { DEBUGT("got {}~0x{:x} from zero-pin {}~0x{:x}", ctx.t, l_current, @@ -1633,12 +1638,12 @@ ObjectDataHandler::fiemap_ret ObjectDataHandler::fiemap( aligned_length ).si_then([l_start, len, &object_data, &ret](auto &&pins) { ceph_assert(pins.size() >= 1); - ceph_assert((*pins.begin())->get_key() <= l_start); + ceph_assert(pins.front().get_key() <= l_start); for (auto &&i: pins) { - if (!(i->get_val().is_zero())) { - laddr_offset_t ret_left = std::max(laddr_offset_t(i->get_key(), 0), l_start); + if (!(i.get_val().is_zero())) { + laddr_offset_t ret_left = std::max(laddr_offset_t(i.get_key(), 0), l_start); laddr_offset_t ret_right = std::min( - i->get_key() + i->get_length(), + i.get_key() + i.get_length(), l_start + len); assert(ret_right > ret_left); ret.emplace( @@ -1703,7 +1708,7 @@ ObjectDataHandler::clear_ret ObjectDataHandler::clear( ObjectDataHandler::clone_ret ObjectDataHandler::clone_extents( context_t ctx, object_data_t &object_data, - lba_pin_list_t &pins, + lba_mapping_list_t &pins, laddr_t data_base) { LOG_PREFIX(ObjectDataHandler::clone_extents); @@ -1723,21 +1728,20 @@ ObjectDataHandler::clone_ret ObjectDataHandler::clone_extents( return trans_intr::do_for_each( pins, [&last_pos, &object_data, ctx, data_base](auto &pin) { - auto offset = pin->get_key().template get_byte_distance< + auto offset = pin.get_key().template get_byte_distance< extent_len_t>(data_base); ceph_assert(offset == last_pos); - auto fut = TransactionManager::alloc_extent_iertr - ::make_ready_future(); laddr_t addr = (object_data.get_reserved_data_base() + offset) .checked_to_laddr(); - if (pin->get_val().is_zero()) { - fut = ctx.tm.reserve_region(ctx.t, addr, pin->get_length()); - } else { - fut = ctx.tm.clone_pin(ctx.t, addr, *pin); - } - return fut.si_then( + return seastar::futurize_invoke([ctx, addr, &pin] { + if (pin.get_val().is_zero()) { + return ctx.tm.reserve_region(ctx.t, addr, pin.get_length()); + } else { + return ctx.tm.clone_pin(ctx.t, addr, pin); + } + }).si_then( [&pin, &last_pos, offset](auto) { - last_pos = offset + pin->get_length(); + last_pos = offset + pin.get_length(); return seastar::now(); }).handle_error_interruptible( crimson::ct_error::input_output_error::pass_further(), diff --git a/src/crimson/os/seastore/object_data_handler.h b/src/crimson/os/seastore/object_data_handler.h index 3fc02a3383621..123a7889b9157 100644 --- a/src/crimson/os/seastore/object_data_handler.h +++ b/src/crimson/os/seastore/object_data_handler.h @@ -234,7 +234,7 @@ private: objaddr_t offset, ///< [in] write offset extent_len_t len, ///< [in] len to write, len == bl->length() if bl std::optional &&bl, ///< [in] buffer to write, empty for zeros - lba_pin_list_t &&pins ///< [in] set of pins overlapping above region + lba_mapping_list_t &&pins ///< [in] set of pins overlapping above region ); /// Ensures object_data reserved region is prepared @@ -252,7 +252,7 @@ private: clone_ret clone_extents( context_t ctx, object_data_t &object_data, - lba_pin_list_t &pins, + lba_mapping_list_t &pins, laddr_t data_base); private: diff --git a/src/crimson/os/seastore/transaction_manager.cc b/src/crimson/os/seastore/transaction_manager.cc index e7bf690c5e3d0..3a2ce9efe6e7e 100644 --- a/src/crimson/os/seastore/transaction_manager.cc +++ b/src/crimson/os/seastore/transaction_manager.cc @@ -586,8 +586,8 @@ TransactionManager::rewrite_logical_extent( *nextent, refcount ).si_then([extent, nextent, off](auto mapping) { - ceph_assert(mapping->get_key() == extent->get_laddr() + off); - ceph_assert(mapping->get_val() == nextent->get_paddr()); + ceph_assert(mapping.get_key() == extent->get_laddr() + off); + ceph_assert(mapping.get_val() == nextent->get_paddr()); return seastar::now(); }); } @@ -712,7 +712,7 @@ TransactionManager::get_extents_if_live( t, laddr, len - ).si_then([this, FNAME, type, paddr, laddr, len, &t](lba_pin_list_t pin_list) { + ).si_then([this, FNAME, type, paddr, laddr, len, &t](lba_mapping_list_t pin_list) { return seastar::do_with( std::list(), std::move(pin_list), @@ -723,10 +723,10 @@ TransactionManager::get_extents_if_live( return trans_intr::parallel_for_each( pin_list, [this, FNAME, type, paddr_seg_id, &extent_list, &t]( - LBAMappingRef& pin) -> Cache::get_extent_iertr::future<> + LBAMapping& pin) -> Cache::get_extent_iertr::future<> { - DEBUGT("got pin, try read in parallel ... -- {}", t, *pin); - auto pin_paddr = pin->get_val(); + DEBUGT("got pin, try read in parallel ... -- {}", t, pin); + auto pin_paddr = pin.get_val(); if (!pin_paddr.is_absolute_segmented()) { return seastar::now(); } diff --git a/src/crimson/os/seastore/transaction_manager.h b/src/crimson/os/seastore/transaction_manager.h index f2169e0ffdcc7..348f27b8366dd 100644 --- a/src/crimson/os/seastore/transaction_manager.h +++ b/src/crimson/os/seastore/transaction_manager.h @@ -102,15 +102,15 @@ public: * Get the logical pin at offset */ using get_pin_iertr = LBAManager::get_mapping_iertr; - using get_pin_ret = LBAManager::get_mapping_iertr::future; + using get_pin_ret = LBAManager::get_mapping_iertr::future; get_pin_ret get_pin( Transaction &t, laddr_t offset) { LOG_PREFIX(TransactionManager::get_pin); SUBDEBUGT(seastore_tm, "{} ...", t, offset); return lba_manager->get_mapping(t, offset - ).si_then([FNAME, &t](LBAMappingRef pin) { - SUBDEBUGT(seastore_tm, "got {}", t, *pin); + ).si_then([FNAME, &t](LBAMapping pin) { + SUBDEBUGT(seastore_tm, "got {}", t, pin); return pin; }); } @@ -121,7 +121,7 @@ public: * Get logical pins overlapping offset~length */ using get_pins_iertr = LBAManager::get_mappings_iertr; - using get_pins_ret = get_pins_iertr::future; + using get_pins_ret = get_pins_iertr::future; get_pins_ret get_pins( Transaction &t, laddr_t offset, @@ -130,7 +130,7 @@ public: SUBDEBUGT(seastore_tm, "{}~0x{:x} ...", t, offset, length); return lba_manager->get_mappings( t, offset, length - ).si_then([FNAME, &t](lba_pin_list_t pins) { + ).si_then([FNAME, &t](lba_mapping_list_t pins) { SUBDEBUGT(seastore_tm, "got {} pins", t, pins.size()); return pins; }); @@ -213,9 +213,9 @@ public: ).si_then([this, FNAME, &t, offset, length, maybe_init=std::move(maybe_init)] (auto pin) mutable -> read_extent_ret { - if (length != pin->get_length() || !pin->get_val().is_real_location()) { + if (length != pin.get_length() || !pin.get_val().is_real_location()) { SUBERRORT(seastore_tm, "{}~0x{:x} {} got wrong pin {}", - t, offset, length, T::TYPE, *pin); + t, offset, length, T::TYPE, pin); ceph_abort("Impossible"); } return this->read_pin(t, std::move(pin), std::move(maybe_init)); @@ -240,9 +240,9 @@ public: ).si_then([this, FNAME, &t, offset, maybe_init=std::move(maybe_init)] (auto pin) mutable -> read_extent_ret { - if (!pin->get_val().is_real_location()) { + if (!pin.get_val().is_real_location()) { SUBERRORT(seastore_tm, "{} {} got wrong pin {}", - t, offset, T::TYPE, *pin); + t, offset, T::TYPE, pin); ceph_abort("Impossible"); } return this->read_pin(t, std::move(pin), std::move(maybe_init)); @@ -252,7 +252,7 @@ public: template base_iertr::future> read_pin( Transaction &t, - LBAMappingRef pin, + LBAMapping pin, extent_len_t partial_off, extent_len_t partial_len, lextent_init_func_t maybe_init = [](T&) {}) @@ -264,35 +264,21 @@ public: assert(is_user_transaction(t.get_src())); extent_len_t direct_partial_off = partial_off; - bool is_clone = pin->is_clone(); + bool is_clone = pin.is_clone(); std::optional maybe_indirect_info; - if (pin->is_indirect()) { - auto intermediate_offset = pin->get_intermediate_offset(); + if (pin.is_indirect()) { + auto intermediate_offset = pin.get_intermediate_offset(); direct_partial_off = intermediate_offset + partial_off; maybe_indirect_info = indirect_info_t{ - intermediate_offset, pin->get_length()}; + intermediate_offset, pin.get_length()}; } LOG_PREFIX(TransactionManager::read_pin); SUBDEBUGT(seastore_tm, "{} {} 0x{:x}~0x{:x} direct_off=0x{:x} ...", - t, T::TYPE, *pin, partial_off, partial_len, direct_partial_off); + t, T::TYPE, pin, partial_off, partial_len, direct_partial_off); - auto fut = base_iertr::make_ready_future(); - if (!pin->is_parent_viewable()) { - if (pin->is_parent_valid()) { - pin = pin->refresh_with_pending_parent(); - fut = base_iertr::make_ready_future(std::move(pin)); - } else { - fut = get_pin(t, pin->get_key() - ).handle_error_interruptible( - crimson::ct_error::enoent::assert_failure{"unexpected enoent"}, - crimson::ct_error::input_output_error::pass_further{} - ); - } - } else { - pin->maybe_fix_pos(); - fut = base_iertr::make_ready_future(std::move(pin)); - } + auto fut = base_iertr::make_ready_future(); + // TODO: refresh pin return fut.si_then([&t, this, direct_partial_off, partial_len, maybe_init=std::move(maybe_init)](auto npin) mutable { // checking the lba child must be atomic with creating @@ -333,13 +319,12 @@ public: template base_iertr::future> read_pin( Transaction &t, - LBAMappingRef pin, + LBAMapping pin, lextent_init_func_t maybe_init = [](T&) {}) { - auto& pin_ref = *pin; + auto len = pin.get_length(); return read_pin( - t, std::move(pin), 0, - pin_ref.get_length(), + t, std::move(pin), 0, len, std::move(maybe_init)); } @@ -466,9 +451,9 @@ public: SUBDEBUGT(seastore_tm, "{}~0x{:x} ...", t, laddr, len); return get_pin(t, laddr ).si_then([this, &t, len](auto pin) { - ceph_assert(pin->is_data_stable() && !pin->is_zero_reserved()); - ceph_assert(!pin->is_clone()); - ceph_assert(pin->get_length() == len); + ceph_assert(pin.is_data_stable() && !pin.is_zero_reserved()); + ceph_assert(!pin.is_clone()); + ceph_assert(pin.get_length() == len); return this->read_pin(t, std::move(pin)); }).si_then([this, &t, FNAME](auto maybe_indirect_extent) { assert(!maybe_indirect_extent.is_indirect()); @@ -489,11 +474,11 @@ public: */ using remap_entry_t = LBAManager::remap_entry_t; using remap_pin_iertr = base_iertr; - using remap_pin_ret = remap_pin_iertr::future>; + using remap_pin_ret = remap_pin_iertr::future>; template remap_pin_ret remap_pin( Transaction &t, - LBAMappingRef &&pin, + LBAMapping &&pin, std::array remaps) { static_assert(std::is_base_of_v); // data extents don't need maybe_init yet, currently, @@ -506,7 +491,7 @@ public: [](remap_entry_t x, remap_entry_t y) { return x.offset < y.offset; }); - auto original_len = pin->get_length(); + auto original_len = pin.get_length(); extent_len_t total_remap_len = 0; extent_len_t last_offset = 0; extent_len_t last_len = 0; @@ -532,44 +517,29 @@ public: std::move(pin), std::move(remaps), [&t, this](auto &extents, auto &pin, auto &remaps) { - laddr_t original_laddr = pin->get_key(); - extent_len_t original_len = pin->get_length(); - paddr_t original_paddr = pin->get_val(); + laddr_t original_laddr = pin.get_key(); + extent_len_t original_len = pin.get_length(); + paddr_t original_paddr = pin.get_val(); LOG_PREFIX(TransactionManager::remap_pin); SUBDEBUGT(seastore_tm, "{}~0x{:x} {} into {} remaps ... {}", - t, original_laddr, original_len, original_paddr, remaps.size(), *pin); + t, original_laddr, original_len, original_paddr, remaps.size(), pin); // The according extent might be stable or pending. auto fut = base_iertr::now(); if (!pin->is_indirect()) { ceph_assert(!pin->is_clone()); - if (!pin->is_parent_viewable()) { - if (pin->is_parent_valid()) { - pin = pin->refresh_with_pending_parent(); - } else { - fut = get_pin(t, pin->get_key() - ).si_then([&pin](auto npin) { - assert(npin); - pin = std::move(npin); - return seastar::now(); - }).handle_error_interruptible( - crimson::ct_error::enoent::assert_failure{"unexpected enoent"}, - crimson::ct_error::input_output_error::pass_further{} - ); - } - } else { - pin->maybe_fix_pos(); - } + + // TODO: refresh pin fut = fut.si_then([this, &t, &pin] { if (full_extent_integrity_check) { - return read_pin(t, pin->duplicate() + return read_pin(t, pin.duplicate() ).si_then([](auto maybe_indirect_extent) { assert(!maybe_indirect_extent.is_indirect()); assert(!maybe_indirect_extent.is_clone); return maybe_indirect_extent.extent; }); } else { - auto ret = get_extent_if_linked(t, pin->duplicate()); + auto ret = get_extent_if_linked(t, pin.duplicate()); if (ret.index() == 1) { return std::get<1>(ret ).si_then([](auto extent) { @@ -650,7 +620,7 @@ public: } using reserve_extent_iertr = alloc_extent_iertr; - using reserve_extent_ret = reserve_extent_iertr::future; + using reserve_extent_ret = reserve_extent_iertr::future; reserve_extent_ret reserve_region( Transaction &t, laddr_t hint, @@ -662,7 +632,7 @@ public: hint, len ).si_then([FNAME, &t](auto pin) { - SUBDEBUGT(seastore_tm, "reserved {}", t, *pin); + SUBDEBUGT(seastore_tm, "reserved {}", t, pin); return pin; }); } @@ -676,7 +646,7 @@ public: * Note that the cloned extent must be stable */ using clone_extent_iertr = alloc_extent_iertr; - using clone_extent_ret = clone_extent_iertr::future; + using clone_extent_ret = clone_extent_iertr::future; clone_extent_ret clone_pin( Transaction &t, laddr_t hint, @@ -699,7 +669,7 @@ public: intermediate_key, intermediate_base ).si_then([FNAME, &t](auto pin) { - SUBDEBUGT(seastore_tm, "cloned as {}", t, *pin); + SUBDEBUGT(seastore_tm, "cloned as {}", t, pin); return pin; }); } @@ -984,27 +954,27 @@ private: using LBALeafNode = lba_manager::btree::LBALeafNode; struct unlinked_child_t { - LBAMappingRef mapping; + LBAMapping mapping; child_pos_t child_pos; }; template std::variant> get_extent_if_linked( Transaction &t, - LBAMappingRef pin) + LBAMapping pin) { - ceph_assert(pin->is_parent_viewable()); + ceph_assert(pin.is_viewable()); // checking the lba child must be atomic with creating // and linking the absent child - auto v = pin->get_logical_extent(t); + auto v = pin.get_logical_extent(t); if (v.has_child()) { return v.get_child_fut( ).si_then([pin=std::move(pin)](auto extent) { #ifndef NDEBUG auto lextent = extent->template cast(); - auto pin_laddr = pin->get_key(); - if (pin->is_indirect()) { - pin_laddr = pin->get_intermediate_base(); + auto pin_laddr = pin.get_key(); + if (pin.is_indirect()) { + pin_laddr = pin.get_intermediate_base(); } assert(lextent->get_laddr() == pin_laddr); #endif @@ -1019,13 +989,13 @@ private: base_iertr::future read_pin_by_type( Transaction &t, - LBAMappingRef pin, + LBAMapping pin, extent_types_t type) { - ceph_assert(!pin->parent_modified()); - assert(!pin->is_indirect()); + ceph_assert(pin.is_viewable()); + assert(!pin.is_indirect()); // Note: pin might be a clone - auto v = pin->get_logical_extent(t); + auto v = pin.get_logical_extent(t); // checking the lba child must be atomic with creating // and linking the absent child if (v.has_child()) { @@ -1066,7 +1036,7 @@ private: template pin_to_extent_ret pin_to_extent( Transaction &t, - LBAMappingRef pin, + LBAMapping pin, child_pos_t child_pos, extent_len_t direct_partial_off, extent_len_t partial_len, @@ -1075,33 +1045,31 @@ private: // must be user-oriented required by maybe_init assert(is_user_transaction(t.get_src())); using ret = pin_to_extent_ret; - auto &pref = *pin; - auto direct_length = pref.is_indirect() ? - pref.get_intermediate_length() : - pref.get_length(); + auto direct_length = pin.is_indirect() ? + pin.get_intermediate_length() : + pin.get_length(); if (full_extent_integrity_check) { direct_partial_off = 0; partial_len = direct_length; } LOG_PREFIX(TransactionManager::pin_to_extent); SUBTRACET(seastore_tm, "getting absent extent from pin {}, 0x{:x}~0x{:x} ...", - t, *pin, direct_partial_off, partial_len); + t, pin, direct_partial_off, partial_len); return cache->get_absent_extent( t, - pref.get_val(), + pin.get_val(), direct_length, direct_partial_off, partial_len, - [&pref, maybe_init=std::move(maybe_init), + [pin=pin.duplicate(), maybe_init=std::move(maybe_init), child_pos=std::move(child_pos)] (T &extent) mutable { assert(extent.is_logical()); assert(!extent.has_laddr()); assert(!extent.has_been_invalidated()); - assert(!pref.has_been_invalidated()); - assert(pref.get_parent()); + assert(pin.is_valid()); child_pos.link_child(&extent); - extent.maybe_set_intermediate_laddr(pref); + extent.maybe_set_intermediate_laddr(pin); maybe_init(extent); extent.set_seen_by_users(); } @@ -1113,20 +1081,20 @@ private: "got extent -- {}, chksum in the lba tree: 0x{:x}, actual chksum: 0x{:x}", t, *ref, - pin->get_checksum(), + pin.get_checksum(), crc); bool inconsistent = false; if (full_extent_integrity_check) { - inconsistent = (pin->get_checksum() != crc); + inconsistent = (pin.get_checksum() != crc); } else { // !full_extent_integrity_check: remapped extent may be skipped - inconsistent = !(pin->get_checksum() == 0 || - pin->get_checksum() == crc); + inconsistent = !(pin.get_checksum() == 0 || + pin.get_checksum() == crc); } if (unlikely(inconsistent)) { SUBERRORT(seastore_tm, "extent checksum inconsistent, recorded: 0x{:x}, actual: 0x{:x}, {}", t, - pin->get_checksum(), + pin.get_checksum(), crc, *ref); ceph_abort(); @@ -1149,39 +1117,36 @@ private: LogicalChildNodeRef>; pin_to_extent_by_type_ret pin_to_extent_by_type( Transaction &t, - LBAMappingRef pin, + LBAMapping pin, child_pos_t child_pos, extent_types_t type) { LOG_PREFIX(TransactionManager::pin_to_extent_by_type); SUBTRACET(seastore_tm, "getting absent extent from pin {} type {} ...", - t, *pin, type); + t, pin, type); assert(is_logical_type(type)); assert(is_background_transaction(t.get_src())); - auto &pref = *pin; laddr_t direct_key; extent_len_t direct_length; - if (pref.is_indirect()) { - direct_key = pref.get_intermediate_base(); - direct_length = pref.get_intermediate_length(); + if (pin.is_indirect()) { + direct_key = pin.get_intermediate_base(); + direct_length = pin.get_intermediate_length(); } else { - direct_key = pref.get_key(); - direct_length = pref.get_length(); + direct_key = pin.get_key(); + direct_length = pin.get_length(); } return cache->get_absent_extent_by_type( t, type, - pref.get_val(), + pin.get_val(), direct_key, direct_length, - [&pref, child_pos=std::move(child_pos)](CachedExtent &extent) mutable { + [pin=pin.duplicate(), child_pos=std::move(child_pos)](CachedExtent &extent) mutable { assert(extent.is_logical()); auto &lextent = static_cast(extent); assert(!lextent.has_laddr()); assert(!lextent.has_been_invalidated()); - assert(!pref.has_been_invalidated()); - assert(pref.get_parent()); - assert(!pref.get_parent()->is_pending()); + assert(pin.is_valid()); child_pos.link_child(&lextent); lextent.maybe_set_intermediate_laddr(pref); // No change to extent::seen_by_user because this path is only @@ -1194,21 +1159,21 @@ private: "got extent -- {}, chksum in the lba tree: 0x{:x}, actual chksum: 0x{:x}", t, *ref, - pin->get_checksum(), + pin.get_checksum(), crc); assert(ref->is_fully_loaded()); bool inconsistent = false; if (full_extent_integrity_check) { - inconsistent = (pin->get_checksum() != crc); + inconsistent = (pin.get_checksum() != crc); } else { // !full_extent_integrity_check: remapped extent may be skipped - inconsistent = !(pin->get_checksum() == 0 || - pin->get_checksum() == crc); + inconsistent = !(pin.get_checksum() == 0 || + pin.get_checksum() == crc); } if (unlikely(inconsistent)) { SUBERRORT(seastore_tm, "extent checksum inconsistent, recorded: 0x{:x}, actual: 0x{:x}, {}", t, - pin->get_checksum(), + pin.get_checksum(), crc, *ref); ceph_abort(); diff --git a/src/crimson/tools/store_nbd/tm_driver.cc b/src/crimson/tools/store_nbd/tm_driver.cc index 870809c515332..2daf92fad3c91 100644 --- a/src/crimson/tools/store_nbd/tm_driver.cc +++ b/src/crimson/tools/store_nbd/tm_driver.cc @@ -64,7 +64,7 @@ TMDriver::read_extents_ret TMDriver::read_extents( extent_len_t length) { return seastar::do_with( - lba_pin_list_t(), + lba_mapping_list_t(), lextent_list_t(), [this, &t, offset, length](auto &pins, auto &ret) { return tm->get_pins( @@ -78,8 +78,8 @@ TMDriver::read_extents_ret TMDriver::read_extents( [this, &t, &ret](auto &&pin) { logger().debug( "read_extents: get_extent {}~{}", - pin->get_val(), - pin->get_length()); + pin.get_val(), + pin.get_length()); return tm->read_pin( t, std::move(pin) diff --git a/src/test/crimson/seastore/test_btree_lba_manager.cc b/src/test/crimson/seastore/test_btree_lba_manager.cc index 3c40fb2e01359..4f055cbade4cf 100644 --- a/src/test/crimson/seastore/test_btree_lba_manager.cc +++ b/src/test/crimson/seastore/test_btree_lba_manager.cc @@ -561,16 +561,16 @@ struct btree_lba_manager_test : btree_test_base { }); }).unsafe_get(); for (auto &ret : rets) { - logger().debug("alloc'd: {}", *ret); - EXPECT_EQ(len, ret->get_length()); - auto [b, e] = get_overlap(t, ret->get_key(), len); + logger().debug("alloc'd: {}", ret); + EXPECT_EQ(len, ret.get_length()); + auto [b, e] = get_overlap(t, ret.get_key(), len); EXPECT_EQ(b, e); t.mappings.emplace( std::make_pair( - ret->get_key(), + ret.get_key(), test_extent_t{ - ret->get_val(), - ret->get_length(), + ret.get_val(), + ret.get_length(), 1 } )); @@ -652,9 +652,9 @@ struct btree_lba_manager_test : btree_test_base { }).unsafe_get(); EXPECT_EQ(ret_list.size(), 1); auto &ret = *ret_list.begin(); - EXPECT_EQ(i.second.addr, ret->get_val()); - EXPECT_EQ(laddr, ret->get_key()); - EXPECT_EQ(len, ret->get_length()); + EXPECT_EQ(i.second.addr, ret.get_val()); + EXPECT_EQ(laddr, ret.get_key()); + EXPECT_EQ(len, ret.get_length()); auto ret_pin = with_trans_intr( *t.t, @@ -662,9 +662,9 @@ struct btree_lba_manager_test : btree_test_base { return lba_manager->get_mapping( t, laddr); }).unsafe_get(); - EXPECT_EQ(i.second.addr, ret_pin->get_val()); - EXPECT_EQ(laddr, ret_pin->get_key()); - EXPECT_EQ(len, ret_pin->get_length()); + EXPECT_EQ(i.second.addr, ret_pin.get_val()); + EXPECT_EQ(laddr, ret_pin.get_key()); + EXPECT_EQ(len, ret_pin.get_length()); } with_trans_intr( *t.t, diff --git a/src/test/crimson/seastore/test_object_data_handler.cc b/src/test/crimson/seastore/test_object_data_handler.cc index fe9a003dbdeb0..1677762fd4411 100644 --- a/src/test/crimson/seastore/test_object_data_handler.cc +++ b/src/test/crimson/seastore/test_object_data_handler.cc @@ -219,7 +219,7 @@ struct object_data_handler_test_t: } } } - std::list get_mappings( + std::list get_mappings( Transaction &t, objaddr_t offset, extent_len_t length) { @@ -231,7 +231,7 @@ struct object_data_handler_test_t: }).unsafe_get(); return ret; } - std::list get_mappings(objaddr_t offset, extent_len_t length) { + std::list get_mappings(objaddr_t offset, extent_len_t length) { auto t = create_mutate_transaction(); auto ret = with_trans_intr(*t, [&](auto &t) { auto &layout = onode->get_layout(); @@ -243,9 +243,9 @@ struct object_data_handler_test_t: } using remap_entry_t = TransactionManager::remap_entry_t; - LBAMappingRef remap_pin( + std::optional remap_pin( Transaction &t, - LBAMappingRef &&opin, + LBAMapping &&opin, extent_len_t new_offset, extent_len_t new_len) { auto pin = with_trans_intr(t, [&](auto& trans) { @@ -253,11 +253,12 @@ struct object_data_handler_test_t: trans, std::move(opin), std::array{ remap_entry_t(new_offset, new_len)} ).si_then([](auto ret) { - return std::move(ret[0]); + return TransactionManager::base_iertr::make_ready_future< + std::optional>(std::move(ret[0])); }); }).handle_error(crimson::ct_error::eagain::handle([] { - LBAMappingRef t = nullptr; - return t; + return TransactionManager::base_iertr::make_ready_future< + std::optional>(); }), crimson::ct_error::pass_further_all{}).unsafe_get(); EXPECT_TRUE(pin); return pin; @@ -648,10 +649,10 @@ TEST_P(object_data_handler_test_t, remap_left) { EXPECT_EQ(pins.size(), 2); size_t res[2] = {0, 64<<10}; - auto base = pins.front()->get_key(); + auto base = pins.front().get_key(); int i = 0; for (auto &pin : pins) { - EXPECT_EQ(pin->get_key().get_byte_distance(base), res[i]); + EXPECT_EQ(pin.get_key().get_byte_distance(base), res[i]); i++; } read(0, 128<<10); @@ -682,10 +683,10 @@ TEST_P(object_data_handler_test_t, remap_right) { EXPECT_EQ(pins.size(), 2); size_t res[2] = {0, 64<<10}; - auto base = pins.front()->get_key(); + auto base = pins.front().get_key(); int i = 0; for (auto &pin : pins) { - EXPECT_EQ(pin->get_key().get_byte_distance(base), res[i]); + EXPECT_EQ(pin.get_key().get_byte_distance(base), res[i]); i++; } read(0, 128<<10); @@ -715,10 +716,10 @@ TEST_P(object_data_handler_test_t, remap_right_left) { EXPECT_EQ(pins.size(), 3); size_t res[3] = {0, 48<<10, 80<<10}; - auto base = pins.front()->get_key(); + auto base = pins.front().get_key(); int i = 0; for (auto &pin : pins) { - EXPECT_EQ(pin->get_key().get_byte_distance(base), res[i]); + EXPECT_EQ(pin.get_key().get_byte_distance(base), res[i]); i++; } enable_max_extent_size(); @@ -746,10 +747,10 @@ TEST_P(object_data_handler_test_t, multiple_remap) { EXPECT_EQ(pins.size(), 3); size_t res[3] = {0, 120<<10, 124<<10}; - auto base = pins.front()->get_key(); + auto base = pins.front().get_key(); int i = 0; for (auto &pin : pins) { - EXPECT_EQ(pin->get_key().get_byte_distance(base), res[i]); + EXPECT_EQ(pin.get_key().get_byte_distance(base), res[i]); i++; } read(0, 128<<10); diff --git a/src/test/crimson/seastore/test_transaction_manager.cc b/src/test/crimson/seastore/test_transaction_manager.cc index 515c924f75b73..ba0c5c0691360 100644 --- a/src/test/crimson/seastore/test_transaction_manager.cc +++ b/src/test/crimson/seastore/test_transaction_manager.cc @@ -496,13 +496,13 @@ struct transaction_manager_test_t : TestBlockRef read_pin( test_transaction_t &t, - LBAMappingRef pin) { - auto addr = pin->is_indirect() - ? pin->get_intermediate_base() - : pin->get_key(); - auto len = pin->is_indirect() - ? pin->get_intermediate_length() - : pin->get_length(); + LBAMapping pin) { + auto addr = pin.is_indirect() + ? pin.get_intermediate_base() + : pin.get_key(); + auto len = pin.is_indirect() + ? pin.get_intermediate_length() + : pin.get_length(); ceph_assert(test_mappings.contains(addr, t.mapping_delta)); ceph_assert(test_mappings.get(addr, t.mapping_delta).desc.len == len); @@ -581,11 +581,11 @@ struct transaction_manager_test_t : TestBlockRef try_read_pin( test_transaction_t &t, - LBAMappingRef &&pin) { + LBAMapping &&pin) { using ertr = with_trans_ertr; - bool indirect = pin->is_indirect(); - auto addr = pin->get_key(); - auto im_addr = indirect ? pin->get_intermediate_base() : L_ADDR_NULL; + bool indirect = pin.is_indirect(); + auto addr = pin.get_key(); + auto im_addr = indirect ? pin.get_intermediate_base() : L_ADDR_NULL; auto ext = with_trans_intr(*(t.t), [&](auto& trans) { return tm->read_pin(trans, std::move(pin)); }).safe_then([](auto ret) { @@ -638,44 +638,44 @@ struct transaction_manager_test_t : return ext; } - LBAMappingRef get_pin( + LBAMapping get_pin( test_transaction_t &t, laddr_t offset) { ceph_assert(test_mappings.contains(offset, t.mapping_delta)); auto pin = with_trans_intr(*(t.t), [&](auto& trans) { return tm->get_pin(trans, offset); }).unsafe_get(); - EXPECT_EQ(offset, pin->get_key()); + EXPECT_EQ(offset, pin.get_key()); return pin; } - LBAMappingRef clone_pin( + LBAMapping clone_pin( test_transaction_t &t, laddr_t offset, const LBAMapping &mapping) { auto pin = with_trans_intr(*(t.t), [&](auto &trans) { return tm->clone_pin(trans, offset, mapping); }).unsafe_get(); - EXPECT_EQ(offset, pin->get_key()); - EXPECT_EQ(mapping.get_key(), pin->get_intermediate_key()); - EXPECT_EQ(mapping.get_key(), pin->get_intermediate_base()); - test_mappings.inc_ref(pin->get_intermediate_key(), t.mapping_delta); + EXPECT_EQ(offset, pin.get_key()); + EXPECT_EQ(mapping.get_key(), pin.get_intermediate_key()); + EXPECT_EQ(mapping.get_key(), pin.get_intermediate_base()); + test_mappings.inc_ref(pin.get_intermediate_key(), t.mapping_delta); return pin; } - LBAMappingRef try_get_pin( + std::optional try_get_pin( test_transaction_t &t, laddr_t offset) { ceph_assert(test_mappings.contains(offset, t.mapping_delta)); using ertr = with_trans_ertr; - using ret = ertr::future; + using ret = ertr::future>; auto pin = with_trans_intr(*(t.t), [&](auto& trans) { return tm->get_pin(trans, offset); }).safe_then([](auto pin) -> ret { - return ertr::make_ready_future(std::move(pin)); + return ertr::make_ready_future>(std::move(pin)); }).handle_error( [](const crimson::ct_error::eagain &e) { - return seastar::make_ready_future(); + return seastar::make_ready_future>(); }, crimson::ct_error::assert_all{ "get_extent got invalid error" @@ -1098,32 +1098,33 @@ struct transaction_manager_test_t : } using remap_entry_t = TransactionManager::remap_entry_t; - LBAMappingRef remap_pin( + std::optional remap_pin( test_transaction_t &t, - LBAMappingRef &&opin, + LBAMapping &&opin, extent_len_t new_offset, extent_len_t new_len) { if (t.t->is_conflicted()) { - return nullptr; + return std::nullopt; } - auto o_laddr = opin->get_key(); - bool indirect_opin = opin->is_indirect(); + auto o_laddr = opin.get_key(); + bool indirect_opin = opin.is_indirect(); auto data_laddr = indirect_opin - ? opin->get_intermediate_base() + ? opin.get_intermediate_base() : o_laddr; auto pin = with_trans_intr(*(t.t), [&](auto& trans) { return tm->remap_pin( trans, std::move(opin), std::array{ remap_entry_t(new_offset, new_len)} ).si_then([](auto ret) { - return std::move(ret[0]); + return TransactionManager::base_iertr::make_ready_future< + std::optional>(std::move(ret[0])); }); }).handle_error(crimson::ct_error::eagain::handle([] { - LBAMappingRef t = nullptr; - return t; + return TransactionManager::base_iertr::make_ready_future< + std::optional>(); }), crimson::ct_error::pass_further_all{}).unsafe_get(); if (t.t->is_conflicted()) { - return nullptr; + return {}; } if (indirect_opin) { test_mappings.inc_ref(data_laddr, t.mapping_delta); @@ -1145,22 +1146,27 @@ struct transaction_manager_test_t : } } else { ceph_assert(t.t->is_conflicted()); - return nullptr; + return {}; } return pin; } + struct overwrite_pin_ret_bare_t { + std::optional lpin; + TestBlockRef extent; + std::optional rpin; + }; + using _overwrite_pin_iertr = TransactionManager::get_pin_iertr; - using _overwrite_pin_ret = _overwrite_pin_iertr::future< - std::tuple>; + using _overwrite_pin_ret = _overwrite_pin_iertr::future; _overwrite_pin_ret _overwrite_pin( Transaction &t, - LBAMappingRef &&opin, + LBAMapping &&opin, extent_len_t new_offset, extent_len_t new_len, ceph::bufferlist &bl) { - auto o_laddr = opin->get_key(); - auto o_len = opin->get_length(); + auto o_laddr = opin.get_key(); + auto o_len = opin.get_length(); if (new_offset != 0 && o_len != new_offset + new_len) { return tm->remap_pin( t, @@ -1189,10 +1195,8 @@ struct transaction_manager_test_t : return tm->get_pin(t, r_laddr ).si_then([lpin = std::move(lpin), ext = std::move(ext)] (auto rpin) mutable { - return _overwrite_pin_iertr::make_ready_future< - std::tuple>( - std::make_tuple( - std::move(lpin), std::move(ext), std::move(rpin))); + return _overwrite_pin_iertr::make_ready_future( + std::move(lpin), std::move(ext), std::move(rpin)); }); }); }).handle_error_interruptible( @@ -1221,10 +1225,8 @@ struct transaction_manager_test_t : auto r_laddr = (o_laddr + new_offset + new_len).checked_to_laddr(); return tm->get_pin(t, r_laddr ).si_then([ext = std::move(ext)](auto rpin) mutable { - return _overwrite_pin_iertr::make_ready_future< - std::tuple>( - std::make_tuple( - nullptr, std::move(ext), std::move(rpin))); + return _overwrite_pin_iertr::make_ready_future( + std::nullopt, std::move(ext), std::move(rpin)); }); }); }).handle_error_interruptible( @@ -1251,10 +1253,8 @@ struct transaction_manager_test_t : iter.copy(new_len, ext->get_bptr().c_str()); return tm->get_pin(t, o_laddr ).si_then([ext = std::move(ext)](auto lpin) mutable { - return _overwrite_pin_iertr::make_ready_future< - std::tuple>( - std::make_tuple( - std::move(lpin), std::move(ext), nullptr)); + return _overwrite_pin_iertr::make_ready_future( + std::move(lpin), std::move(ext), std::nullopt); }); }); }).handle_error_interruptible( @@ -1263,36 +1263,30 @@ struct transaction_manager_test_t : ); } else { ceph_abort("impossible"); - return _overwrite_pin_iertr::make_ready_future< - std::tuple>( - std::make_tuple(nullptr, nullptr, nullptr)); + return _overwrite_pin_iertr::make_ready_future(); } } - using overwrite_pin_ret = std::tuple; - overwrite_pin_ret overwrite_pin( + overwrite_pin_ret_bare_t overwrite_pin( test_transaction_t &t, - LBAMappingRef &&opin, + LBAMapping &&opin, extent_len_t new_offset, extent_len_t new_len, ceph::bufferlist &bl) { if (t.t->is_conflicted()) { - return std::make_tuple( - nullptr, nullptr, nullptr); + return {}; } - auto o_laddr = opin->get_key(); - auto o_paddr = opin->get_val(); - auto o_len = opin->get_length(); + auto o_laddr = opin.get_key(); + auto o_paddr = opin.get_val(); + auto o_len = opin.get_length(); auto res = with_trans_intr(*(t.t), [&](auto& trans) { return _overwrite_pin( trans, std::move(opin), new_offset, new_len, bl); }).handle_error(crimson::ct_error::eagain::handle([] { - return std::make_tuple( - nullptr, nullptr, nullptr); + return _overwrite_pin_iertr::make_ready_future(); }), crimson::ct_error::pass_further_all{}).unsafe_get(); if (t.t->is_conflicted()) { - return std::make_tuple( - nullptr, nullptr, nullptr); + return {}; } test_mappings.dec_ref(o_laddr, t.mapping_delta); EXPECT_FALSE(test_mappings.contains(o_laddr, t.mapping_delta)); @@ -1311,8 +1305,7 @@ struct transaction_manager_test_t : EXPECT_TRUE(lext->is_exist_clean()); } else { ceph_assert(t.t->is_conflicted()); - return std::make_tuple( - nullptr, nullptr, nullptr); + return {}; } } EXPECT_EQ(ext->get_laddr(), o_laddr + new_offset); @@ -1329,12 +1322,10 @@ struct transaction_manager_test_t : EXPECT_TRUE(rext->is_exist_clean()); } else { ceph_assert(t.t->is_conflicted()); - return std::make_tuple( - nullptr, nullptr, nullptr); + return {}; } } - return std::make_tuple( - std::move(lpin), std::move(ext), std::move(rpin)); + return res; } void test_remap_pin() { @@ -1359,11 +1350,11 @@ struct transaction_manager_test_t : //split left auto pin1 = remap_pin(t, std::move(lpin), 0, 16 << 10); ASSERT_TRUE(pin1); - auto pin2 = remap_pin(t, std::move(pin1), 0, 8 << 10); + auto pin2 = remap_pin(t, std::move(*pin1), 0, 8 << 10); ASSERT_TRUE(pin2); - auto pin3 = remap_pin(t, std::move(pin2), 0, 4 << 10); + auto pin3 = remap_pin(t, std::move(*pin2), 0, 4 << 10); ASSERT_TRUE(pin3); - auto lext = read_pin(t, std::move(pin3)); + auto lext = read_pin(t, std::move(*pin3)); EXPECT_EQ('l', lext->get_bptr().c_str()[0]); auto mlext = mutate_extent(t, lext); ASSERT_TRUE(mlext->is_exist_mutation_pending()); @@ -1372,11 +1363,11 @@ struct transaction_manager_test_t : //split right auto pin4 = remap_pin(t, std::move(rpin), 16 << 10, 16 << 10); ASSERT_TRUE(pin4); - auto pin5 = remap_pin(t, std::move(pin4), 8 << 10, 8 << 10); + auto pin5 = remap_pin(t, std::move(*pin4), 8 << 10, 8 << 10); ASSERT_TRUE(pin5); - auto pin6 = remap_pin(t, std::move(pin5), 4 << 10, 4 << 10); + auto pin6 = remap_pin(t, std::move(*pin5), 4 << 10, 4 << 10); ASSERT_TRUE(pin6); - auto rext = read_pin(t, std::move(pin6)); + auto rext = read_pin(t, std::move(*pin6)); EXPECT_EQ('r', rext->get_bptr().c_str()[0]); auto mrext = mutate_extent(t, rext); ASSERT_TRUE(mrext->is_exist_mutation_pending()); @@ -1414,27 +1405,27 @@ struct transaction_manager_test_t : auto t = create_transaction(); auto lpin = get_pin(t, l_offset); auto rpin = get_pin(t, r_offset); - auto l_clone_pin = clone_pin(t, l_clone_offset, *lpin); - auto r_clone_pin = clone_pin(t, r_clone_offset, *rpin); + auto l_clone_pin = clone_pin(t, l_clone_offset, lpin); + auto r_clone_pin = clone_pin(t, r_clone_offset, rpin); //split left auto pin1 = remap_pin(t, std::move(l_clone_pin), 0, 16 << 10); ASSERT_TRUE(pin1); - auto pin2 = remap_pin(t, std::move(pin1), 0, 8 << 10); + auto pin2 = remap_pin(t, std::move(*pin1), 0, 8 << 10); ASSERT_TRUE(pin2); - auto pin3 = remap_pin(t, std::move(pin2), 0, 4 << 10); + auto pin3 = remap_pin(t, std::move(*pin2), 0, 4 << 10); ASSERT_TRUE(pin3); - auto lext = read_pin(t, std::move(pin3)); + auto lext = read_pin(t, std::move(*pin3)); EXPECT_EQ('l', lext->get_bptr().c_str()[0]); //split right auto pin4 = remap_pin(t, std::move(r_clone_pin), 16 << 10, 16 << 10); ASSERT_TRUE(pin4); - auto pin5 = remap_pin(t, std::move(pin4), 8 << 10, 8 << 10); + auto pin5 = remap_pin(t, std::move(*pin4), 8 << 10, 8 << 10); ASSERT_TRUE(pin5); - auto pin6 = remap_pin(t, std::move(pin5), 4 << 10, 4 << 10); + auto pin6 = remap_pin(t, std::move(*pin5), 4 << 10, 4 << 10); ASSERT_TRUE(pin6); auto int_offset = pin6->get_intermediate_offset(); - auto rext = read_pin(t, std::move(pin6)); + auto rext = read_pin(t, std::move(*pin6)); EXPECT_EQ('r', rext->get_bptr().c_str()[int_offset]); submit_transaction(std::move(t)); @@ -1480,9 +1471,9 @@ struct transaction_manager_test_t : auto [mlp1, mext1, mrp1] = overwrite_pin( t, std::move(mpin), 8 << 10 , 8 << 10, mbl1); auto [mlp2, mext2, mrp2] = overwrite_pin( - t, std::move(mrp1), 4 << 10 , 16 << 10, mbl2); + t, std::move(*mrp1), 4 << 10 , 16 << 10, mbl2); auto [mlpin3, me3, mrpin3] = overwrite_pin( - t, std::move(mrp2), 4 << 10 , 12 << 10, mbl3); + t, std::move(*mrp2), 4 << 10 , 12 << 10, mbl3); auto mlext1 = get_extent(t, mlp1->get_key(), mlp1->get_length()); auto mlext2 = get_extent(t, mlp2->get_key(), mlp2->get_length()); auto mlext3 = get_extent(t, mlpin3->get_key(), mlpin3->get_length()); @@ -1570,8 +1561,8 @@ struct transaction_manager_test_t : continue; } auto new_off = get_laddr_hint(off << 10) - .get_byte_distance(last_pin->get_key()); - auto new_len = last_pin->get_length() - new_off; + .get_byte_distance(last_pin.get_key()); + auto new_len = last_pin.get_length() - new_off; //always remap right extent at new split_point auto pin = remap_pin(t, std::move(last_pin), new_off, new_len); if (!pin) { @@ -1580,7 +1571,7 @@ struct transaction_manager_test_t : } last_pin = pin->duplicate(); } - auto last_ext = try_get_extent(t, last_pin->get_key()); + auto last_ext = try_get_extent(t, last_pin.get_key()); if (last_ext) { auto last_ext1 = mutate_extent(t, last_ext); ASSERT_TRUE(last_ext1->is_exist_mutation_pending()); @@ -1670,12 +1661,12 @@ struct transaction_manager_test_t : } empty_transaction = false; auto new_off = get_laddr_hint(start_off << 10) - .get_byte_distance(last_rpin->get_key()); + .get_byte_distance(last_rpin.get_key()); auto new_len = (end_off - start_off) << 10; bufferlist bl; bl.append(ceph::bufferptr(ceph::buffer::create(new_len, 0))); auto [lpin, ext, rpin] = overwrite_pin( - t, last_rpin->duplicate(), new_off, new_len, bl); + t, last_rpin.duplicate(), new_off, new_len, bl); if (!ext) { conflicted++; return; @@ -1696,7 +1687,7 @@ struct transaction_manager_test_t : ASSERT_TRUE(rpin); last_rpin = rpin->duplicate(); } - auto last_rext = try_get_extent(t, last_rpin->get_key()); + auto last_rext = try_get_extent(t, last_rpin.get_key()); if (!last_rext) { conflicted++; return; @@ -2143,16 +2134,17 @@ TEST_P(tm_single_device_test_t, invalid_lba_mapping_detect) { auto t = create_transaction(); auto pin = get_pin(t, get_laddr_hint((LEAF_NODE_CAPACITY - 1) * 4096)); - assert(pin->is_parent_viewable()); + assert(pin.is_viewable()); auto extent = alloc_extent(t, get_laddr_hint(LEAF_NODE_CAPACITY * 4096), 4096, 'a'); - assert(!pin->is_parent_viewable()); + assert(!pin.is_viewable()); pin = get_pin(t, get_laddr_hint(LEAF_NODE_CAPACITY * 4096)); + assert(pin.is_viewable()); std::ignore = alloc_extent(t, get_laddr_hint((LEAF_NODE_CAPACITY + 1) * 4096), 4096, 'a'); - assert(pin->is_parent_viewable()); - assert(pin->parent_modified()); - pin->maybe_fix_pos(); + assert(!pin.is_viewable()); + // TODO: refresh pin + // pin->maybe_fix_pos(); auto extent2 = with_trans_intr(*(t.t), [&pin](auto& trans) { - auto v = pin->get_logical_extent(trans); + auto v = pin.get_logical_extent(trans); assert(v.has_child()); return std::move(v.get_child_fut()); }).unsafe_get(); -- 2.39.5