seastore: use config value and avoid passing a hint

Signed-off-by: Myoungwon Oh <myoungwon.oh@samsung.com>

diff --git a/src/crimson/os/seastore/transaction_manager.cc b/src/crimson/os/seastore/transaction_manager.cc
index 48d59c1078d0dafe878767b6537fbbc1b4dff30b..28e9b77bd83c1395fcf1db816168b28740ebb495 100644 (file)
--- a/src/crimson/os/seastore/transaction_manager.cc
+++ b/src/crimson/os/seastore/transaction_manager.cc
@@ -28,14 +28,16 @@ TransactionManager::TransactionManager(
   CacheRef _cache,
   LBAManagerRef _lba_manager,
   ExtentPlacementManagerRef &&epm,
-  BackrefManagerRef&& backref_manager)
+  BackrefManagerRef&& backref_manager,
+  tm_make_config_t config)
   : segment_cleaner(std::move(_segment_cleaner)),
     cache(std::move(_cache)),
     lba_manager(std::move(_lba_manager)),
     journal(std::move(_journal)),
     epm(std::move(epm)),
     backref_manager(std::move(backref_manager)),
-    sm_group(*segment_cleaner->get_segment_manager_group())
+    sm_group(*segment_cleaner->get_segment_manager_group()),
+    config(config)
 {
   segment_cleaner->set_extent_callback(this);
   journal->set_write_pipeline(&write_pipeline);
@@ -668,7 +670,8 @@ TransactionManagerRef make_transaction_manager(tm_make_config_t config)
     std::move(cache),
     std::move(lba_manager),
     std::move(epm),
-    std::move(backref_manager));
+    std::move(backref_manager),
+    config);
 }
 
 }

