From a2e244388c4822d81474bf0e45caada6c97e6e24 Mon Sep 17 00:00:00 2001 From: Xuehan Xu Date: Thu, 9 May 2024 20:02:07 +0800 Subject: [PATCH] crimson/os/seastore/transaction_manager: refactor extent remapping dec_ref/alloc/clone/inc_ref of lba mappings caused by extent remappings are now integrated into a single LBAManager::remap_lba_mappings() interface Signed-off-by: Xuehan Xu --- src/crimson/os/seastore/cache.h | 2 +- src/crimson/os/seastore/lba_manager.h | 32 +- .../lba_manager/btree/btree_lba_manager.h | 232 +++++++++++---- src/crimson/os/seastore/transaction_manager.h | 274 ++++++------------ 4 files changed, 287 insertions(+), 253 deletions(-) diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h index fb7f44b4ee1..b2bcbcae9ff 100644 --- a/src/crimson/os/seastore/cache.h +++ b/src/crimson/os/seastore/cache.h @@ -930,7 +930,7 @@ public: paddr_t remap_paddr, extent_len_t remap_length, laddr_t original_laddr, - std::optional &&original_bptr) { + std::optional &original_bptr) { LOG_PREFIX(Cache::alloc_remapped_extent); assert(remap_laddr >= original_laddr); TCachedExtentRef ext; diff --git a/src/crimson/os/seastore/lba_manager.h b/src/crimson/os/seastore/lba_manager.h index b4d34c93616..7467a36f99c 100644 --- a/src/crimson/os/seastore/lba_manager.h +++ b/src/crimson/os/seastore/lba_manager.h @@ -101,8 +101,7 @@ public: laddr_t hint, extent_len_t len, laddr_t intermediate_key, - laddr_t intermediate_base, - bool inc_ref) = 0; + laddr_t intermediate_base) = 0; virtual alloc_extent_ret reserve_region( Transaction &t, @@ -147,6 +146,35 @@ public: laddr_t addr, int delta) = 0; + struct remap_entry { + extent_len_t offset; + extent_len_t len; + remap_entry(extent_len_t _offset, extent_len_t _len) { + offset = _offset; + len = _len; + } + }; + struct lba_remap_ret_t { + ref_update_result_t ruret; + std::vector remapped_mappings; + }; + using remap_iertr = ref_iertr; + using remap_ret = remap_iertr::future; + + /** + * remap_mappings + * + * Remap an original mapping into new ones + * Return the old mapping's info and new mappings + */ + virtual remap_ret remap_mappings( + Transaction &t, + LBAMappingRef orig_mapping, + std::vector remaps, + std::vector extents // Required if and only + // if pin isn't indirect + ) = 0; + /** * Should be called after replay on each cached extent. * Implementation must initialize the LBAMapping on any diff --git a/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.h b/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.h index c4124bcf0f5..54d28195000 100644 --- a/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.h +++ b/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.h @@ -95,13 +95,10 @@ public: laddr_t interkey = L_ADDR_NULL) { assert(!indirect); - assert(value.is_paddr()); - intermediate_base = key; - intermediate_key = (interkey == L_ADDR_NULL ? key : interkey); indirect = true; - key = new_key; + intermediate_base = key; intermediate_length = len; - len = length; + adjust_mutable_indirect_attrs(new_key, length, interkey); } laddr_t get_key() const final { @@ -145,6 +142,18 @@ public: return get_map_val().checksum; } + void adjust_mutable_indirect_attrs( + laddr_t new_key, + extent_len_t length, + laddr_t interkey = L_ADDR_NULL) + { + assert(indirect); + assert(value.is_paddr()); + intermediate_key = (interkey == L_ADDR_NULL ? key : interkey); + key = new_key; + len = length; + } + protected: std::unique_ptr> _duplicate( op_context_t ctx) const final { @@ -247,63 +256,33 @@ public: alloc_extent_ret clone_mapping( Transaction &t, - laddr_t hint, + laddr_t laddr, extent_len_t len, laddr_t intermediate_key, - laddr_t intermediate_base, - bool inc_ref) final + laddr_t intermediate_base) final { - assert(intermediate_key != L_ADDR_NULL); - assert(intermediate_base != L_ADDR_NULL); - std::vector alloc_infos = { - alloc_mapping_info_t{ - len, - intermediate_key, - 0, // crc will only be used and checked with LBA direct mappings - // also see pin_to_extent(_by_type) - nullptr}}; - return seastar::do_with( - std::move(alloc_infos), - [this, &t, intermediate_base, hint, inc_ref](auto &alloc_infos) { - return _alloc_extents( - t, - hint, - alloc_infos, - EXTENT_DEFAULT_REF_COUNT - ).si_then([&t, this, intermediate_base, inc_ref](auto mappings) { - assert(mappings.size() == 1); - auto indirect_mapping = std::move(mappings.front()); - assert(indirect_mapping->is_indirect()); - auto to_indirect = [](auto &mapping, const auto &imapping) mutable { - ceph_assert(mapping->is_stable()); - mapping->make_indirect( - imapping->get_key(), - imapping->get_length(), - imapping->get_intermediate_key()); - }; - if (inc_ref) { - return update_refcount(t, intermediate_base, 1, false - ).si_then([imapping=std::move(indirect_mapping), - to_indirect=std::move(to_indirect)](auto p) mutable { - auto mapping = std::move(p.mapping); - to_indirect(mapping, imapping); - return seastar::make_ready_future< - LBAMappingRef>(std::move(mapping)); - }); - } else { - return _get_mapping(t, intermediate_base - ).si_then([imapping=std::move(indirect_mapping), - to_indirect=std::move(to_indirect)](auto mapping) mutable { - to_indirect(mapping, imapping); - return seastar::make_ready_future< - LBAMappingRef>(std::move(mapping)); - }); - } - }).handle_error_interruptible( - crimson::ct_error::input_output_error::pass_further{}, - crimson::ct_error::assert_all{"unexpect enoent"} - ); - }); + return alloc_cloned_mapping( + t, + laddr, + len, + intermediate_key + ).si_then([&t, this, intermediate_base](auto imapping) { + return update_refcount(t, intermediate_base, 1, false + ).si_then([imapping=std::move(imapping)](auto p) mutable { + auto mapping = std::move(p.mapping); + ceph_assert(mapping->is_stable()); + ceph_assert(imapping->is_indirect()); + mapping->make_indirect( + imapping->get_key(), + imapping->get_length(), + imapping->get_intermediate_key()); + return seastar::make_ready_future< + LBAMappingRef>(std::move(mapping)); + }); + }).handle_error_interruptible( + crimson::ct_error::input_output_error::pass_further{}, + crimson::ct_error::assert_all{"unexpect enoent"} + ); } alloc_extent_ret alloc_extent( @@ -383,6 +362,109 @@ public: }); } + remap_ret remap_mappings( + Transaction &t, + LBAMappingRef orig_mapping, + std::vector remaps, + std::vector extents) final { + LOG_PREFIX(BtreeLBAManager::remap_mappings); + assert((orig_mapping->is_indirect()) + == (remaps.size() != extents.size())); + return seastar::do_with( + lba_remap_ret_t{}, + std::move(remaps), + std::move(extents), + std::move(orig_mapping), + [&t, FNAME, this](auto &ret, auto &remaps, + auto &extents, auto &orig_mapping) { + return update_refcount(t, orig_mapping->get_key(), -1, false + ).si_then([&ret, this, &extents, &remaps, + &t, &orig_mapping, FNAME](auto r) { + ret.ruret = std::move(r.ref_update_res); + if (!orig_mapping->is_indirect()) { + ceph_assert(ret.ruret.refcount == 0 && + ret.ruret.addr.is_paddr() && + !ret.ruret.addr.get_paddr().is_zero()); + } + return trans_intr::do_for_each( + boost::make_counting_iterator(size_t(0)), + boost::make_counting_iterator(remaps.size()), + [&remaps, &t, this, &orig_mapping, &extents, FNAME, &ret](auto i) { + laddr_t orig_laddr = orig_mapping->get_key(); + extent_len_t orig_len = orig_mapping->get_length(); + paddr_t orig_paddr = orig_mapping->get_val(); + laddr_t intermediate_base = orig_mapping->is_indirect() + ? orig_mapping->get_intermediate_base() + : L_ADDR_NULL; + laddr_t intermediate_key = orig_mapping->is_indirect() + ? orig_mapping->get_intermediate_key() + : L_ADDR_NULL; + auto &remap = remaps[i]; + auto remap_offset = remap.offset; + auto remap_len = remap.len; + auto remap_laddr = orig_laddr + remap_offset; + auto remap_paddr = orig_paddr.add_offset(remap_offset); + if (orig_mapping->is_indirect()) { + ceph_assert(intermediate_base != L_ADDR_NULL); + ceph_assert(intermediate_key != L_ADDR_NULL); + remap_paddr = orig_paddr; + } + ceph_assert(remap_len < orig_len); + ceph_assert(remap_offset + remap_len <= orig_len); + ceph_assert(remap_len != 0); + SUBDEBUGT(seastore_lba, + "remap laddr: {}, remap paddr: {}, remap length: {}," + " intermediate_base: {}, intermediate_key: {}", t, + remap_laddr, remap_paddr, remap_len, + intermediate_base, intermediate_key); + auto fut = alloc_extent_iertr::make_ready_future(); + if (orig_mapping->is_indirect()) { + assert(intermediate_base != L_ADDR_NULL + && intermediate_key != L_ADDR_NULL); + auto remapped_intermediate_key = intermediate_key + remap_offset; + fut = alloc_cloned_mapping( + t, + remap_laddr, + remap_len, + remapped_intermediate_key + ).si_then([&orig_mapping](auto imapping) mutable { + auto mapping = orig_mapping->duplicate(); + auto bmapping = static_cast(mapping.get()); + bmapping->adjust_mutable_indirect_attrs( + imapping->get_key(), + imapping->get_length(), + imapping->get_intermediate_key()); + return seastar::make_ready_future( + std::move(mapping)); + }); + } else { + fut = alloc_extent(t, remap_laddr, *extents[i]); + } + return fut.si_then([remap_laddr, remap_len, &ret, + remap_paddr](auto &&ref) { + assert(ref->get_key() == remap_laddr); + assert(ref->get_val() == remap_paddr); + assert(ref->get_length() == remap_len); + ret.remapped_mappings.emplace_back(std::move(ref)); + return seastar::now(); + }); + }); + }).si_then([&remaps, &t, &orig_mapping, this] { + if (remaps.size() > 1 && orig_mapping->is_indirect()) { + auto intermediate_base = orig_mapping->get_intermediate_base(); + return incref_extent(t, intermediate_base, remaps.size() - 1 + ).si_then([](auto) { + return seastar::now(); + }); + } + return ref_iertr::now(); + }).si_then([&ret, &remaps] { + assert(ret.remapped_mappings.size() == remaps.size()); + return seastar::make_ready_future(std::move(ret)); + }); + }); + } + /** * init_cached_extent * @@ -484,6 +566,38 @@ private: std::vector &alloc_infos, extent_ref_count_t refcount); + alloc_extent_iertr::future alloc_cloned_mapping( + Transaction &t, + laddr_t laddr, + extent_len_t len, + laddr_t intermediate_key) + { + assert(intermediate_key != L_ADDR_NULL); + std::vector alloc_infos = { + alloc_mapping_info_t{ + len, + intermediate_key, + 0, // crc will only be used and checked with LBA direct mappings + // also see pin_to_extent(_by_type) + nullptr}}; + return seastar::do_with( + std::move(alloc_infos), + [this, &t, laddr](auto &alloc_infos) { + return _alloc_extents( + t, + laddr, + alloc_infos, + EXTENT_DEFAULT_REF_COUNT + ).si_then([laddr](auto mappings) { + ceph_assert(mappings.size() == 1); + auto mapping = std::move(mappings.front()); + ceph_assert(mapping->get_key() == laddr); + return std::unique_ptr( + static_cast(mapping.release())); + }); + }); + } + using _get_mapping_ret = get_mapping_iertr::future; _get_mapping_ret _get_mapping( Transaction &t, diff --git a/src/crimson/os/seastore/transaction_manager.h b/src/crimson/os/seastore/transaction_manager.h index 67e04175815..9de52456793 100644 --- a/src/crimson/os/seastore/transaction_manager.h +++ b/src/crimson/os/seastore/transaction_manager.h @@ -396,19 +396,11 @@ public: * Remap original extent to new extents. * Return the pins of new extent. */ - struct remap_entry { - extent_len_t offset; - extent_len_t len; - remap_entry(extent_len_t _offset, extent_len_t _len) { - offset = _offset; - len = _len; - } - }; + using remap_entry = LBAManager::remap_entry; using remap_pin_iertr = base_iertr; - template - using remap_pin_ret = remap_pin_iertr::future>; + using remap_pin_ret = remap_pin_iertr::future>; template - remap_pin_ret remap_pin( + remap_pin_ret remap_pin( Transaction &t, LBAMappingRef &&pin, std::array remaps) { @@ -439,124 +431,88 @@ public: } #endif - // The according extent might be stable or pending. - auto fut = base_iertr::make_ready_future>(); - if (full_extent_integrity_check) { - fut = read_pin(t, pin->duplicate()); - } else { - fut = cache->get_extent_if_cached( - t, pin->get_val(), T::TYPE - ).si_then([](auto extent) { - if (extent) { - return extent->template cast(); - } else { - return TCachedExtentRef(); - } - }); - } - return fut.si_then([this, &t, remaps, - original_laddr = pin->get_key(), - intermediate_base = pin->is_indirect() - ? pin->get_intermediate_base() - : L_ADDR_NULL, - intermediate_key = pin->is_indirect() - ? pin->get_intermediate_key() - : L_ADDR_NULL, - original_paddr = pin->get_val(), - original_len = pin->get_length()](auto ext) mutable { - std::optional original_bptr; + return seastar::do_with( + std::vector(), + 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(); LOG_PREFIX(TransactionManager::remap_pin); SUBDEBUGT(seastore_tm, - "original laddr: {}, original paddr: {}, original length: {}," - " intermediate_base: {}, intermediate_key: {}," - " remap to {} extents", - t, original_laddr, original_paddr, original_len, - intermediate_base, intermediate_key, remaps.size()); - ceph_assert( - (intermediate_base == L_ADDR_NULL) - == (intermediate_key == L_ADDR_NULL)); - ceph_assert(full_extent_integrity_check - ? (ext && ext->is_fully_loaded()) - : true); - if (ext) { - ceph_assert(!ext->is_mutable()); - ceph_assert(ext->get_length() >= original_len); - ceph_assert(ext->get_paddr() == original_paddr); - original_bptr = ext->get_bptr(); - } - return seastar::do_with( - std::array(), - 0, - std::move(original_bptr), - std::vector(remaps.begin(), remaps.end()), - [this, &t, original_laddr, original_paddr, - original_len, intermediate_base, intermediate_key] - (auto &ret, auto &count, auto &original_bptr, auto &remaps) { - return _dec_ref(t, original_laddr, false - ).si_then([this, &t, &original_bptr, &ret, &count, - &remaps, intermediate_base, intermediate_key, - original_laddr, original_paddr, original_len](auto) { - return trans_intr::do_for_each( - remaps.begin(), - remaps.end(), - [this, &t, &original_bptr, &ret, - &count, intermediate_base, intermediate_key, - original_laddr, original_paddr, original_len](auto &remap) { - LOG_PREFIX(TransactionManager::remap_pin); - auto remap_offset = remap.offset; - auto remap_len = remap.len; - auto remap_laddr = original_laddr + remap_offset; - auto remap_paddr = original_paddr.add_offset(remap_offset); - if (intermediate_key != L_ADDR_NULL) { - remap_paddr = original_paddr; - } - ceph_assert(remap_len < original_len); - ceph_assert(remap_offset + remap_len <= original_len); - ceph_assert(remap_len != 0); - ceph_assert(remap_offset % cache->get_block_size() == 0); - ceph_assert(remap_len % cache->get_block_size() == 0); - SUBDEBUGT(seastore_tm, - "remap laddr: {}, remap paddr: {}, remap length: {}", t, - remap_laddr, remap_paddr, remap_len); - auto remapped_intermediate_key = intermediate_key; - if (remapped_intermediate_key != L_ADDR_NULL) { - assert(intermediate_base != L_ADDR_NULL); - remapped_intermediate_key += remap_offset; - } - return alloc_remapped_extent( - t, - remap_laddr, - remap_paddr, - remap_len, - original_laddr, - intermediate_base, - remapped_intermediate_key, - std::move(original_bptr) - ).si_then([&ret, &count, remap_laddr](auto &&npin) { - ceph_assert(npin->get_key() == remap_laddr); - ret[count++] = std::move(npin); - }); - }); - }).si_then([this, &t, intermediate_base, intermediate_key] { - if (N > 1 && intermediate_key != L_ADDR_NULL) { - return lba_manager->incref_extent( - t, intermediate_base, N - 1 - ).si_then([](auto) { - return seastar::now(); - }); + "original laddr: {}, original paddr: {}, original length: {}," + " remap to {} extents", + t, original_laddr, original_paddr, original_len, remaps.size()); + // The according extent might be stable or pending. + auto fut = base_iertr::now(); + if (!pin->is_indirect()) { + auto fut2 = base_iertr::make_ready_future>(); + if (full_extent_integrity_check) { + fut2 = read_pin(t, pin->duplicate()); + } else { + auto ret = get_extent_if_linked(t, pin->duplicate()); + if (ret.index() == 1) { + fut2 = std::move(std::get<1>(ret)); } - return LBAManager::ref_iertr::now(); - }).handle_error_interruptible( - remap_pin_iertr::pass_further{}, - crimson::ct_error::assert_all{ - "TransactionManager::remap_pin hit invalid error" - } - ).si_then([&ret, &count] { - ceph_assert(count == N); - return remap_pin_iertr::make_ready_future< - std::array>(std::move(ret)); - }); - }); + } + fut = fut2.si_then([this, &t, &remaps, original_paddr, + original_laddr, original_len, + &extents, FNAME](auto ext) mutable { + ceph_assert(full_extent_integrity_check + ? (ext && ext->is_fully_loaded()) + : true); + std::optional original_bptr; + if (ext && ext->is_fully_loaded()) { + ceph_assert(!ext->is_mutable()); + ceph_assert(ext->get_length() >= original_len); + ceph_assert(ext->get_paddr() == original_paddr); + original_bptr = ext->get_bptr(); + } + if (ext) { + cache->retire_extent(t, ext); + } else { + cache->retire_absent_extent_addr(t, original_paddr, original_len); + } + for (auto &remap : remaps) { + auto remap_offset = remap.offset; + auto remap_len = remap.len; + auto remap_laddr = original_laddr + remap_offset; + auto remap_paddr = original_paddr.add_offset(remap_offset); + ceph_assert(remap_len < original_len); + ceph_assert(remap_offset + remap_len <= original_len); + ceph_assert(remap_len != 0); + ceph_assert(remap_offset % cache->get_block_size() == 0); + ceph_assert(remap_len % cache->get_block_size() == 0); + SUBDEBUGT(seastore_tm, + "remap laddr: {}, remap paddr: {}, remap length: {}", t, + remap_laddr, remap_paddr, remap_len); + extents.emplace_back(cache->alloc_remapped_extent( + t, + remap_laddr, + remap_paddr, + remap_len, + original_laddr, + original_bptr)); + } + }); + } + return fut.si_then([this, &t, &pin, &remaps, &extents] { + return lba_manager->remap_mappings( + t, + std::move(pin), + std::vector(remaps.begin(), remaps.end()), + std::move(extents) + ).si_then([](auto ret) { + return Cache::retire_extent_iertr::make_ready_future< + std::vector>(std::move(ret.remapped_mappings)); + }); + }).handle_error_interruptible( + remap_pin_iertr::pass_further{}, + crimson::ct_error::assert_all{ + "TransactionManager::remap_pin hit invalid error" + } + ); }); } @@ -607,8 +563,7 @@ public: hint, mapping.get_length(), intermediate_key, - intermediate_base, - true + intermediate_base ); } @@ -987,69 +942,6 @@ private: }); } - /** - * alloc_remapped_extent - * - * Allocates a new extent at given remap_paddr that must be absolute and - * use the buffer to fill the new extent if buffer exists. Otherwise, will - * not read disk to fill the new extent. - * Returns the new extent. - * - * Should make sure the end laddr of remap extent <= the end laddr of - * original extent when using this method. - */ - using alloc_remapped_extent_iertr = - alloc_extent_iertr::extend_ertr; - using alloc_remapped_extent_ret = - alloc_remapped_extent_iertr::future; - template - alloc_remapped_extent_ret alloc_remapped_extent( - Transaction &t, - laddr_t remap_laddr, - paddr_t remap_paddr, - extent_len_t remap_length, - laddr_t original_laddr, - laddr_t intermediate_base, - laddr_t intermediate_key, - std::optional &&original_bptr) { - LOG_PREFIX(TransactionManager::alloc_remapped_extent); - SUBDEBUG(seastore_tm, "alloc remapped extent: remap_laddr: {}, " - "remap_paddr: {}, remap_length: {}, has data in cache: {} ", - remap_laddr, remap_paddr, remap_length, - original_bptr.has_value() ? "true":"false"); - TCachedExtentRef ext; - auto fut = LBAManager::alloc_extent_iertr::make_ready_future< - LBAMappingRef>(); - assert((intermediate_key == L_ADDR_NULL) - == (intermediate_base == L_ADDR_NULL)); - if (intermediate_key == L_ADDR_NULL) { - // remapping direct mapping - ext = cache->alloc_remapped_extent( - t, - remap_laddr, - remap_paddr, - remap_length, - original_laddr, - std::move(original_bptr)); - fut = lba_manager->alloc_extent(t, remap_laddr, *ext); - } else { - fut = lba_manager->clone_mapping( - t, - remap_laddr, - remap_length, - intermediate_key, - intermediate_base, - false); - } - return fut.si_then([remap_laddr, remap_length, remap_paddr](auto &&ref) { - assert(ref->get_key() == remap_laddr); - assert(ref->get_val() == remap_paddr); - assert(ref->get_length() == remap_length); - return alloc_remapped_extent_iertr::make_ready_future - (std::move(ref)); - }); - } - public: // Testing interfaces auto get_epm() { -- 2.39.5