From e92273a4f15987d08b9e342b18e67dc310329887 Mon Sep 17 00:00:00 2001 From: Myoungwon Oh Date: Thu, 4 Jul 2024 18:04:38 +0900 Subject: [PATCH] crimson/os/seastore: disable crc calculation if end to end data protection is enabled Signed-off-by: Myoungwon Oh --- src/crimson/os/seastore/cache.cc | 8 +++- src/crimson/os/seastore/cache.h | 8 +++- .../os/seastore/extent_placement_manager.h | 9 ++++ src/crimson/os/seastore/journal.h | 2 + .../journal/circular_bounded_journal.h | 4 ++ .../seastore/journal/circular_journal_space.h | 4 ++ .../os/seastore/journal/segmented_journal.h | 5 +++ src/crimson/os/seastore/lba_manager.cc | 1 - src/crimson/os/seastore/seastore_types.h | 1 + .../os/seastore/transaction_manager.cc | 41 ++++++++++++++----- src/crimson/os/seastore/transaction_manager.h | 7 ++++ 11 files changed, 76 insertions(+), 14 deletions(-) diff --git a/src/crimson/os/seastore/cache.cc b/src/crimson/os/seastore/cache.cc index ef1fec8766c7a..74688d8db22b5 100644 --- a/src/crimson/os/seastore/cache.cc +++ b/src/crimson/os/seastore/cache.cc @@ -1562,7 +1562,13 @@ void Cache::complete_commit( is_inline = true; i->set_paddr(final_block_start.add_relative(i->get_paddr())); } - assert(i->get_last_committed_crc() == i->calc_crc32c()); +#ifndef NDEBUG + if (i->get_paddr().is_root() || epm.get_checksum_needed(i->get_paddr())) { + assert(i->get_last_committed_crc() == i->calc_crc32c()); + } else { + assert(i->get_last_committed_crc() == CRC_NULL); + } +#endif i->pending_for_transaction = TRANS_ID_NULL; i->on_initial_write(); diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h index 5af65f4b9e438..0e4e14f557422 100644 --- a/src/crimson/os/seastore/cache.h +++ b/src/crimson/os/seastore/cache.h @@ -1651,7 +1651,7 @@ private: extent->get_length(), extent->get_bptr() ).safe_then( - [extent=std::move(extent)]() mutable { + [extent=std::move(extent), this]() mutable { LOG_PREFIX(Cache::read_extent); if (likely(extent->state == CachedExtent::extent_state_t::CLEAN_PENDING)) { extent->state = CachedExtent::extent_state_t::CLEAN; @@ -1662,7 +1662,11 @@ private: if (extent->is_valid()) { // crc will be checked against LBA leaf entry for logical extents, // or check against in-extent crc for physical extents. - extent->last_committed_crc = extent->calc_crc32c(); + if (epm.get_checksum_needed(extent->get_paddr())) { + extent->last_committed_crc = extent->calc_crc32c(); + } else { + extent->last_committed_crc = CRC_NULL; + } extent->on_clean_read(); } extent->complete_io(); diff --git a/src/crimson/os/seastore/extent_placement_manager.h b/src/crimson/os/seastore/extent_placement_manager.h index 2985308e13bbd..8c51d587586ec 100644 --- a/src/crimson/os/seastore/extent_placement_manager.h +++ b/src/crimson/os/seastore/extent_placement_manager.h @@ -551,6 +551,15 @@ public: return background_process.run_until_halt(); } + bool get_checksum_needed(paddr_t addr) { + // checksum offloading only for blocks physically stored in the device + if (addr.is_fake()) { + return true; + } + assert(addr.is_absolute()); + return !devices_by_id[addr.get_device_id()]->is_end_to_end_data_protection(); + } + private: rewrite_gen_t adjust_generation( data_category_t category, diff --git a/src/crimson/os/seastore/journal.h b/src/crimson/os/seastore/journal.h index 724b50041fd52..a5c9029c43cb2 100644 --- a/src/crimson/os/seastore/journal.h +++ b/src/crimson/os/seastore/journal.h @@ -107,6 +107,8 @@ public: virtual ~Journal() {} virtual backend_type_t get_type() = 0; + + virtual bool is_checksum_needed() = 0; }; using JournalRef = std::unique_ptr; diff --git a/src/crimson/os/seastore/journal/circular_bounded_journal.h b/src/crimson/os/seastore/journal/circular_bounded_journal.h index 077da32c9a22e..874bd8dc086d9 100644 --- a/src/crimson/os/seastore/journal/circular_bounded_journal.h +++ b/src/crimson/os/seastore/journal/circular_bounded_journal.h @@ -186,6 +186,10 @@ public: return get_journal_end(); } + bool is_checksum_needed() final { + return cjs.is_checksum_needed(); + } + // Test interfaces CircularJournalSpace& get_cjs() { diff --git a/src/crimson/os/seastore/journal/circular_journal_space.h b/src/crimson/os/seastore/journal/circular_journal_space.h index c88b65ad5e6b3..ae18656b06162 100644 --- a/src/crimson/os/seastore/journal/circular_journal_space.h +++ b/src/crimson/os/seastore/journal/circular_journal_space.h @@ -242,6 +242,10 @@ class CircularJournalSpace : public JournalAllocator { return header; } + bool is_checksum_needed() { + return !device->is_end_to_end_data_protection(); + } + private: std::string print_name; cbj_header_t header; diff --git a/src/crimson/os/seastore/journal/segmented_journal.h b/src/crimson/os/seastore/journal/segmented_journal.h index 736b8c01293ed..891de7ec30697 100644 --- a/src/crimson/os/seastore/journal/segmented_journal.h +++ b/src/crimson/os/seastore/journal/segmented_journal.h @@ -63,6 +63,11 @@ public: return seastar::now(); } + bool is_checksum_needed() final { + // segmented journal always requires checksum + return true; + } + private: submit_record_ret do_submit_record( record_t &&record, diff --git a/src/crimson/os/seastore/lba_manager.cc b/src/crimson/os/seastore/lba_manager.cc index fc0b72a9814c1..6a029efc66edd 100644 --- a/src/crimson/os/seastore/lba_manager.cc +++ b/src/crimson/os/seastore/lba_manager.cc @@ -13,7 +13,6 @@ LBAManager::update_mappings( { return trans_intr::do_for_each(extents, [this, &t](auto &extent) { - assert(extent->get_last_committed_crc()); return update_mapping( t, extent->get_laddr(), diff --git a/src/crimson/os/seastore/seastore_types.h b/src/crimson/os/seastore/seastore_types.h index c2c6ec56882fc..dc2f478136ea3 100644 --- a/src/crimson/os/seastore/seastore_types.h +++ b/src/crimson/os/seastore/seastore_types.h @@ -39,6 +39,7 @@ inline depth_le_t init_depth_le(uint32_t i) { } using checksum_t = uint32_t; +constexpr checksum_t CRC_NULL = 0; // Immutable metadata for seastore to set at mkfs time struct seastore_meta_t { diff --git a/src/crimson/os/seastore/transaction_manager.cc b/src/crimson/os/seastore/transaction_manager.cc index 0a4e59ebd2617..778618bfc1b86 100644 --- a/src/crimson/os/seastore/transaction_manager.cc +++ b/src/crimson/os/seastore/transaction_manager.cc @@ -331,7 +331,7 @@ TransactionManager::update_lba_mappings( std::list(), std::list(), [this, &t, &pre_allocated_extents](auto &lextents, auto &pextents) { - auto chksum_func = [&lextents, &pextents](auto &extent) { + auto chksum_func = [&lextents, &pextents, this](auto &extent) { if (!extent->is_valid() || !extent->is_fully_loaded() || // EXIST_MUTATION_PENDING extents' crc will be calculated when @@ -343,10 +343,20 @@ TransactionManager::update_lba_mappings( // for rewritten extents, last_committed_crc should have been set // because the crc of the original extent may be reused. // also see rewrite_logical_extent() - if (!extent->get_last_committed_crc()) { - extent->set_last_committed_crc(extent->calc_crc32c()); - } - assert(extent->calc_crc32c() == extent->get_last_committed_crc()); + if (!extent->get_last_committed_crc()) { + if (get_checksum_needed(extent->get_paddr())) { + extent->set_last_committed_crc(extent->calc_crc32c()); + } else { + extent->set_last_committed_crc(CRC_NULL); + } + } +#ifndef NDEBUG + if (get_checksum_needed(extent->get_paddr())) { + assert(extent->get_last_committed_crc() == extent->calc_crc32c()); + } else { + assert(extent->get_last_committed_crc() == CRC_NULL); + } +#endif lextents.emplace_back(extent->template cast()); } else { pextents.emplace_back(extent); @@ -367,15 +377,20 @@ TransactionManager::update_lba_mappings( return lba_manager->update_mappings( t, lextents - ).si_then([&pextents] { + ).si_then([&pextents, this] { for (auto &extent : pextents) { assert(!extent->is_logical() && extent->is_valid()); // for non-logical extents, we update its last_committed_crc // and in-extent checksum fields // For pre-allocated fresh physical extents, update in-extent crc. - auto crc = extent->calc_crc32c(); - extent->set_last_committed_crc(crc); - extent->update_in_extent_chksum_field(crc); + checksum_t crc; + if (get_checksum_needed(extent->get_paddr())) { + crc = extent->calc_crc32c(); + } else { + crc = CRC_NULL; + } + extent->set_last_committed_crc(crc); + extent->update_in_extent_chksum_field(crc); } }); }); @@ -516,7 +531,13 @@ TransactionManager::rewrite_logical_extent( DEBUGT("rewriting logical extent -- {} to {}", t, *lextent, *nlextent); - assert(lextent->get_last_committed_crc() == lextent->calc_crc32c()); +#ifndef NDEBUG + if (get_checksum_needed(lextent->get_paddr())) { + assert(lextent->get_last_committed_crc() == lextent->calc_crc32c()); + } else { + assert(lextent->get_last_committed_crc() == CRC_NULL); + } +#endif nlextent->set_last_committed_crc(lextent->get_last_committed_crc()); /* This update_mapping is, strictly speaking, unnecessary for delayed_alloc * extents since we're going to do it again once we either do the ool write diff --git a/src/crimson/os/seastore/transaction_manager.h b/src/crimson/os/seastore/transaction_manager.h index 8db88628ed967..c5bdda6741bc2 100644 --- a/src/crimson/os/seastore/transaction_manager.h +++ b/src/crimson/os/seastore/transaction_manager.h @@ -991,6 +991,13 @@ private: }); } + bool get_checksum_needed(paddr_t paddr) { + if (paddr.is_record_relative()) { + return journal->is_checksum_needed(); + } + return epm->get_checksum_needed(paddr); + } + public: // Testing interfaces auto get_epm() { -- 2.39.5