From: Yingxin Cheng <yingxin.cheng@intel.com>
Date: Wed, 23 Oct 2024 08:40:26 +0000 (+0800)
Subject: crimson/os/seastore: misc cleanups and asserts
X-Git-Tag: v20.0.0~619^2~15
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=48d6d1d1fa0810e0227f487ed7be0b1b21fd711f;p=ceph.git

crimson/os/seastore: misc cleanups and asserts

Signed-off-by: Yingxin Cheng <yingxin.cheng@intel.com>
---

diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h
index e93e0f91a9b..6303f50b3cc 100644
--- a/src/crimson/os/seastore/cache.h
+++ b/src/crimson/os/seastore/cache.h
@@ -400,6 +400,7 @@ public:
                 t, T::TYPE, offset, length, *ret);
       ceph_abort("impossible");
     } else if (result == Transaction::get_extent_ret::PRESENT) {
+      assert(ret->get_length() == length);
       if (ret->is_fully_loaded()) {
         SUBTRACET(seastore_cache, "{} {}~{} is present on t -- {}",
                   t, T::TYPE, offset, length, *ret);
@@ -584,6 +585,7 @@ public:
     // user should not see RETIRED_PLACEHOLDER extents
     ceph_assert(!is_retired_placeholder_type(p_extent->get_type()));
     if (!p_extent->is_fully_loaded()) {
+      assert(is_logical_type(p_extent->get_type()));
       assert(!p_extent->is_mutable());
       ++access_stats.load_present;
       ++stats.access.s.load_present;
@@ -689,30 +691,26 @@ private:
       extent_init_func(*ret);
       return read_extent<T>(
 	std::move(ret));
-    } else if (!cached->is_fully_loaded()) {
-      auto ret = TCachedExtentRef<T>(static_cast<T*>(cached.get()));
-      on_cache(*ret);
+    }
+
+    auto ret = TCachedExtentRef<T>(static_cast<T*>(cached.get()));
+    on_cache(*ret);
+    if (ret->is_fully_loaded()) {
+      SUBTRACE(seastore_cache,
+          "{} {}~{} is present in cache -- {}",
+          T::TYPE, offset, length, *ret);
+      return ret->wait_io().then([ret] {
+        // ret may be invalid, caller must check
+        return seastar::make_ready_future<TCachedExtentRef<T>>(ret);
+      });
+    } else {
       SUBDEBUG(seastore_cache,
-        "{} {}~{} is present without been fully loaded, reading ... -- {}",
-        T::TYPE, offset, length, *ret);
+          "{} {}~{} is present without been fully loaded, reading ... -- {}",
+          T::TYPE, offset, length, *ret);
       auto bp = create_extent_ptr_rand(length);
       ret->set_bptr(std::move(bp));
       return read_extent<T>(
         std::move(ret));
-    } else {
-      SUBTRACE(seastore_cache,
-          "{} {}~{} is present in cache -- {}",
-          T::TYPE, offset, length, *cached);
-      auto ret = TCachedExtentRef<T>(static_cast<T*>(cached.get()));
-      on_cache(*ret);
-      return ret->wait_io(
-      ).then([ret=std::move(ret)]() mutable
-	     -> read_extent_ret<T> {
-        // ret may be invalid, caller must check
-        return read_extent_ret<T>(
-          get_extent_ertr::ready_future_marker{},
-          std::move(ret));
-      });
     }
   }
 
@@ -781,6 +779,7 @@ private:
                 t, type, offset, length, laddr, *ret);
       ceph_abort("impossible");
     } else if (status == Transaction::get_extent_ret::PRESENT) {
+      assert(ret->get_length() == length);
       if (ret->is_fully_loaded()) {
         SUBTRACET(seastore_cache, "{} {}~{} {} is present on t -- {}",
                   t, type, offset, length, laddr, *ret);
diff --git a/src/crimson/os/seastore/transaction_manager.h b/src/crimson/os/seastore/transaction_manager.h
index 841c5638abc..5cba9c4ab38 100644
--- a/src/crimson/os/seastore/transaction_manager.h
+++ b/src/crimson/os/seastore/transaction_manager.h
@@ -214,7 +214,7 @@ public:
       pin->maybe_fix_pos();
       fut = base_iertr::make_ready_future<LBAMappingRef>(std::move(pin));
     }
-    return fut.si_then([&t, this](auto npin) mutable {
+    return fut.si_then([&t, this](auto npin) {
       // checking the lba child must be atomic with creating
       // and linking the absent child
       auto ret = get_extent_if_linked<T>(t, std::move(npin));