crimson/os/seastore/cache: minor cleanup

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 3af5c0d4500caa12f076207bca3e9fdc037991bf..44d36694e24d2fa3f04d2e64993b6a34c2038f21 100644 (file)
--- a/src/crimson/os/seastore/cache.cc
+++ b/src/crimson/os/seastore/cache.cc
@@ -1346,24 +1346,22 @@ record_t Cache::prepare_record(
     get_by_ext(efforts.retire_by_ext,
                extent->get_type()).increment(extent->get_length());
     retire_stat.increment(extent->get_length());
-    DEBUGT("retired and remove extent -- {}", t, *extent);
+    DEBUGT("retired and remove extent {}~0x{:x} -- {}",
+          t, extent->get_paddr(), extent->get_length(), *extent);
     commit_retire_extent(t, extent);
-    if (is_backref_mapped_type(extent->get_type()) ||
-       is_retired_placeholder_type(extent->get_type())) {
-      rel_delta.alloc_blk_ranges.emplace_back(
-       alloc_blk_t::create_retire(
-         extent->get_paddr(),
-         extent->get_length(),
-         extent->get_type()));
-    }
 
     // Note: commit extents and backref allocations in the same place
     if (is_backref_mapped_type(extent->get_type()) ||
        is_retired_placeholder_type(extent->get_type())) {
-      DEBUGT("backref_entry free {} len 0x{:x}",
+      DEBUGT("backref_entry free {}~0x{:x}",
             t,
             extent->get_paddr(),
             extent->get_length());
+      rel_delta.alloc_blk_ranges.emplace_back(
+       alloc_blk_t::create_retire(
+         extent->get_paddr(),
+         extent->get_length(),
+         extent->get_type()));
       backref_entries.emplace_back(
        backref_entry_t::create_retire(
          extent->get_paddr(),
@@ -1424,8 +1422,11 @@ record_t Cache::prepare_record(
        std::move(bl)
       },
       modify_time);
-    if (i->is_valid() &&
-       is_backref_mapped_type(i->get_type())) {
+
+    if (!i->is_valid()) {
+      continue;
+    }
+    if (is_backref_mapped_type(i->get_type())) {
       laddr_t alloc_laddr;
       if (i->is_logical()) {
        alloc_laddr = i->cast<LogicalCachedExtent>()->get_laddr();
@@ -1726,7 +1727,7 @@ void Cache::complete_commit(
 
     // Note: commit extents and backref allocations in the same place
     if (is_backref_mapped_type(i->get_type())) {
-      DEBUGT("backref_entry alloc {} len 0x{:x}",
+      DEBUGT("backref_entry alloc {}~0x{:x}",
             t,
             i->get_paddr(),
             i->get_length());
@@ -1781,9 +1782,10 @@ void Cache::complete_commit(
     epm.mark_space_free(extent->get_paddr(), extent->get_length());
   }
   for (auto &i: t.existing_block_list) {
-    if (i->is_valid()) {
-      epm.mark_space_used(i->get_paddr(), i->get_length());
+    if (!i->is_valid()) {
+      continue;
     }
+    epm.mark_space_used(i->get_paddr(), i->get_length());
   }
 
   for (auto &i: t.mutated_block_list) {
@@ -1807,31 +1809,32 @@ void Cache::complete_commit(
         existing_stats.clean_num,
         existing_stats.mutated_num);
   for (auto &i: t.existing_block_list) {
-    if (i->is_valid()) {
-      if (i->is_exist_clean()) {
-       i->state = CachedExtent::extent_state_t::CLEAN;
-      } else {
-       assert(i->state == CachedExtent::extent_state_t::DIRTY);
-      }
-      add_extent(i);
+    if (!i->is_valid()) {
+      continue;
+    }
+    if (i->is_exist_clean()) {
+      i->state = CachedExtent::extent_state_t::CLEAN;
+    } else {
+      assert(i->state == CachedExtent::extent_state_t::DIRTY);
+    }
+    add_extent(i);
 
-      // Note: commit extents and backref allocations in the same place
-      DEBUGT("backref_entry alloc existing {} len 0x{:x}",
-            t,
-            i->get_paddr(),
-            i->get_length());
-      backref_entries.emplace_back(
-       backref_entry_t::create_alloc(
-         i->get_paddr(),
-         i->cast<LogicalCachedExtent>()->get_laddr(),
-         i->get_length(),
-         i->get_type()));
-      const auto t_src = t.get_src();
-      if (i->is_dirty()) {
-        add_to_dirty(i, &t_src);
-      } else {
-        touch_extent(*i, &t_src);
-      }
+    // Note: commit extents and backref allocations in the same place
+    DEBUGT("backref_entry alloc existing {} len 0x{:x}",
+          t,
+          i->get_paddr(),
+          i->get_length());
+    backref_entries.emplace_back(
+      backref_entry_t::create_alloc(
+       i->get_paddr(),
+       i->cast<LogicalCachedExtent>()->get_laddr(),
+       i->get_length(),
+       i->get_type()));
+    const auto t_src = t.get_src();
+    if (i->is_dirty()) {
+      add_to_dirty(i, &t_src);
+    } else {
+      touch_extent(*i, &t_src);
     }
   }
 

diff --git a/src/test/crimson/seastore/test_block.h b/src/test/crimson/seastore/test_block.h
index e1fe8e06f8a465e0d91ee7edcc5141ac2a8745d9..546f357dea057879900951ca74e81f86501b4441 100644 (file)
--- a/src/test/crimson/seastore/test_block.h
+++ b/src/test/crimson/seastore/test_block.h
@@ -39,8 +39,8 @@ struct test_block_delta_t {
 
 inline std::ostream &operator<<(
   std::ostream &lhs, const test_extent_desc_t &rhs) {
-  return lhs << "test_extent_desc_t(len=" << rhs.len
-            << ", checksum=" << rhs.checksum << ")";
+  return lhs << "test_extent_desc_t(len=0x" << std::hex << rhs.len
+            << ", checksum=0x" << rhs.checksum << std::dec << ")";
 }
 
 struct TestBlock : crimson::os::seastore::LogicalCachedExtent {