From d91f745372135b909b7cda6814bfcc9435511363 Mon Sep 17 00:00:00 2001 From: Matan Breizman Date: Sun, 7 Dec 2025 09:53:03 +0000 Subject: [PATCH] crimson/os/seastore/transaction_manager: Move CRC_NULL checks check_full_extent_integrity would allow for CRC_NULL (0) checksums when `full_extent_integrity_check` was false. Instead, move this check as an assertiion into TransactionManager::read_pin. This would allow us to reuse CRC_NULL concept for more purposes e.g skipping integrity checks when no CRC is passed (next commits). Note: With this change check_full_extent_integrity could be called only on non CRC_NULL, the check would be moved in next commits. Signed-off-by: Matan Breizman --- src/crimson/os/seastore/lba/btree_lba_manager.cc | 4 ++-- src/crimson/os/seastore/transaction_manager.cc | 9 +-------- src/crimson/os/seastore/transaction_manager.h | 11 +++++++---- 3 files changed, 10 insertions(+), 14 deletions(-) diff --git a/src/crimson/os/seastore/lba/btree_lba_manager.cc b/src/crimson/os/seastore/lba/btree_lba_manager.cc index d1d4fc0d61d9..9da688d6652f 100644 --- a/src/crimson/os/seastore/lba/btree_lba_manager.cc +++ b/src/crimson/os/seastore/lba/btree_lba_manager.cc @@ -1281,8 +1281,8 @@ BtreeLBAManager::remap_mappings( val.pladdr = paddr + remap.offset; } val.refcount = EXTENT_DEFAULT_REF_COUNT; - val.checksum = 0; // the checksum should be updated later when - // committing the transaction + // Checksum will be updated when the committing the transaction + val.checksum = CRC_NULL; return btree.insert(c, iter, new_key, std::move(val) ).si_then([c, &remap, &mapping, &ret, &iter](auto p) { auto &[it, inserted] = p; diff --git a/src/crimson/os/seastore/transaction_manager.cc b/src/crimson/os/seastore/transaction_manager.cc index f62c8a774ba1..8c0b5f0d3823 100644 --- a/src/crimson/os/seastore/transaction_manager.cc +++ b/src/crimson/os/seastore/transaction_manager.cc @@ -634,14 +634,7 @@ void TransactionManager::check_full_extent_integrity( t, pin_crc, ref_crc); - bool inconsistent = false; - if (full_extent_integrity_check) { - inconsistent = (pin_crc != ref_crc); - } else { // !full_extent_integrity_check: remapped extent may be skipped - inconsistent = !(pin_crc == 0 || - pin_crc == ref_crc); - } - if (unlikely(inconsistent)) { + if (unlikely(pin_crc != ref_crc)) { SUBERRORT(seastore_tm, "extent checksum inconsistent, recorded: 0x{:x}, actual: 0x{:x}", t, diff --git a/src/crimson/os/seastore/transaction_manager.h b/src/crimson/os/seastore/transaction_manager.h index 884b8fe3f0c2..487bebe8939e 100644 --- a/src/crimson/os/seastore/transaction_manager.h +++ b/src/crimson/os/seastore/transaction_manager.h @@ -1332,9 +1332,12 @@ private: } }).si_then([this, &t, &remaps, original_paddr, original_laddr, original_len, FNAME](auto ext) mutable { - ceph_assert(full_extent_integrity_check - ? (ext && ext->is_fully_loaded()) - : true); + if (full_extent_integrity_check) { + ceph_assert(ext && ext->is_fully_loaded()); + // CRC_NULL shouldn't be possible when full extent + // integrity checks are enabled. + assert(ext->calc_crc32c() != CRC_NULL); + } std::optional original_bptr; // TODO: preserve the bufferspace if partially loaded if (ext && ext->is_fully_loaded()) { @@ -1485,7 +1488,7 @@ private: }); } - void check_full_extent_integrity( + static void check_full_extent_integrity( Transaction &t, uint32_t ref_crc, uint32_t pin_crc); /** -- 2.47.3