crimson/.../btree_lba_manager: simplify _update_mapping_ret, remove update_mapping_re...

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

diff --git a/src/crimson/os/seastore/lba/btree_lba_manager.cc b/src/crimson/os/seastore/lba/btree_lba_manager.cc
index 59b35ed6b50c72c89fc43beab5374c7d2c3d067a..dd8687f493831c16e39d38f40ee00ba132feb8d4 100644 (file)
--- a/src/crimson/os/seastore/lba/btree_lba_manager.cc
+++ b/src/crimson/os/seastore/lba/btree_lba_manager.cc
@@ -731,10 +731,9 @@ BtreeLBAManager::update_mapping(
       "Invalid error in BtreeLBAManager::update_mapping"
     }
   );
-  assert(res.is_alive_mapping());
   DEBUGT("laddr={}, paddr {}~0x{:x} => {}~0x{:x}, crc=0x{:x} done -- {}",
-        t, laddr, prev_addr, prev_len, addr, len, checksum, res.get_cursor());
-  co_return res.get_cursor().get_refcount();
+        t, laddr, prev_addr, prev_len, addr, len, checksum, *cursor);
+  co_return res->get_refcount();
 }
 
 BtreeLBAManager::update_mappings_ret
@@ -788,7 +787,7 @@ BtreeLBAManager::update_mappings(
                    checksum, FNAME](auto res) {
              DEBUGT("cursor={}, paddr {}~0x{:x} => {}, crc=0x{:x} done -- {}",
                     c.trans, *cursor, prev_addr, len,
-                    addr, checksum, res.get_cursor());
+                    addr, checksum, *res);
              return update_mapping_iertr::make_ready_future();
            },
            update_mapping_iertr::pass_further{},
@@ -865,14 +864,12 @@ BtreeLBAManager::_update_mapping(
   auto btree = co_await get_btree<LBABtree>(cache, c);
   auto iter = btree.make_partial_iter(c, cursor);
   auto ret = f(iter.get_val());
-  auto laddr = cursor.key;
   if (ret.refcount == 0) {
     iter = co_await btree.remove(
       c,
       iter
     );
-    co_return update_mapping_ret_bare_t{
-      laddr, std::move(ret), iter.get_cursor(c)};
+    co_return iter.get_cursor(c);
   } else {
     iter = co_await btree.update(
       c,
@@ -888,7 +885,7 @@ BtreeLBAManager::_update_mapping(
            && nextent->peek_parent_node().get() == iter.get_leaf_node().get()));
     LBACursorRef cursor = iter.get_cursor(c);
     assert(cursor->val);
-    co_return update_mapping_ret_bare_t{std::move(cursor)};
+    co_return cursor;
   }
 }
 