diff --git a/src/crimson/os/seastore/transaction_manager.h b/src/crimson/os/seastore/transaction_manager.h
index f0e858b9f42ce5464b8e0fdb201037506f2a9ab0..f319d368b63f1b1b6b303b7d1d4907fb6075c686 100644 (file)
--- a/src/crimson/os/seastore/transaction_manager.h
+++ b/src/crimson/os/seastore/transaction_manager.h
@@ -34,6 +34,20 @@
 namespace crimson::os::seastore {
 class Journal;
 
+struct tm_make_config_t {
+  bool detailed = true;
+  journal_type_t j_type = journal_type_t::SEGMENT_JOURNAL;
+  placement_hint_t default_placement_hint =
+    placement_hint_t::HOT;
+  static tm_make_config_t get_default() {
+    return tm_make_config_t {
+      true,
+      journal_type_t::SEGMENT_JOURNAL,
+      placement_hint_t::HOT
+    };
+  }
+};
+
 template <typename F>
 auto repeat_eagain(F &&f) {
   return seastar::do_with(
@@ -71,7 +85,8 @@ public:
     CacheRef cache,
     LBAManagerRef lba_manager,
     ExtentPlacementManagerRef &&epm,
-    BackrefManagerRef&& backref_manager);
+    BackrefManagerRef&& backref_manager,
+    tm_make_config_t config = tm_make_config_t::get_default());
 
   /// Writes initial metadata to disk
   using mkfs_ertr = base_ertr;
@@ -294,14 +309,13 @@ public:
   alloc_extent_ret<T> alloc_extent(
     Transaction &t,
     laddr_t laddr_hint,
-    extent_len_t len,
-    placement_hint_t hint = placement_hint_t::HOT) {
+    extent_len_t len) {
     placement_hint_t placement_hint;
     if constexpr (T::TYPE == extent_types_t::OBJECT_DATA_BLOCK ||
                   T::TYPE == extent_types_t::COLL_BLOCK) {
       placement_hint = placement_hint_t::COLD;
     } else {
-      placement_hint = hint;
+      placement_hint = config.default_placement_hint;
     }
     LOG_PREFIX(TransactionManager::alloc_extent);
     SUBTRACET(seastore_tm, "{} len={}, placement_hint={}, laddr_hint={}",
@@ -565,6 +579,7 @@ private:
 
   WritePipeline write_pipeline;
 
+  tm_make_config_t config;
   rewrite_extent_ret rewrite_logical_extent(
     Transaction& t,
     LogicalCachedExtentRef extent);
@@ -591,19 +606,5 @@ public:
 };
 using TransactionManagerRef = std::unique_ptr<TransactionManager>;
 
-struct tm_make_config_t {
-  bool detailed = true;
-  journal_type_t j_type = journal_type_t::SEGMENT_JOURNAL;
-  placement_hint_t default_placement_hint =
-    placement_hint_t::HOT;
-  static tm_make_config_t get_default() {
-    return tm_make_config_t {
-      true,
-      journal_type_t::SEGMENT_JOURNAL,
-      placement_hint_t::HOT
-    };
-  }
-};
-
 TransactionManagerRef make_transaction_manager(tm_make_config_t config);
 }

diff --git a/src/test/crimson/seastore/test_transaction_manager.cc b/src/test/crimson/seastore/test_transaction_manager.cc
index 649d681796f328ab30c038f1815a671285c561e3..93b30b0414fbe25c33b392cebf0f750d8da64e65 100644 (file)
--- a/src/test/crimson/seastore/test_transaction_manager.cc
+++ b/src/test/crimson/seastore/test_transaction_manager.cc
@@ -371,10 +371,9 @@ struct transaction_manager_test_t :
     test_transaction_t &t,
     laddr_t hint,
     extent_len_t len,
-    char contents,
-    placement_hint_t p_hint = placement_hint_t::HOT) {
+    char contents) {
     auto extent = with_trans_intr(*(t.t), [&](auto& trans) {
-      return tm->alloc_extent<TestBlock>(trans, hint, len, p_hint);
+      return tm->alloc_extent<TestBlock>(trans, hint, len);
     }).unsafe_get0();
     extent->set_contents(contents);
     EXPECT_FALSE(test_mappings.contains(extent->get_laddr(), t.mapping_delta));
@@ -386,14 +385,12 @@ struct transaction_manager_test_t :
   TestBlockRef alloc_extent(
     test_transaction_t &t,
     laddr_t hint,
-    extent_len_t len,
-    placement_hint_t p_hint = placement_hint_t::HOT) {
+    extent_len_t len) {
     return alloc_extent(
       t,
       hint,
       len,
-      get_random_contents(),
-      p_hint);
+      get_random_contents());
   }
 
   bool check_usage() {
@@ -623,8 +620,7 @@ struct transaction_manager_test_t :
                boost::make_counting_iterator(num),
                [&, this](auto) {
                  return tm->alloc_extent<TestBlock>(
-                   *(t.t), L_ADDR_MIN, size,
-                   tm_config.default_placement_hint
+                   *(t.t), L_ADDR_MIN, size
                  ).si_then([&, this](auto extent) {
                    extent->set_contents(get_random_contents());
                    EXPECT_FALSE(
@@ -666,8 +662,7 @@ struct transaction_manager_test_t :
               auto extent = alloc_extent(
                 t,
                 i * BSIZE,
-                BSIZE,
-               tm_config.default_placement_hint);
+                BSIZE);
               ASSERT_EQ(i * BSIZE, extent->get_laddr());
               if (try_submit_transaction(std::move(t)))
                 break;
@@ -734,8 +729,7 @@ TEST_P(tm_single_device_test_t, basic)
        t,
        ADDR,
        SIZE,
-       'a',
-       tm_config.default_placement_hint);
+       'a');
       ASSERT_EQ(ADDR, extent->get_laddr());
       check_mappings(t);
       check();
@@ -756,8 +750,7 @@ TEST_P(tm_single_device_test_t, mutate)
        t,
        ADDR,
        SIZE,
-       'a',
-       tm_config.default_placement_hint);
+       'a');
       ASSERT_EQ(ADDR, extent->get_laddr());
       check_mappings(t);
       check();
