crimson/os/seastore/seastore_types: unify checks to the extent types

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

diff --git a/src/crimson/os/seastore/cache.cc b/src/crimson/os/seastore/cache.cc
index 6990bf183a200c0b6cbc4e6a243a25e6abb37ed9..a118b822203d0f95cd6458b203015e1eccf7119c 100644 (file)
--- a/src/crimson/os/seastore/cache.cc
+++ b/src/crimson/os/seastore/cache.cc
@@ -795,7 +795,7 @@ void Cache::commit_replace_extent(
   assert(next->version == prev->version + 1);
   extents.replace(*next, *prev);
 
-  if (prev->get_type() == extent_types_t::ROOT) {
+  if (is_root_type(prev->get_type())) {
     assert(prev->is_stable_clean()
       || prev->primary_ref_list_hook.is_linked());
     if (prev->is_dirty()) {
@@ -1081,7 +1081,7 @@ CachedExtentRef Cache::duplicate_for_write(
   auto [iter, inserted] = i->mutation_pendings.insert(*ret);
   ceph_assert(inserted);
   t.add_mutated_extent(ret);
-  if (ret->get_type() == extent_types_t::ROOT) {
+  if (is_root_type(ret->get_type())) {
     t.root = ret->cast<RootBlock>();
   } else {
     ret->last_committed_crc = i->last_committed_crc;
@@ -1175,7 +1175,7 @@ record_t Cache::prepare_record(
 
     assert(i->get_version() > 0);
     auto final_crc = i->calc_crc32c();
-    if (i->get_type() == extent_types_t::ROOT) {
+    if (is_root_type(i->get_type())) {
       SUBTRACET(seastore_t, "writing out root delta {}B -- {}",
                 t, delta_length, *i);
       assert(t.root == i);
@@ -1243,8 +1243,8 @@ record_t Cache::prepare_record(
     retire_stat.increment(extent->get_length());
     DEBUGT("retired and remove extent -- {}", t, *extent);
     commit_retire_extent(t, extent);
-    if (is_backref_mapped_extent_node(extent)
-         || is_retired_placeholder(extent->get_type())) {
+    if (is_backref_mapped_extent_node(extent) ||
+        is_retired_placeholder_type(extent->get_type())) {
       rel_delta.alloc_blk_ranges.emplace_back(
        extent->get_paddr(),
        L_ADDR_NULL,
@@ -1277,7 +1277,7 @@ record_t Cache::prepare_record(
     i->prepare_write();
     i->prepare_commit();
     bl.append(i->get_bptr());
-    if (i->get_type() == extent_types_t::ROOT) {
+    if (is_root_type(i->get_type())) {
       ceph_assert(0 == "ROOT never gets written as a fresh block");
     }
 
@@ -1609,7 +1609,7 @@ void Cache::complete_commit(
     i->prior_instance = CachedExtentRef();
     i->state = CachedExtent::extent_state_t::DIRTY;
     assert(i->version > 0);
-    if (i->version == 1 || i->get_type() == extent_types_t::ROOT) {
+    if (i->version == 1 || is_root_type(i->get_type())) {
       i->dirty_from_or_retired_at = start_seq;
       DEBUGT("commit extent done, become dirty -- {}", t, *i);
     } else {
@@ -1638,8 +1638,8 @@ void Cache::complete_commit(
   for (auto &i: t.retired_set) {
     auto &extent = i.extent;
     extent->dirty_from_or_retired_at = start_seq;
-    if (is_backref_mapped_extent_node(extent)
-         || is_retired_placeholder(extent->get_type())) {
+    if (is_backref_mapped_extent_node(extent) ||
+        is_retired_placeholder_type(extent->get_type())) {
       DEBUGT("backref_list free {} len {}",
             t,
             extent->get_paddr(),
@@ -1855,7 +1855,7 @@ Cache::replay_delta(
       std::make_pair(false, nullptr));
   }
 
-  if (delta.type == extent_types_t::ROOT) {
+  if (is_root_type(delta.type)) {
     TRACE("replay root delta at {} {}, remove extent ... -- {}, prv_root={}",
           journal_seq, record_base, delta, *root);
     remove_extent(root);
@@ -1877,7 +1877,7 @@ Cache::replay_delta(
       if (ret) {
         // no retired-placeholder should be exist yet because no transaction
         // has been created.
-        assert(ret->get_type() != extent_types_t::RETIRED_PLACEHOLDER);
+        assert(!is_retired_placeholder_type(ret->get_type()));
         return ret->wait_io().then([ret] {
           return ret;
         });
@@ -2007,7 +2007,7 @@ Cache::get_next_dirty_extents_ret Cache::get_next_dirty_extents(
            if (result == Transaction::get_extent_ret::ABSENT) {
              DEBUGT("extent is absent on t -- {}", t, *ext);
              t.add_to_read_set(ext);
-             if (ext->get_type() == extent_types_t::ROOT) {
+             if (is_root_type(ext->get_type())) {
                if (t.root) {
                  assert(&*t.root == &*ext);
                  ceph_assert(0 == "t.root would have to already be in the read set");

diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h
index 4d6cb356a9db6f53d5f0c3c9db0309594b12bdbe..681879b13a0bf574695bb0b6286a80c33ebe3e76 100644 (file)
--- a/src/crimson/os/seastore/cache.h
+++ b/src/crimson/os/seastore/cache.h
@@ -312,7 +312,7 @@ public:
     if (!ret) {
       SUBDEBUGT(seastore_cache, "{} {} is absent", t, type, offset);
       return get_extent_if_cached_iertr::make_ready_future<CachedExtentRef>();
-    } else if (ret->get_type() == extent_types_t::RETIRED_PLACEHOLDER) {
+    } else if (is_retired_placeholder_type(ret->get_type())) {
       // retired_placeholder is not really cached yet
       SUBDEBUGT(seastore_cache, "{} {} is absent(placeholder)",
                 t, type, offset);
@@ -515,7 +515,7 @@ public:
     }
 
     // user should not see RETIRED_PLACEHOLDER extents
-    ceph_assert(p_extent->get_type() != extent_types_t::RETIRED_PLACEHOLDER);
+    ceph_assert(!is_retired_placeholder_type(p_extent->get_type()));
     if (!p_extent->is_fully_loaded()) {
       assert(!p_extent->is_mutable());
       LOG_PREFIX(Cache::get_extent_viewable_by_trans);
@@ -597,7 +597,7 @@ private:
     }
 
     // extent PRESENT in cache
-    if (cached->get_type() == extent_types_t::RETIRED_PLACEHOLDER) {
+    if (is_retired_placeholder_type(cached->get_type())) {
       auto ret = CachedExtent::make_cached_extent_ref<T>(
         alloc_cache_buf(length));
       ret->init(CachedExtent::extent_state_t::CLEAN_PENDING,
@@ -1736,7 +1736,7 @@ private:
         iter != extents.end()) {
       if (p_metric_key &&
           // retired_placeholder is not really cached yet
-          iter->get_type() != extent_types_t::RETIRED_PLACEHOLDER) {
+          !is_retired_placeholder_type(iter->get_type())) {
         ++p_counters->hit;
       }
       return CachedExtentRef(&*iter);

diff --git a/src/crimson/os/seastore/cached_extent.h b/src/crimson/os/seastore/cached_extent.h
index 4c394bbe281dc60af75388907b9716b38eeea108..86d35fd9de04e08f4e42b9bf0e0ec2401641418d 100644 (file)
--- a/src/crimson/os/seastore/cached_extent.h
+++ b/src/crimson/os/seastore/cached_extent.h
@@ -515,7 +515,7 @@ public:
 
   /// Returns true if extent is a plcaeholder
   bool is_placeholder() const {
-    return get_type() == extent_types_t::RETIRED_PLACEHOLDER;
+    return is_retired_placeholder_type(get_type());
   }
 
   bool is_pending_io() const {

diff --git a/src/crimson/os/seastore/extent_placement_manager.h b/src/crimson/os/seastore/extent_placement_manager.h
index 0f2d55ef04aeb5d99ee829aae1fb7dcf28de8b60..14047d8650848f1937e3af3568c8563b6e104af2 100644 (file)
--- a/src/crimson/os/seastore/extent_placement_manager.h
+++ b/src/crimson/os/seastore/extent_placement_manager.h
@@ -177,7 +177,7 @@ public:
       return false;
     }
     assert(t.get_src() == transaction_type_t::TRIM_DIRTY);
-    ceph_assert_always(extent->get_type() == extent_types_t::ROOT ||
+    ceph_assert_always(is_root_type(extent->get_type()) ||
        extent->get_paddr().is_absolute());
     return crimson::os::seastore::can_inplace_rewrite(extent->get_type());
   }
@@ -571,7 +571,8 @@ private:
       extent_types_t type,
       placement_hint_t hint,
       rewrite_gen_t gen) {
-    if (type == extent_types_t::ROOT) {
+    assert(is_real_type(type));
+    if (is_root_type(type)) {
       gen = INLINE_GENERATION;
     } else if (get_main_backend_type() == backend_type_t::SEGMENTED &&
                is_lba_backref_node(type)) {

diff --git a/src/crimson/os/seastore/seastore_types.cc b/src/crimson/os/seastore/seastore_types.cc
index 5481a680f27e3bff74414b6b62d244576fea66fc..58fe134b3777e811b04a4052d529b6a4c780aaf8 100644 (file)
--- a/src/crimson/os/seastore/seastore_types.cc
+++ b/src/crimson/os/seastore/seastore_types.cc
@@ -245,6 +245,10 @@ std::ostream &operator<<(std::ostream &out, extent_types_t t)
     return out << "OBJECT_DATA_BLOCK";
   case extent_types_t::RETIRED_PLACEHOLDER:
     return out << "RETIRED_PLACEHOLDER";
+  case extent_types_t::ALLOC_INFO:
+    return out << "ALLOC_INFO";
+  case extent_types_t::JOURNAL_TAIL:
+    return out << "JOURNAL_TAIL";
   case extent_types_t::TEST_BLOCK:
     return out << "TEST_BLOCK";
   case extent_types_t::TEST_BLOCK_PHYSICAL:
@@ -256,7 +260,7 @@ std::ostream &operator<<(std::ostream &out, extent_types_t t)
   case extent_types_t::NONE:
     return out << "NONE";
   default:
-    return out << "UNKNOWN";
+    return out << "UNKNOWN(" << (unsigned)t << ")";
   }
 }
 
@@ -290,7 +294,7 @@ std::ostream &operator<<(std::ostream &out, data_category_t c)
 }
 
 bool can_inplace_rewrite(extent_types_t type) {
-  return get_extent_category(type) == data_category_t::DATA;
+  return is_data_type(type);
 }
 
 std::ostream &operator<<(std::ostream &out, sea_time_point_printer_t tp)

diff --git a/src/crimson/os/seastore/seastore_types.h b/src/crimson/os/seastore/seastore_types.h
index 35c331d50963ef987514fbfaac72ec32a7fa5c0a..1713b98a0929b66705d1d3754c998497f33fb137 100644 (file)
--- a/src/crimson/os/seastore/seastore_types.h
+++ b/src/crimson/os/seastore/seastore_types.h
@@ -1197,42 +1197,83 @@ constexpr size_t BACKREF_NODE_SIZE = 4096;
 
 std::ostream &operator<<(std::ostream &out, extent_types_t t);
 
+constexpr bool is_data_type(extent_types_t type) {
+  return type == extent_types_t::OBJECT_DATA_BLOCK ||
+         type == extent_types_t::TEST_BLOCK;
+}
+
+constexpr bool is_logical_metadata_type(extent_types_t type) {
+  return type >= extent_types_t::OMAP_INNER &&
+         type <= extent_types_t::COLL_BLOCK;
+}
+
 constexpr bool is_logical_type(extent_types_t type) {
-  switch (type) {
-  case extent_types_t::ROOT:
-  case extent_types_t::LADDR_INTERNAL:
-  case extent_types_t::LADDR_LEAF:
-  case extent_types_t::BACKREF_INTERNAL:
-  case extent_types_t::BACKREF_LEAF:
-    return false;
-  default:
+  if ((type >= extent_types_t::OMAP_INNER &&
+       type <= extent_types_t::OBJECT_DATA_BLOCK) ||
+      type == extent_types_t::TEST_BLOCK) {
+    assert(is_logical_metadata_type(type) ||
+           is_data_type(type));
     return true;
+  } else {
+    assert(!is_logical_metadata_type(type) &&
+           !is_data_type(type));
+    return false;
   }
 }
 
-constexpr bool is_retired_placeholder(extent_types_t type)
-{
+constexpr bool is_retired_placeholder_type(extent_types_t type) {
   return type == extent_types_t::RETIRED_PLACEHOLDER;
 }
 
-constexpr bool is_lba_node(extent_types_t type)
-{
+constexpr bool is_root_type(extent_types_t type) {
+  return type == extent_types_t::ROOT;
+}
+
+constexpr bool is_lba_node(extent_types_t type) {
   return type == extent_types_t::LADDR_INTERNAL ||
-    type == extent_types_t::LADDR_LEAF ||
-    type == extent_types_t::DINK_LADDR_LEAF;
+         type == extent_types_t::LADDR_LEAF ||
+         type == extent_types_t::DINK_LADDR_LEAF;
 }
 
-constexpr bool is_backref_node(extent_types_t type)
-{
+constexpr bool is_backref_node(extent_types_t type) {
   return type == extent_types_t::BACKREF_INTERNAL ||
-    type == extent_types_t::BACKREF_LEAF;
+         type == extent_types_t::BACKREF_LEAF;
 }
 
-constexpr bool is_lba_backref_node(extent_types_t type)
-{
+constexpr bool is_lba_backref_node(extent_types_t type) {
   return is_lba_node(type) || is_backref_node(type);
 }
 
+constexpr bool is_physical_type(extent_types_t type) {
+  if (type <= extent_types_t::DINK_LADDR_LEAF ||
+      (type >= extent_types_t::TEST_BLOCK_PHYSICAL &&
+       type <= extent_types_t::BACKREF_LEAF)) {
+    assert(is_root_type(type) ||
+           is_lba_backref_node(type) ||
+           type == extent_types_t::TEST_BLOCK_PHYSICAL);
+    return true;
+  } else {
+    assert(!is_root_type(type) &&
+           !is_lba_backref_node(type) &&
+           type != extent_types_t::TEST_BLOCK_PHYSICAL);
+    return false;
+  }
+}
+
+constexpr bool is_real_type(extent_types_t type) {
+  if (type <= extent_types_t::OBJECT_DATA_BLOCK ||
+      (type >= extent_types_t::TEST_BLOCK &&
+       type <= extent_types_t::BACKREF_LEAF)) {
+    assert(is_logical_type(type) ||
+           is_physical_type(type));
+    return true;
+  } else {
+    assert(!is_logical_type(type) &&
+           !is_physical_type(type));
+    return false;
+  }
+}
+
 std::ostream &operator<<(std::ostream &out, extent_types_t t);
 
 /**
@@ -1304,8 +1345,7 @@ enum class data_category_t : uint8_t {
 std::ostream &operator<<(std::ostream &out, data_category_t c);
 
 constexpr data_category_t get_extent_category(extent_types_t type) {
-  if (type == extent_types_t::OBJECT_DATA_BLOCK ||
-      type == extent_types_t::TEST_BLOCK) {
+  if (is_data_type(type)) {
     return data_category_t::DATA;
   } else {
     return data_category_t::METADATA;

diff --git a/src/crimson/os/seastore/transaction.h b/src/crimson/os/seastore/transaction.h
index d3fffc4724c7f51d9cc872999682d296e9f5bb7c..7dd3bf13fcbe861f5e353b79d00dd05f0804affa 100644 (file)
--- a/src/crimson/os/seastore/transaction.h
+++ b/src/crimson/os/seastore/transaction.h
@@ -101,7 +101,7 @@ public:
       iter != read_set.end()) {
       // placeholder in read-set should be in the retired-set
       // at the same time.
-      assert(iter->ref->get_type() != extent_types_t::RETIRED_PLACEHOLDER);
+      assert(!is_retired_placeholder_type(iter->ref->get_type()));
       if (out)
        *out = iter->ref;
       SUBTRACET(seastore_cache, "{} is present in read_set -- {}",
@@ -270,9 +270,9 @@ public:
   void replace_placeholder(CachedExtent& placeholder, CachedExtent& extent) {
     ceph_assert(!is_weak());
 
-    assert(placeholder.get_type() == extent_types_t::RETIRED_PLACEHOLDER);
-    assert(extent.get_type() != extent_types_t::RETIRED_PLACEHOLDER);
-    assert(extent.get_type() != extent_types_t::ROOT);
+    assert(is_retired_placeholder_type(placeholder.get_type()));
+    assert(!is_retired_placeholder_type(extent.get_type()));
+    assert(!is_root_type(extent.get_type()));
     assert(extent.get_paddr() == placeholder.get_paddr());
     {
       auto where = read_set.find(placeholder.get_paddr());

diff --git a/src/crimson/os/seastore/transaction_manager.cc b/src/crimson/os/seastore/transaction_manager.cc
index 1b309a036838c595486f1b29259548d015464911..7e75b8bc4975bbfda12ee71331155bbd9efae33a 100644 (file)
--- a/src/crimson/os/seastore/transaction_manager.cc
+++ b/src/crimson/os/seastore/transaction_manager.cc
@@ -658,7 +658,7 @@ TransactionManager::rewrite_extent_ret TransactionManager::rewrite_extent(
     return backref_manager->rewrite_extent(t, extent);
   }
 
-  if (extent->get_type() == extent_types_t::ROOT) {
+  if (is_root_type(extent->get_type())) {
     DEBUGT("rewriting root extent -- {}", t, *extent);
     cache->duplicate_for_write(t, extent);
     return rewrite_extent_iertr::now();

diff --git a/src/test/crimson/seastore/test_transaction_manager.cc b/src/test/crimson/seastore/test_transaction_manager.cc
index f8845dc8ceaede53315e94c3926fa28ea9e3106e..e4b750577a1df34c54039336e3d1357e0fdc90bf 100644 (file)
--- a/src/test/crimson/seastore/test_transaction_manager.cc
+++ b/src/test/crimson/seastore/test_transaction_manager.cc
@@ -978,14 +978,14 @@ struct transaction_manager_test_t :
         extent_types_t::ONODE_BLOCK_STAGED,
         extent_types_t::COLL_BLOCK,
         extent_types_t::OBJECT_DATA_BLOCK,
-        extent_types_t::RETIRED_PLACEHOLDER,
-        extent_types_t::ALLOC_INFO,
-        extent_types_t::JOURNAL_TAIL,
         extent_types_t::TEST_BLOCK,
         extent_types_t::TEST_BLOCK_PHYSICAL,
         extent_types_t::BACKREF_INTERNAL,
         extent_types_t::BACKREF_LEAF
       };
+      // exclude DINK_LADDR_LEAF, RETIRED_PLACEHOLDER,
+      //         ALLOC_INFO, JOURNAL_TAIL
+      assert(all_extent_types.size() == EXTENT_TYPES_MAX - 4);
 
       std::vector<rewrite_gen_t> all_generations;
       for (auto i = INIT_GENERATION; i < REWRITE_GENERATIONS; i++) {
@@ -998,8 +998,9 @@ struct transaction_manager_test_t :
 
       // this loop should be consistent with EPM::adjust_generation
       for (auto t : all_extent_types) {
+        assert(is_real_type(t));
         expected_generations[t] = {};
-        if (!is_logical_type(t)) {
+        if (is_root_type(t) || is_lba_backref_node(t)) {
           for (auto gen : all_generations) {
             expected_generations[t][gen] = INLINE_GENERATION;
           }
@@ -1018,7 +1019,7 @@ struct transaction_manager_test_t :
 
       auto update_data_gen_mapping = [&](std::function<rewrite_gen_t(rewrite_gen_t)> func) {
         for (auto t : all_extent_types) {
-          if (!is_logical_type(t)) {
+          if (is_root_type(t) || is_lba_backref_node(t)) {
             continue;
           }
           for (auto i = INIT_GENERATION + 1; i < REWRITE_GENERATIONS; i++) {