diff --git a/src/crimson/os/seastore/lba/btree_lba_manager.h b/src/crimson/os/seastore/lba/btree_lba_manager.h
index 87b60e06d315aa3f45b0f2b7f55b0ad0d65d0f55..39c288e2f7c7e605fc416707f8df1674c993c6e8 100644 (file)
--- a/src/crimson/os/seastore/lba/btree_lba_manager.h
+++ b/src/crimson/os/seastore/lba/btree_lba_manager.h
@@ -181,7 +181,7 @@ public:
     Transaction &t,
     LBACursorRef cursor,
     int delta) final {
-    co_return (co_await _update_mapping(
+    co_return co_await _update_mapping(
       t,
       *cursor,
       [delta](lba_map_val_t ret) {
@@ -193,7 +193,7 @@ public:
     ).handle_error_interruptible(
       base_iertr::pass_further{},
       crimson::ct_error::assert_all{}
-    )).take_cursor();
+    );
   }
 
   remap_ret remap_mappings(
@@ -309,89 +309,12 @@ private:
   seastar::metrics::metric_group metrics;
   void register_metrics();
 
-  struct update_mapping_ret_bare_t {
-    update_mapping_ret_bare_t()
-       : update_mapping_ret_bare_t(LBACursorRef(nullptr)) {}
-
-    update_mapping_ret_bare_t(LBACursorRef cursor)
-       : ret(std::move(cursor)) {}
-
-    update_mapping_ret_bare_t(
-      laddr_t laddr, lba_map_val_t value, LBACursorRef &&cursor)
-       : ret(removed_mapping_t{laddr, value, std::move(cursor)}) {}
-
-    struct removed_mapping_t {
-      laddr_t laddr;
-      lba_map_val_t map_value;
-      LBACursorRef next;
-    };
-    std::variant<removed_mapping_t, LBACursorRef> ret;
-
-    bool is_removed_mapping() const {
-      return ret.index() == 0;
-    }
-
-    bool is_alive_mapping() const {
-      if (ret.index() == 1) {
-       assert(std::get<1>(ret));
-       return true;
-      } else {
-       return false;
-      }
-    }
-
-    removed_mapping_t &get_removed_mapping() {
-      assert(is_removed_mapping());
-      return std::get<0>(ret);
-    }
-
-    const removed_mapping_t& get_removed_mapping() const {
-      assert(is_removed_mapping());
-      return std::get<0>(ret);
-    }
-
-    const LBACursor& get_cursor() const {
-      assert(is_alive_mapping());
-      return *std::get<1>(ret);
-    }
-
-    LBACursorRef take_cursor() {
-      assert(is_alive_mapping());
-      return std::move(std::get<1>(ret));
-    }
-  };
-
-  mapping_update_result_t get_mapping_update_result(
-    update_mapping_ret_bare_t &result) {
-    if (result.is_removed_mapping()) {
-      auto &v = result.get_removed_mapping();
-      auto &val = v.map_value;
-      return {v.laddr,
-             val.refcount,
-             val.pladdr,
-             val.len,
-             v.next->is_indirect()
-               ? LBAMapping::create_indirect(nullptr, std::move(v.next))
-               : LBAMapping::create_direct(std::move(v.next))};
-    } else {
-      assert(result.is_alive_mapping());
-      auto &c = result.get_cursor();
-      assert(c.val);
-      ceph_assert(!c.is_indirect());
-      return {c.get_laddr(), c.val->refcount, 
-       c.val->pladdr, c.val->len,
-       LBAMapping::create_direct(result.take_cursor())};
-    }
-  }
-
   /**
    * _update_mapping
    *
    * Updates mapping, removes if f returns nullopt
    */
-  using _update_mapping_iertr = ref_iertr;
-  using _update_mapping_ret = ref_iertr::future<
-    update_mapping_ret_bare_t>;
+  using _update_mapping_ret = ref_iertr::future<LBACursorRef>;
   using update_func_t = std::function<
     lba_map_val_t(const lba_map_val_t &v)
     >;

diff --git a/src/crimson/os/seastore/lba_manager.h b/src/crimson/os/seastore/lba_manager.h
index c1543d4bcf047425998a6c2ec9e9070edc04030c..1de8a4a3598e0ede8c15bf97c34e78e1b94c2fd7 100644 (file)
--- a/src/crimson/os/seastore/lba_manager.h
+++ b/src/crimson/os/seastore/lba_manager.h
@@ -126,18 +126,6 @@ public:
     laddr_t hint,
     extent_len_t len) = 0;
 
-  struct mapping_update_result_t {
-    laddr_t key;
-    extent_ref_count_t refcount = 0;
-    pladdr_t addr;
-    extent_len_t length = 0;
-    LBAMapping mapping; // the mapping pointing to the updated lba entry if
-                       // refcount is non-zero; the next lba entry or the
-                       // end mapping otherwise.
-    bool need_to_remove_extent() const {
-      return refcount == 0 && addr.is_paddr() && !addr.get_paddr().is_zero();
-    }
-  };
   using ref_iertr = base_iertr::extend<
     crimson::ct_error::enoent>;