From: Yingxin Cheng Date: Wed, 2 Apr 2025 02:56:45 +0000 (+0800) Subject: crimson/os/seastore/lba_manager: simplify LBAManager::update_mapping(s)() signature X-Git-Tag: v20.3.0~129^2~6 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=d6f5f3b9e6c7dd1b12100950dfd452afba4b6f90;p=ceph.git crimson/os/seastore/lba_manager: simplify LBAManager::update_mapping(s)() signature Signed-off-by: Yingxin Cheng --- diff --git a/src/crimson/os/seastore/lba_manager.cc b/src/crimson/os/seastore/lba_manager.cc index 7d32855ee451..50850a7de56d 100644 --- a/src/crimson/os/seastore/lba_manager.cc +++ b/src/crimson/os/seastore/lba_manager.cc @@ -6,27 +6,6 @@ namespace crimson::os::seastore { -LBAManager::update_mappings_ret -LBAManager::update_mappings( - Transaction& t, - const std::list& extents) -{ - return trans_intr::do_for_each(extents, - [this, &t](auto &extent) { - return update_mapping( - t, - extent->get_laddr(), - extent->get_length(), - extent->get_prior_paddr_and_reset(), - extent->get_length(), - extent->get_paddr(), - extent->get_last_committed_crc(), - nullptr // all the extents should have already been - // added to the fixed_kv_btree - ).discard_result(); - }); -} - LBAManagerRef lba_manager::create_lba_manager(Cache &cache) { return LBAManagerRef(new btree::BtreeLBAManager(cache)); } diff --git a/src/crimson/os/seastore/lba_manager.h b/src/crimson/os/seastore/lba_manager.h index a4da86f111b5..331e8b56e152 100644 --- a/src/crimson/os/seastore/lba_manager.h +++ b/src/crimson/os/seastore/lba_manager.h @@ -192,7 +192,7 @@ public: /** * update_mapping * - * update lba mapping for a delayed allocated extent + * update lba mapping for rewrite */ using update_mapping_iertr = base_iertr; using update_mapping_ret = base_iertr::future; @@ -201,10 +201,7 @@ public: laddr_t laddr, extent_len_t prev_len, paddr_t prev_addr, - extent_len_t len, - paddr_t paddr, - uint32_t checksum, - LogicalChildNode *nextent) = 0; + LogicalChildNode& nextent) = 0; /** * update_mappings @@ -213,9 +210,9 @@ public: */ using update_mappings_iertr = update_mapping_iertr; using update_mappings_ret = update_mappings_iertr::future<>; - update_mappings_ret update_mappings( + virtual update_mappings_ret update_mappings( Transaction& t, - const std::list& extents); + const std::list& extents) = 0; /** * get_physical_extent_if_live diff --git a/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.cc b/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.cc index 5f4816e6cc46..74d85390a333 100644 --- a/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.cc +++ b/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.cc @@ -595,19 +595,21 @@ BtreeLBAManager::update_mapping( laddr_t laddr, extent_len_t prev_len, paddr_t prev_addr, - extent_len_t len, - paddr_t addr, - uint32_t checksum, - LogicalChildNode *nextent) + LogicalChildNode& nextent) { LOG_PREFIX(BtreeLBAManager::update_mapping); - TRACET("laddr={}, paddr {} => {}", t, laddr, prev_addr, addr); + auto addr = nextent.get_paddr(); + auto len = nextent.get_length(); + auto checksum = nextent.get_last_committed_crc(); + TRACET("laddr={}, paddr {}~0x{:x} => {}~0x{:x}, crc=0x{:x}", + t, laddr, prev_addr, prev_len, addr, len, checksum); + assert(laddr == nextent.get_laddr()); + assert(!addr.is_null()); return _update_mapping( t, laddr, - [prev_addr, addr, prev_len, len, checksum]( - const lba_map_val_t &in) { - assert(!addr.is_null()); + [prev_addr, addr, prev_len, len, checksum] + (const lba_map_val_t &in) { lba_map_val_t ret = in; ceph_assert(in.pladdr.is_paddr()); ceph_assert(in.pladdr.get_paddr() == prev_addr); @@ -617,11 +619,11 @@ BtreeLBAManager::update_mapping( ret.checksum = checksum; return ret; }, - nextent - ).si_then([&t, laddr, prev_addr, addr, FNAME](auto res) { + &nextent + ).si_then([&t, laddr, prev_addr, prev_len, addr, len, checksum, FNAME](auto res) { auto &result = res.map_value; - DEBUGT("laddr={}, paddr {} => {} done -- {}", - t, laddr, prev_addr, addr, result); + DEBUGT("laddr={}, paddr {}~0x{:x} => {}~0x{:x}, crc=0x{:x} done -- {}", + t, laddr, prev_addr, prev_len, addr, len, checksum, result); return update_mapping_iertr::make_ready_future< extent_ref_count_t>(result.refcount); }, @@ -633,6 +635,51 @@ BtreeLBAManager::update_mapping( ); } +BtreeLBAManager::update_mappings_ret +BtreeLBAManager::update_mappings( + Transaction& t, + const std::list& extents) +{ + return trans_intr::do_for_each(extents, [this, &t](auto &extent) { + LOG_PREFIX(BtreeLBAManager::update_mappings); + auto laddr = extent->get_laddr(); + auto prev_addr = extent->get_prior_paddr_and_reset(); + auto len = extent->get_length(); + auto addr = extent->get_paddr(); + auto checksum = extent->get_last_committed_crc(); + TRACET("laddr={}, paddr {}~0x{:x} => {}, crc=0x{:x}", + t, laddr, prev_addr, len, addr, checksum); + assert(!addr.is_null()); + return _update_mapping( + t, + laddr, + [prev_addr, addr, len, checksum]( + const lba_map_val_t &in) { + lba_map_val_t ret = in; + ceph_assert(in.pladdr.is_paddr()); + ceph_assert(in.pladdr.get_paddr() == prev_addr); + ceph_assert(in.len == len); + ret.pladdr = addr; + ret.checksum = checksum; + return ret; + }, + nullptr // all the extents should have already been + // added to the fixed_kv_btree + ).si_then([&t, laddr, prev_addr, len, addr, checksum, FNAME](auto res) { + auto &result = res.map_value; + DEBUGT("laddr={}, paddr {}~0x{:x} => {}, crc=0x{:x} done -- {}", + t, laddr, prev_addr, len, addr, checksum, result); + return update_mapping_iertr::make_ready_future(); + }, + update_mapping_iertr::pass_further{}, + /* ENOENT in particular should be impossible */ + crimson::ct_error::assert_all{ + "Invalid error in BtreeLBAManager::update_mappings" + } + ); + }); +} + BtreeLBAManager::get_physical_extent_if_live_ret BtreeLBAManager::get_physical_extent_if_live( Transaction &t, 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 39cd0d995dbc..d9e1247113b0 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 @@ -580,10 +580,11 @@ public: laddr_t laddr, extent_len_t prev_len, paddr_t prev_addr, - extent_len_t len, - paddr_t paddr, - uint32_t checksum, - LogicalChildNode*) final; + LogicalChildNode&) final; + + update_mappings_ret update_mappings( + Transaction& t, + const std::list& extents); get_physical_extent_if_live_ret get_physical_extent_if_live( Transaction &t, @@ -594,7 +595,6 @@ public: private: Cache &cache; - struct { uint64_t num_alloc_extents = 0; uint64_t num_alloc_extents_iter_nexts = 0; diff --git a/src/crimson/os/seastore/transaction_manager.cc b/src/crimson/os/seastore/transaction_manager.cc index c979ec1b58cc..dd77577315e9 100644 --- a/src/crimson/os/seastore/transaction_manager.cc +++ b/src/crimson/os/seastore/transaction_manager.cc @@ -526,10 +526,7 @@ TransactionManager::rewrite_logical_extent( extent->get_laddr(), extent->get_length(), extent->get_paddr(), - nextent->get_length(), - nextent->get_paddr(), - nextent->get_last_committed_crc(), - nextent.get() + *nextent ).discard_result(); } else { assert(get_extent_category(extent->get_type()) == data_category_t::DATA); @@ -570,15 +567,13 @@ TransactionManager::rewrite_logical_extent( * avoid this complication. */ auto fut = base_iertr::now(); if (first_extent) { + assert(off == 0); fut = lba_manager->update_mapping( t, - (extent->get_laddr() + off).checked_to_laddr(), + extent->get_laddr(), extent->get_length(), extent->get_paddr(), - nextent->get_length(), - nextent->get_paddr(), - nextent->get_last_committed_crc(), - nextent.get() + *nextent ).si_then([&refcount](auto c) { refcount = c; });