crimson/os/seastore: simplify the processing of indirect LBAMapping

Signed-off-by: Zhang Song <zhangsong02@qianxin.com>

diff --git a/src/crimson/os/seastore/cached_extent.cc b/src/crimson/os/seastore/cached_extent.cc
index 23e221392df2d5a09d29a281c7d18776fc5236ba..44127df625a0334e7763220baac9dab562c38b8a 100644 (file)
--- a/src/crimson/os/seastore/cached_extent.cc
+++ b/src/crimson/os/seastore/cached_extent.cc
@@ -104,12 +104,6 @@ void CachedExtent::set_invalid(Transaction &t) {
   on_invalidated(t);
 }
 
-void LogicalCachedExtent::maybe_set_intermediate_laddr(LBAMapping &mapping) {
-  laddr = mapping.is_indirect()
-    ? mapping.get_intermediate_base()
-    : mapping.get_key();
-}
-
 std::pair<bool, CachedExtent::viewable_state_t>
 CachedExtent::is_viewable_by_trans(Transaction &t) {
   if (!is_valid()) {

diff --git a/src/crimson/os/seastore/cached_extent.h b/src/crimson/os/seastore/cached_extent.h
index 02730f9ad625d1befe2b13a189e7d8efaf8e04f7..dd34c8fbdabf69d98796c67e309b9d93042c5af1 100644 (file)
--- a/src/crimson/os/seastore/cached_extent.h
+++ b/src/crimson/os/seastore/cached_extent.h
@@ -1415,8 +1415,6 @@ public:
     laddr = nladdr;
   }
 
-  void maybe_set_intermediate_laddr(LBAMapping &mapping);
-
   void apply_delta_and_adjust_crc(
     paddr_t base, const ceph::bufferlist &bl) final {
     apply_delta(bl);

diff --git a/src/crimson/os/seastore/transaction_manager.h b/src/crimson/os/seastore/transaction_manager.h
index 348f27b8366dd683f74f6f3773eb8856eb3b4f11..c506bbf1c5ff9c4806fe4be8f3f0da593884b393 100644 (file)
--- a/src/crimson/os/seastore/transaction_manager.h
+++ b/src/crimson/os/seastore/transaction_manager.h
@@ -972,10 +972,7 @@ private:
       ).si_then([pin=std::move(pin)](auto extent) {
 #ifndef NDEBUG
         auto lextent = extent->template cast<LogicalChildNode>();
-        auto pin_laddr = pin.get_key();
-        if (pin.is_indirect()) {
-          pin_laddr = pin.get_intermediate_base();
-        }
+        auto pin_laddr = pin.get_intermediate_base();
         assert(lextent->get_laddr() == pin_laddr);
 #endif
        return extent->template cast<T>();
@@ -1045,9 +1042,7 @@ private:
     // must be user-oriented required by maybe_init
     assert(is_user_transaction(t.get_src()));
     using ret = pin_to_extent_ret<T>;
-    auto direct_length = pin.is_indirect() ?
-      pin.get_intermediate_length() :
-      pin.get_length();
+    auto direct_length = pin.get_intermediate_length();
     if (full_extent_integrity_check) {
       direct_partial_off = 0;
       partial_len = direct_length;
@@ -1061,15 +1056,15 @@ private:
       direct_length,
       direct_partial_off,
       partial_len,
-      [pin=pin.duplicate(), maybe_init=std::move(maybe_init),
+      [laddr=pin.get_intermediate_base(),
+       maybe_init=std::move(maybe_init),
        child_pos=std::move(child_pos)]
       (T &extent) mutable {
        assert(extent.is_logical());
        assert(!extent.has_laddr());
        assert(!extent.has_been_invalidated());
-       assert(pin.is_valid());
        child_pos.link_child(&extent);
-       extent.maybe_set_intermediate_laddr(pin);
+       extent.set_laddr(laddr);
        maybe_init(extent);
        extent.set_seen_by_users();
       }
@@ -1126,29 +1121,21 @@ private:
               t, pin, type);
     assert(is_logical_type(type));
     assert(is_background_transaction(t.get_src()));
-    laddr_t direct_key;
-    extent_len_t direct_length;
-    if (pin.is_indirect()) {
-      direct_key = pin.get_intermediate_base();
-      direct_length = pin.get_intermediate_length();
-    } else {
-      direct_key = pin.get_key();
-      direct_length = pin.get_length();
-    }
+    laddr_t direct_key = pin.get_intermediate_base();
+    extent_len_t direct_length = pin.get_intermediate_length();
     return cache->get_absent_extent_by_type(
       t,
       type,
       pin.get_val(),
       direct_key,
       direct_length,
-      [pin=pin.duplicate(), child_pos=std::move(child_pos)](CachedExtent &extent) mutable {
+      [direct_key, child_pos=std::move(child_pos)](CachedExtent &extent) mutable {
        assert(extent.is_logical());
        auto &lextent = static_cast<LogicalChildNode&>(extent);
        assert(!lextent.has_laddr());
        assert(!lextent.has_been_invalidated());
-       assert(pin.is_valid());
        child_pos.link_child(&lextent);
-       lextent.maybe_set_intermediate_laddr(pref);
+       lextent.set_laddr(direct_key);
         // No change to extent::seen_by_user because this path is only
         // for background cleaning.
       }

diff --git a/src/test/crimson/seastore/test_transaction_manager.cc b/src/test/crimson/seastore/test_transaction_manager.cc
index ba0c5c0691360c15992297f6d0fc35b05b8297a5..334bf1ac106b1e0b12707f5d7ad536e49e8dd383 100644 (file)
--- a/src/test/crimson/seastore/test_transaction_manager.cc
+++ b/src/test/crimson/seastore/test_transaction_manager.cc
@@ -497,12 +497,8 @@ struct transaction_manager_test_t :
   TestBlockRef read_pin(
     test_transaction_t &t,
     LBAMapping pin) {
-    auto addr = pin.is_indirect()
-      ? pin.get_intermediate_base()
-      : pin.get_key();
-    auto len = pin.is_indirect()
-      ? pin.get_intermediate_length()
-      : pin.get_length();
+    auto addr = pin.get_intermediate_base();
+    auto len = pin.get_intermediate_length();
     ceph_assert(test_mappings.contains(addr, t.mapping_delta));
     ceph_assert(test_mappings.get(addr, t.mapping_delta).desc.len == len);
 
@@ -585,7 +581,7 @@ struct transaction_manager_test_t :
     using ertr = with_trans_ertr<TransactionManager::base_iertr>;
     bool indirect = pin.is_indirect();
     auto addr = pin.get_key();
-    auto im_addr = indirect ? pin.get_intermediate_base() : L_ADDR_NULL;
+    auto im_addr = pin.get_intermediate_base();
     auto ext = with_trans_intr(*(t.t), [&](auto& trans) {
       return tm->read_pin<TestBlock>(trans, std::move(pin));
     }).safe_then([](auto ret) {
@@ -1108,9 +1104,7 @@ struct transaction_manager_test_t :
     }
     auto o_laddr = opin.get_key();
     bool indirect_opin = opin.is_indirect();
-    auto data_laddr = indirect_opin
-      ? opin.get_intermediate_base()
-      : o_laddr;
+    auto data_laddr = opin.get_intermediate_base();
     auto pin = with_trans_intr(*(t.t), [&](auto& trans) {
       return tm->remap_pin<TestBlock>(
         trans, std::move(opin), std::array{