crimson/os/seastore: drop unused inc/decref_extent and dec_ref with tests

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

diff --git a/src/crimson/os/seastore/lba_manager.h b/src/crimson/os/seastore/lba_manager.h
index 2d347e504c93fe38b86e5c365ddd96324c1e7009..26a59f82e92bdb18827269ac1c2e524caef68b04 100644 (file)
--- a/src/crimson/os/seastore/lba_manager.h
+++ b/src/crimson/os/seastore/lba_manager.h
@@ -120,20 +120,11 @@ public:
   using ref_ret = ref_iertr::future<ref_update_result_t>;
 
   /**
-   * Decrements ref count on extent
+   * Removes a mapping and deal with indirection
    *
    * @return returns resulting refcount
    */
-  virtual ref_ret decref_extent(
-    Transaction &t,
-    laddr_t addr) = 0;
-
-  /**
-   * Increments ref count on extent
-   *
-   * @return returns resulting refcount
-   */
-  virtual ref_ret incref_extent(
+  virtual ref_ret remove_mapping(
     Transaction &t,
     laddr_t addr) = 0;
 

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 50bf2ca5e0cafa642c7c2bfa6cae0ce2861d09a8..cdc62a5de33cf8b53cad8a21cc3cb4671d31c601 100644 (file)
--- a/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.h
+++ b/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.h
@@ -430,7 +430,7 @@ public:
     });
   }
 
-  ref_ret decref_extent(
+  ref_ret remove_mapping(
     Transaction &t,
     laddr_t addr) final {
     return update_refcount(t, addr, -1, true
@@ -439,15 +439,6 @@ public:
     });
   }
 
-  ref_ret incref_extent(
-    Transaction &t,
-    laddr_t addr) final {
-    return update_refcount(t, addr, 1, false
-    ).si_then([](auto res) {
-      return std::move(res.ref_update_res);
-    });
-  }
-
   remap_ret remap_mappings(
     Transaction &t,
     LBAMappingRef orig_mapping,

diff --git a/src/crimson/os/seastore/transaction_manager.cc b/src/crimson/os/seastore/transaction_manager.cc
index e8f4801b42ec69ef4af93d6ac0dd7eca4d0ffd9a..45c5610658b86469913a5a9face5c31abd026980 100644 (file)
--- a/src/crimson/os/seastore/transaction_manager.cc
+++ b/src/crimson/os/seastore/transaction_manager.cc
@@ -198,45 +198,13 @@ TransactionManager::close() {
   });
 }
 
