From 58f7b6e07777228f30d6610f67fff55e1d3fbc41 Mon Sep 17 00:00:00 2001 From: Yingxin Cheng Date: Wed, 18 Dec 2024 17:09:54 +0800 Subject: [PATCH] crimson/os/seastore: make the updates to backref_entry_mset be consistent with extents Generally, make alloc/free be consistent with add/commit/retire extents in Cache. Signed-off-by: Yingxin Cheng Signed-off-by: Zhang Song --- src/crimson/os/seastore/cache.cc | 55 ++++++++++++++++----------- src/crimson/os/seastore/cache.h | 18 ++++++++- src/crimson/os/seastore/transaction.h | 17 ++++++++- 3 files changed, 63 insertions(+), 27 deletions(-) diff --git a/src/crimson/os/seastore/cache.cc b/src/crimson/os/seastore/cache.cc index f40fd32b2bc6d..3af5c0d4500ca 100644 --- a/src/crimson/os/seastore/cache.cc +++ b/src/crimson/os/seastore/cache.cc @@ -1339,6 +1339,7 @@ record_t Cache::prepare_record( io_stat_t retire_stat; std::vector alloc_deltas; alloc_delta_t rel_delta; + backref_entry_refs_t backref_entries; rel_delta.op = alloc_delta_t::op_types_t::CLEAR; for (auto &i: t.retired_set) { auto &extent = i.extent; @@ -1355,6 +1356,25 @@ record_t Cache::prepare_record( extent->get_length(), extent->get_type())); } + + // Note: commit extents and backref allocations in the same place + if (is_backref_mapped_type(extent->get_type()) || + is_retired_placeholder_type(extent->get_type())) { + DEBUGT("backref_entry free {} len 0x{:x}", + t, + extent->get_paddr(), + extent->get_length()); + backref_entries.emplace_back( + backref_entry_t::create_retire( + extent->get_paddr(), + extent->get_length(), + extent->get_type())); + } else if (is_backref_node(extent->get_type())) { + remove_backref_extent(extent->get_paddr()); + } else { + ERRORT("Got unexpected extent type: {}", t, *extent); + ceph_abort("imposible"); + } } alloc_deltas.emplace_back(std::move(rel_delta)); @@ -1529,6 +1549,9 @@ record_t Cache::prepare_record( record.push_back(std::move(delta)); } + apply_backref_mset(backref_entries); + t.set_backref_entries(std::move(backref_entries)); + ceph_assert(t.get_fresh_block_stats().num == t.inline_block_list.size() + t.ool_block_list.size() + @@ -1628,20 +1651,17 @@ record_t Cache::prepare_record( return record; } -void Cache::commit_backref_entries( +void Cache::apply_backref_byseq( backref_entry_refs_t&& backref_entries, const journal_seq_t& seq) { - LOG_PREFIX(Cache::commit_backref_entries); + LOG_PREFIX(Cache::apply_backref_byseq); DEBUG("backref_entry apply {} entries at {}", backref_entries.size(), seq); assert(seq != JOURNAL_SEQ_NULL); if (backref_entries.empty()) { return; } - for (auto &entry : backref_entries) { - backref_entry_mset.insert(*entry); - } if (backref_entryrefs_by_seq.empty()) { backref_entryrefs_by_seq.insert( backref_entryrefs_by_seq.end(), @@ -1703,6 +1723,8 @@ void Cache::complete_commit( const auto t_src = t.get_src(); touch_extent(*i, &t_src); epm.commit_space_used(i->get_paddr(), i->get_length()); + + // Note: commit extents and backref allocations in the same place if (is_backref_mapped_type(i->get_type())) { DEBUGT("backref_entry alloc {} len 0x{:x}", t, @@ -1775,23 +1797,6 @@ void Cache::complete_commit( for (auto &i: t.retired_set) { auto &extent = i.extent; extent->dirty_from_or_retired_at = start_seq; - if (is_backref_mapped_type(extent->get_type()) || - is_retired_placeholder_type(extent->get_type())) { - DEBUGT("backref_entry free {} len 0x{:x}", - t, - extent->get_paddr(), - extent->get_length()); - backref_entries.emplace_back( - backref_entry_t::create_retire( - extent->get_paddr(), - extent->get_length(), - extent->get_type())); - } else if (is_backref_node(extent->get_type())) { - remove_backref_extent(extent->get_paddr()); - } else { - ERRORT("Got unexpected extent type: {}", t, *extent); - ceph_abort("impossible"); - } } auto existing_stats = t.get_existing_block_stats(); @@ -1808,6 +1813,9 @@ void Cache::complete_commit( } else { assert(i->state == CachedExtent::extent_state_t::DIRTY); } + add_extent(i); + + // Note: commit extents and backref allocations in the same place DEBUGT("backref_entry alloc existing {} len 0x{:x}", t, i->get_paddr(), @@ -1818,7 +1826,6 @@ void Cache::complete_commit( i->cast()->get_laddr(), i->get_length(), i->get_type())); - add_extent(i); const auto t_src = t.get_src(); if (i->is_dirty()) { add_to_dirty(i, &t_src); @@ -1827,6 +1834,8 @@ void Cache::complete_commit( } } } + + apply_backref_byseq(t.move_backref_entries(), start_seq); commit_backref_entries(std::move(backref_entries), start_seq); for (auto &i: t.pre_alloc_list) { diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h index 9a590746addf0..b2248ff37dd5f 100644 --- a/src/crimson/os/seastore/cache.h +++ b/src/crimson/os/seastore/cache.h @@ -1826,9 +1826,23 @@ private: seastar::metrics::metric_group metrics; void register_metrics(); + void apply_backref_mset( + backref_entry_refs_t& backref_entries) { + for (auto& entry : backref_entries) { + backref_entry_mset.insert(*entry); + } + } + + void apply_backref_byseq( + backref_entry_refs_t&& backref_entries, + const journal_seq_t& seq); + void commit_backref_entries( - backref_entry_refs_t&& backref_entries, - const journal_seq_t& seq); + backref_entry_refs_t&& backref_entries, + const journal_seq_t& seq) { + apply_backref_mset(backref_entries); + apply_backref_byseq(std::move(backref_entries), seq); + } /// Add extent to extents handling dirty and refcounting /// diff --git a/src/crimson/os/seastore/transaction.h b/src/crimson/os/seastore/transaction.h index 9b95161a404d5..66a9f8965207c 100644 --- a/src/crimson/os/seastore/transaction.h +++ b/src/crimson/os/seastore/transaction.h @@ -8,11 +8,12 @@ #include #include "crimson/common/log.h" +#include "crimson/os/seastore/backref_entry.h" +#include "crimson/os/seastore/cached_extent.h" #include "crimson/os/seastore/logging.h" #include "crimson/os/seastore/ordering_handle.h" -#include "crimson/os/seastore/seastore_types.h" -#include "crimson/os/seastore/cached_extent.h" #include "crimson/os/seastore/root_block.h" +#include "crimson/os/seastore/seastore_types.h" #include "crimson/os/seastore/transaction_interruptor.h" namespace crimson::os::seastore { @@ -460,6 +461,7 @@ public: ool_write_stats = {}; rewrite_stats = {}; conflicted = false; + assert(backref_entries.empty()); if (!has_reset) { has_reset = true; } @@ -575,6 +577,15 @@ private: friend class Cache; friend Ref make_test_transaction(); + void set_backref_entries(backref_entry_refs_t&& entries) { + assert(backref_entries.empty()); + backref_entries = std::move(entries); + } + + backref_entry_refs_t move_backref_entries() { + return std::move(backref_entries); + } + /** * If set, *this may not be used to perform writes and will not provide * consistentency allowing operations using to avoid maintaining a read_set. @@ -669,6 +680,8 @@ private: transaction_id_t trans_id = TRANS_ID_NULL; seastar::lw_shared_ptr pending_ool; + + backref_entry_refs_t backref_entries; }; using TransactionRef = Transaction::Ref; -- 2.39.5