@@ -798,8 +791,7 @@ TEST_P(tm_single_device_test_t, allocate_lba_conflict)
       t,
       ADDR,
       SIZE,
-      'a',
-      tm_config.default_placement_hint);
+      'a');
     ASSERT_EQ(ADDR, extent->get_laddr());
     check_mappings(t);
     check();
@@ -808,8 +800,7 @@ TEST_P(tm_single_device_test_t, allocate_lba_conflict)
       t2,
       ADDR2,
       SIZE,
-      'a',
-      tm_config.default_placement_hint);
+      'a');
     ASSERT_EQ(ADDR2, extent2->get_laddr());
     check_mappings(t2);
     extent2.reset();
@@ -829,8 +820,7 @@ TEST_P(tm_single_device_test_t, mutate_lba_conflict)
        auto extent = alloc_extent(
          t,
          laddr_t(i * SIZE),
-         SIZE,
-         tm_config.default_placement_hint);
+         SIZE);
       }
       check_mappings(t);
       submit_transaction(std::move(t));
@@ -872,8 +862,7 @@ TEST_P(tm_single_device_test_t, concurrent_mutate_lba_no_conflict)
        auto extent = alloc_extent(
          t,
          laddr_t(i * SIZE),
-         SIZE,
-         tm_config.default_placement_hint);
+         SIZE);
       }
       submit_transaction(std::move(t));
     }
@@ -903,8 +892,7 @@ TEST_P(tm_single_device_test_t, create_remove_same_transaction)
        t,
        ADDR,
        SIZE,
-       'a',
-       tm_config.default_placement_hint);
+       'a');
       ASSERT_EQ(ADDR, extent->get_laddr());
       check_mappings(t);
       dec_ref(t, ADDR);
@@ -914,8 +902,7 @@ TEST_P(tm_single_device_test_t, create_remove_same_transaction)
        t,
        ADDR,
        SIZE,
-       'a',
-       tm_config.default_placement_hint);
+       'a');
 
       submit_transaction(std::move(t));
       check();
@@ -935,8 +922,7 @@ TEST_P(tm_single_device_test_t, split_merge_read_same_transaction)
        auto extent = alloc_extent(
          t,
          laddr_t(i * SIZE),
-         SIZE,
-         tm_config.default_placement_hint);
+         SIZE);
       }
       check_mappings(t);
       submit_transaction(std::move(t));
@@ -967,8 +953,7 @@ TEST_P(tm_single_device_test_t, inc_dec_ref)
        t,
        ADDR,
        SIZE,
-       'a',
-       tm_config.default_placement_hint);
+       'a');
       ASSERT_EQ(ADDR, extent->get_laddr());
       check_mappings(t);
       check();
@@ -1014,8 +999,7 @@ TEST_P(tm_single_device_test_t, cause_lba_split)
        t,
        i * SIZE,
        SIZE,
-       (char)(i & 0xFF),
-       tm_config.default_placement_hint);
+       (char)(i & 0xFF));
       ASSERT_EQ(i * SIZE, extent->get_laddr());
       submit_transaction(std::move(t));
     }
@@ -1035,8 +1019,7 @@ TEST_P(tm_single_device_test_t, random_writes)
       auto extent = alloc_extent(
        t,
        i * BSIZE,
-       BSIZE,
-       tm_config.default_placement_hint);
+       BSIZE);
       ASSERT_EQ(i * BSIZE, extent->get_laddr());
       submit_transaction(std::move(t));
     }
@@ -1054,8 +1037,7 @@ TEST_P(tm_single_device_test_t, random_writes)
          auto padding = alloc_extent(
            t,
            TOTAL + (k * PADDING_SIZE),
-           PADDING_SIZE,
-           tm_config.default_placement_hint);
+           PADDING_SIZE);
          dec_ref(t, padding->get_laddr());
        }
        submit_transaction(std::move(t));