crimson/.../transaction_manager: wait_io in get_extent_if_cached

Exposing wait_io to TransactionManager directly would complicate
any future users.

Signed-off-by: Samuel Just <sjust@redhat.com>

diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h
index 10263ff8d00efb290e8b992ca2ed9e1ac8f38b8b..5f248c69448cfa6c206b0316672e0b88850a0d8d 100644 (file)
--- a/src/crimson/os/seastore/cache.h
+++ b/src/crimson/os/seastore/cache.h
@@ -182,21 +182,19 @@ public:
    *
    * Returns extent at offset if in cache
    */
-  Transaction::get_extent_ret get_extent_if_cached(
+  seastar::future<CachedExtentRef> get_extent_if_cached(
     Transaction &t,
-    paddr_t offset,
-    CachedExtentRef *out) {
-    auto result = t.get_extent(offset, out);
-    if (result != Transaction::get_extent_ret::ABSENT) {
-      return result;
-    } else if (auto iter = extents.find_offset(offset);
-              iter != extents.end()) {
-      if (out)
-       *out = &*iter;
-      return Transaction::get_extent_ret::PRESENT;
-    } else {
-      return Transaction::get_extent_ret::ABSENT;
-    }
+    paddr_t offset) {
+    return seastar::do_with(
+      CachedExtentRef(),
+      [this, &t, offset](auto &ret) {
+       auto status = query_cache_for_extent(t, offset, &ret);
+       auto wait = seastar::now();
+       if (status == Transaction::get_extent_ret::PRESENT) {
+         wait = ret->wait_io();
+       }
+       return wait.then([ret] { return std::move(ret); });
+      });
   }
 
   /**
@@ -251,7 +249,7 @@ public:
     laddr_t laddr,
     segment_off_t length) {
     CachedExtentRef ret;
-    auto status = get_extent_if_cached(t, offset, &ret);
+    auto status = query_cache_for_extent(t, offset, &ret);
     if (status == Transaction::get_extent_ret::RETIRED) {
       return get_extent_ertr::make_ready_future<CachedExtentRef>();
     } else if (status == Transaction::get_extent_ret::PRESENT) {
@@ -535,6 +533,24 @@ private:
 
   /// Replace prev with next
   void replace_extent(CachedExtentRef next, CachedExtentRef prev);
+
+  Transaction::get_extent_ret query_cache_for_extent(
+    Transaction &t,
+    paddr_t offset,
+    CachedExtentRef *out) {
+    auto result = t.get_extent(offset, out);
+    if (result != Transaction::get_extent_ret::ABSENT) {
+      return result;
+    } else if (auto iter = extents.find_offset(offset);
+              iter != extents.end()) {
+      if (out)
+       *out = &*iter;
+      return Transaction::get_extent_ret::PRESENT;
+    } else {
+      return Transaction::get_extent_ret::ABSENT;
+    }
+  }
+
 };
 using CacheRef = std::unique_ptr<Cache>;
 

diff --git a/src/crimson/os/seastore/cached_extent.h b/src/crimson/os/seastore/cached_extent.h
index 2cab4325e46a60c526e96195e03c846eec7f779b..c3df9554283e68fd4dd2c209eebfdc2e52491635 100644 (file)
--- a/src/crimson/os/seastore/cached_extent.h
+++ b/src/crimson/os/seastore/cached_extent.h
@@ -337,6 +337,7 @@ private:
     io_wait_promise->set_value();
     io_wait_promise = std::nullopt;
   }
+
   seastar::future<> wait_io() {
     if (!io_wait_promise) {
       return seastar::now();

diff --git a/src/crimson/os/seastore/transaction_manager.cc b/src/crimson/os/seastore/transaction_manager.cc
index 15c1af7c94792b3302afe7b34bb498765ed66c6e..62abb76efcc7af97eb74b69bd88590476bc8f004 100644 (file)
--- a/src/crimson/os/seastore/transaction_manager.cc
+++ b/src/crimson/os/seastore/transaction_manager.cc
@@ -297,69 +297,73 @@ TransactionManager::get_extent_if_live_ret TransactionManager::get_extent_if_liv
     addr,
     laddr,
     len);
-  CachedExtentRef ret;
-  auto status = cache->get_extent_if_cached(t, addr, &ret);
-  if (status != Transaction::get_extent_ret::ABSENT) {
-    return get_extent_if_live_ret(
-      get_extent_if_live_ertr::ready_future_marker{},
-      ret);
-  }
 
-  if (is_logical_type(type)) {
-    return lba_manager->get_mapping(
-      t,
-      laddr,
-      len).safe_then([=, &t](lba_pin_list_t pins) {
-       ceph_assert(pins.size() <= 1);
-       if (pins.empty()) {
-         return get_extent_if_live_ret(
-           get_extent_if_live_ertr::ready_future_marker{},
-           CachedExtentRef());
-       }
+  return cache->get_extent_if_cached(t, addr
+  ).then([this, &t, type, addr, laddr, len](auto extent)
+        -> get_extent_if_live_ret {
+    if (extent) {
+      return get_extent_if_live_ret(
+       get_extent_if_live_ertr::ready_future_marker{},
+       extent);
+    }
+
+    if (is_logical_type(type)) {
+      return lba_manager->get_mapping(
+       t,
+       laddr,
+       len).safe_then([=, &t](lba_pin_list_t pins) {
+         ceph_assert(pins.size() <= 1);
+         if (pins.empty()) {
+           return get_extent_if_live_ret(
+             get_extent_if_live_ertr::ready_future_marker{},
+             CachedExtentRef());
+         }
 
-       auto pin = std::move(pins.front());
-       pins.pop_front();
-       ceph_assert(pin->get_laddr() == laddr);
-       ceph_assert(pin->get_length() == (extent_len_t)len);
-       if (pin->get_paddr() == addr) {
-         return cache->get_extent_by_type(
-           t,
-           type,
-           addr,
-           laddr,
-           len).safe_then(
-             [this, pin=std::move(pin)](CachedExtentRef ret) mutable
-             -> get_extent_if_live_ret {
-               auto lref = ret->cast<LogicalCachedExtent>();
-               if (!lref->has_pin()) {
-                 if (pin->has_been_invalidated() || lref->has_been_invalidated()) {
-                   return crimson::ct_error::eagain::make();
-                 } else {
-                   lref->set_pin(std::move(pin));
-                   lba_manager->add_pin(lref->get_pin());
+         auto pin = std::move(pins.front());
+         pins.pop_front();
+         ceph_assert(pin->get_laddr() == laddr);
+         ceph_assert(pin->get_length() == (extent_len_t)len);
+         if (pin->get_paddr() == addr) {
+           return cache->get_extent_by_type(
+             t,
+             type,
+             addr,
+             laddr,
+             len).safe_then(
+               [this, pin=std::move(pin)](CachedExtentRef ret) mutable
+               -> get_extent_if_live_ret {
+                 auto lref = ret->cast<LogicalCachedExtent>();
+                 if (!lref->has_pin()) {
+                   if (pin->has_been_invalidated() ||
+                       lref->has_been_invalidated()) {
+                     return crimson::ct_error::eagain::make();
+                   } else {
+                     lref->set_pin(std::move(pin));
+                     lba_manager->add_pin(lref->get_pin());
+                   }
                  }
-               }
-               return get_extent_if_live_ret(
-                 get_extent_if_live_ertr::ready_future_marker{},
-                 ret);
-             });
-       } else {
-         return get_extent_if_live_ret(
-           get_extent_if_live_ertr::ready_future_marker{},
-           CachedExtentRef());
-       }
-      });
-  } else {
-    logger().debug(
-      "TransactionManager::get_extent_if_live: non-logical extent {}",
-      addr);
-    return lba_manager->get_physical_extent_if_live(
-      t,
-      type,
-      addr,
-      laddr,
-      len);
-  }
+                 return get_extent_if_live_ret(
+                   get_extent_if_live_ertr::ready_future_marker{},
+                   ret);
+               });
+         } else {
+           return get_extent_if_live_ret(
+             get_extent_if_live_ertr::ready_future_marker{},
+             CachedExtentRef());
+         }
+       });
+    } else {
+      logger().debug(
+       "TransactionManager::get_extent_if_live: non-logical extent {}",
+       addr);
+      return lba_manager->get_physical_extent_if_live(
+       t,
+       type,
+       addr,
+       laddr,
+       len);
+    }
+  });
 }
 
 TransactionManager::~TransactionManager() {}