-#ifdef UNIT_TESTS_BUILT
-TransactionManager::ref_ret TransactionManager::inc_ref(
-  Transaction &t,
-  LogicalChildNodeRef &ref)
-{
-  LOG_PREFIX(TransactionManager::inc_ref);
-  TRACET("{}", t, *ref);
-  return lba_manager->incref_extent(t, ref->get_laddr()
-  ).si_then([FNAME, ref, &t](auto result) {
-    DEBUGT("extent refcount is incremented to {} -- {}",
-           t, result.refcount, *ref);
-    return result.refcount;
-  }).handle_error_interruptible(
-    ref_iertr::pass_further{},
-    ct_error::assert_all{"unhandled error, TODO"});
-}
-
-TransactionManager::ref_ret TransactionManager::inc_ref(
-  Transaction &t,
-  laddr_t offset)
-{
-  LOG_PREFIX(TransactionManager::inc_ref);
-  TRACET("{}", t, offset);
-  return lba_manager->incref_extent(t, offset
-  ).si_then([FNAME, offset, &t](auto result) {
-    DEBUGT("extent refcount is incremented to {} -- {}~0x{:x}, {}",
-           t, result.refcount, offset, result.length, result.addr);
-    return result.refcount;
-  });
-}
-#endif
-
 TransactionManager::ref_ret TransactionManager::remove(
   Transaction &t,
   LogicalChildNodeRef &ref)
 {
   LOG_PREFIX(TransactionManager::remove);
   DEBUGT("{} ...", t, *ref);
-  return lba_manager->decref_extent(t, ref->get_laddr()
+  return lba_manager->remove_mapping(t, ref->get_laddr()
   ).si_then([this, FNAME, &t, ref](auto result) {
     if (result.refcount == 0) {
       cache->retire_extent(t, ref);
@@ -253,7 +221,7 @@ TransactionManager::ref_ret TransactionManager::remove(
 {
   LOG_PREFIX(TransactionManager::remove);
   DEBUGT("{} ...", t, offset);
-  return lba_manager->decref_extent(t, offset
+  return lba_manager->remove_mapping(t, offset
   ).si_then([this, FNAME, offset, &t](auto result) -> ref_ret {
     auto fut = ref_iertr::now();
     if (result.refcount == 0) {

diff --git a/src/crimson/os/seastore/transaction_manager.h b/src/crimson/os/seastore/transaction_manager.h
index fc364b9a2fbfd175bfdc369bb0df5854328ddfbd..fc7137dfe51869371389472845750304bec6afb5 100644 (file)
--- a/src/crimson/os/seastore/transaction_manager.h
+++ b/src/crimson/os/seastore/transaction_manager.h
@@ -339,18 +339,6 @@ public:
   using ref_iertr = LBAManager::ref_iertr;
   using ref_ret = ref_iertr::future<extent_ref_count_t>;
 
-#ifdef UNIT_TESTS_BUILT
-  /// Add refcount for ref
-  ref_ret inc_ref(
-    Transaction &t,
-    LogicalChildNodeRef &ref);
-
-  /// Add refcount for offset
-  ref_ret inc_ref(
-    Transaction &t,
-    laddr_t offset);
-#endif
-
   /** 
    * remove
    *

diff --git a/src/test/crimson/seastore/test_btree_lba_manager.cc b/src/test/crimson/seastore/test_btree_lba_manager.cc
index 2ec8b7b01620abd685d1e7784e6bb7c0b6e79224..68293262e4e8c238a4a1f75f53538955ddd32495 100644 (file)
--- a/src/test/crimson/seastore/test_btree_lba_manager.cc
+++ b/src/test/crimson/seastore/test_btree_lba_manager.cc
@@ -594,7 +594,7 @@ struct btree_lba_manager_test : btree_test_base {
     (void) with_trans_intr(
       *t.t,
       [=, this](auto &t) {
-       return lba_manager->decref_extent(
+       return lba_manager->remove_mapping(
          t,
          target->first
        ).si_then([this, &t, target](auto result) {
@@ -611,27 +611,6 @@ struct btree_lba_manager_test : btree_test_base {
     }
   }
 
-  auto incref_mapping(
-    test_transaction_t &t,
-    laddr_t addr) {
-    return incref_mapping(t, t.mappings.find(addr));
-  }
-
-  void incref_mapping(
-    test_transaction_t &t,
-    test_lba_mapping_t::iterator target) {
-    ceph_assert(target->second.refcount > 0);
-    target->second.refcount++;
-    auto refcnt = with_trans_intr(
-      *t.t,
-      [=, this](auto &t) {
-       return lba_manager->incref_extent(
-         t,
-         target->first);
-      }).unsafe_get().refcount;
-    EXPECT_EQ(refcnt, target->second.refcount);
-  }
-
   std::vector<laddr_t> get_mapped_addresses() {
     std::vector<laddr_t> addresses;
     addresses.reserve(test_lba_mappings.size());
@@ -754,10 +733,6 @@ TEST_F(btree_lba_manager_test, force_split_merge)
          check_mappings(t);
          check_mappings();
        }
-       for (auto &ret : rets) {
-         incref_mapping(t, ret->get_key());
-         decref_mapping(t, ret->get_key());
-       }
       }
       logger().debug("submitting transaction");
       submit_test_transaction(std::move(t));
@@ -770,8 +745,6 @@ TEST_F(btree_lba_manager_test, force_split_merge)
       auto t = create_transaction();
       for (unsigned i = 0; i != addresses.size(); ++i) {
        if (i % 2 == 0) {
-         incref_mapping(t, addresses[i]);
-         decref_mapping(t, addresses[i]);
          decref_mapping(t, addresses[i]);
        }
        logger().debug("submitting transaction");
@@ -790,8 +763,6 @@ TEST_F(btree_lba_manager_test, force_split_merge)
       auto addresses = get_mapped_addresses();
       auto t = create_transaction();
       for (unsigned i = 0; i != addresses.size(); ++i) {
-       incref_mapping(t, addresses[i]);
-       decref_mapping(t, addresses[i]);
        decref_mapping(t, addresses[i]);
       }
       check_mappings(t);

diff --git a/src/test/crimson/seastore/test_transaction_manager.cc b/src/test/crimson/seastore/test_transaction_manager.cc
index 6e0fe65c3457bff34a2cc39cc6fed018ece4f3f5..09cec738900ff73d4ec0901792a973cb4acab128 100644 (file)
--- a/src/test/crimson/seastore/test_transaction_manager.cc
+++ b/src/test/crimson/seastore/test_transaction_manager.cc
@@ -687,18 +687,7 @@ struct transaction_manager_test_t :
     return pin;
   }
 
-  void inc_ref(test_transaction_t &t, laddr_t offset) {
-    ceph_assert(test_mappings.contains(offset, t.mapping_delta));
-    ceph_assert(test_mappings.get(offset, t.mapping_delta).refcount > 0);
-
-    auto refcnt = with_trans_intr(*(t.t), [&](auto& trans) {
-      return tm->inc_ref(trans, offset);
-    }).unsafe_get();
-    auto check_refcnt = test_mappings.inc_ref(offset, t.mapping_delta);
-    EXPECT_EQ(refcnt, check_refcnt);
-  }
-
-  void dec_ref(test_transaction_t &t, laddr_t offset) {
+  void remove(test_transaction_t &t, laddr_t offset) {
     ceph_assert(test_mappings.contains(offset, t.mapping_delta));
     ceph_assert(test_mappings.get(offset, t.mapping_delta).refcount > 0);
 
@@ -1964,7 +1953,7 @@ TEST_P(tm_single_device_test_t, create_remove_same_transaction)
        'a');
       ASSERT_EQ(ADDR, extent->get_laddr());
       check_mappings(t);
-      dec_ref(t, ADDR);
+      remove(t, ADDR);
       check_mappings(t);
 
       extent = alloc_extent(
@@ -2000,7 +1989,7 @@ TEST_P(tm_single_device_test_t, split_merge_read_same_transaction)
     {
       auto t = create_transaction();
       for (unsigned i = 0; i < 240; ++i) {
-       dec_ref(
+       remove(
          t,
          get_laddr_hint(i * SIZE));
       }
@@ -2011,53 +2000,6 @@ TEST_P(tm_single_device_test_t, split_merge_read_same_transaction)
   });
 }
 
-TEST_P(tm_single_device_test_t, inc_dec_ref)
-{
-  constexpr size_t SIZE = 4096;
-  run_async([this] {
-    laddr_t ADDR = get_laddr_hint(0xFF * SIZE);
-    {
-      auto t = create_transaction();
-      auto extent = alloc_extent(
-       t,
-       ADDR,
-       SIZE,
-       'a');
-      ASSERT_EQ(ADDR, extent->get_laddr());
-      check_mappings(t);
-      check();
-      submit_transaction(std::move(t));
-      check();
-    }
-    replay();
-    {
-      auto t = create_transaction();
-      inc_ref(t, ADDR);
-      check_mappings(t);
-      check();
-      submit_transaction(std::move(t));
-      check();
-    }
-    {
-      auto t = create_transaction();
-      dec_ref(t, ADDR);
-      check_mappings(t);
-      check();
-      submit_transaction(std::move(t));
-      check();
-    }
-    replay();
-    {
-      auto t = create_transaction();
-      dec_ref(t, ADDR);
-      check_mappings(t);
-      check();
-      submit_transaction(std::move(t));
-      check();
-    }
-  });
-}
-
 TEST_P(tm_single_device_test_t, cause_lba_split)
 {
   constexpr size_t SIZE = 4096;
@@ -2108,7 +2050,7 @@ TEST_P(tm_single_device_test_t, random_writes)
            get_laddr_hint(TOTAL + (k * PADDING_SIZE)),
            PADDING_SIZE);
          for (auto &padding : paddings) {
-           dec_ref(t, padding->get_laddr());
+           remove(t, padding->get_laddr());
          }
        }
        submit_transaction(std::move(t));