crimson/os/seastore: introduce strict paddr type checks in cache and transaction

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 43715f34c1482cc5269cedd439796799e457a445..a7ef1da98ba812eb41d18fa3261273df608b4680 100644 (file)
--- a/src/crimson/os/seastore/cache.cc
+++ b/src/crimson/os/seastore/cache.cc
@@ -731,11 +731,14 @@ void Cache::add_extent(CachedExtentRef ref)
   assert(ref->is_valid());
   assert(ref->user_hint == PLACEMENT_HINT_NULL);
   assert(ref->rewrite_generation == NULL_GENERATION);
+  assert(ref->get_paddr().is_absolute() ||
+         ref->get_paddr().is_root());
   extents_index.insert(*ref);
 }
 
 void Cache::mark_dirty(CachedExtentRef ref)
 {
+  assert(ref->get_paddr().is_absolute());
   if (ref->is_dirty()) {
     assert(ref->primary_ref_list_hook.is_linked());
     return;
@@ -754,6 +757,8 @@ void Cache::add_to_dirty(
   assert(!ref->primary_ref_list_hook.is_linked());
   ceph_assert(ref->get_modify_time() != NULL_TIME);
   assert(ref->is_fully_loaded());
+  assert(ref->get_paddr().is_absolute() ||
+         ref->get_paddr().is_root());
 
   // Note: next might not be at extent_state_t::DIRTY,
   // also see CachedExtent::is_stable_writting()
@@ -783,6 +788,8 @@ void Cache::remove_from_dirty(
   assert(ref->is_dirty());
   ceph_assert(ref->primary_ref_list_hook.is_linked());
   assert(ref->is_fully_loaded());
+  assert(ref->get_paddr().is_absolute() ||
+         ref->get_paddr().is_root());
 
   auto extent_length = ref->get_length();
   stats.dirty_bytes -= extent_length;
@@ -864,9 +871,12 @@ void Cache::remove_extent(
     const Transaction::src_t* p_src)
 {
   assert(ref->is_valid());
+  assert(ref->get_paddr().is_absolute() ||
+         ref->get_paddr().is_root());
   if (ref->is_dirty()) {
     remove_from_dirty(ref, p_src);
   } else if (!ref->is_placeholder()) {
+    assert(ref->get_paddr().is_absolute());
     lru.remove_from_lru(*ref);
   }
   extents_index.erase(*ref);
@@ -889,6 +899,7 @@ void Cache::commit_replace_extent(
     CachedExtentRef prev)
 {
   assert(next->get_paddr() == prev->get_paddr());
+  assert(next->get_paddr().is_absolute() || next->get_paddr().is_root());
   assert(next->version == prev->version + 1);
   extents_index.replace(*next, *prev);
 
@@ -2006,6 +2017,7 @@ Cache::replay_delta(
   if (is_root_type(delta.type)) {
     TRACE("replay root delta at {} {}, remove extent ... -- {}, prv_root={}",
           journal_seq, record_base, delta, *root);
+    ceph_assert(delta.paddr.is_root());
     remove_extent(root, nullptr);
     root->apply_delta_and_adjust_crc(record_base, delta.bl);
     root->dirty_from_or_retired_at = journal_seq;
@@ -2019,6 +2031,7 @@ Cache::replay_delta(
     return replay_delta_ertr::make_ready_future<std::pair<bool, CachedExtentRef>>(
       std::make_pair(true, root));
   } else {
+    ceph_assert(delta.paddr.is_absolute());
     auto _get_extent_if_cached = [this](paddr_t addr)
       -> get_extent_ertr::future<CachedExtentRef> {
       // replay is not included by the cache hit metrics

diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h
index b52db400778520d306470422a5c73f2c711f8cf3..764ed17c2dfb12a157e24330f588399e3c7400cd 100644 (file)
--- a/src/crimson/os/seastore/cache.h
+++ b/src/crimson/os/seastore/cache.h
@@ -255,6 +255,7 @@ public:
       });
     }
 
+    assert(paddr.is_absolute());
     // get_extent_ret::ABSENT from transaction
     ret = query_cache(paddr);
     if (!ret) {
@@ -590,6 +591,7 @@ public:
 // Interfaces only for tests.
 public:
   CachedExtentRef test_query_cache(paddr_t offset) {
+    assert(offset.is_absolute());
     return query_cache(offset);
   }
 
@@ -678,6 +680,7 @@ private:
     const Transaction::src_t* p_src
   ) {
     LOG_PREFIX(Cache::do_get_caching_extent);
+    assert(offset.is_absolute());
     auto cached = query_cache(offset);
     if (!cached) {
       // partial read
@@ -1484,6 +1487,7 @@ private:
       const Transaction::src_t* p_src,
       cache_hint_t hint)
   {
+    assert(ext.get_paddr().is_absolute());
     if (hint == CACHE_HINT_NOCACHE && is_logical_type(ext.get_type())) {
       return;
     }
@@ -1919,6 +1923,7 @@ private:
     assert(!extent->is_range_loaded(offset, length));
     assert(is_aligned(offset, get_block_size()));
     assert(is_aligned(length, get_block_size()));
+    assert(extent->get_paddr().is_absolute());
     extent->set_io_wait();
     auto old_length = extent->get_loaded_length();
     load_ranges_t to_read = extent->load_ranges(offset, length);

diff --git a/src/crimson/os/seastore/transaction.h b/src/crimson/os/seastore/transaction.h
index 84320e5bb33a4a5728d96b0625855780771848d2..cc0bf09f24c725d48149b985de08aa62d9582b27 100644 (file)
--- a/src/crimson/os/seastore/transaction.h
+++ b/src/crimson/os/seastore/transaction.h
@@ -107,6 +107,7 @@ public:
     RETIRED
   };
   get_extent_ret get_extent(paddr_t addr, CachedExtentRef *out) {
+    assert(addr.is_real_location() || addr.is_root());
     auto [result, ext] = do_get_extent(addr);
     // placeholder in read-set must be in the retired-set
     // at the same time, user should not see a placeholder.
@@ -119,12 +120,14 @@ public:
   }
 
   void add_absent_to_retired_set(CachedExtentRef ref) {
+    assert(ref->get_paddr().is_absolute());
     bool added = do_add_to_read_set(ref);
     ceph_assert(added);
     add_present_to_retired_set(ref);
   }
 
   void add_present_to_retired_set(CachedExtentRef ref) {
+    assert(ref->get_paddr().is_real_location());
     assert(!is_weak());
 #ifndef NDEBUG
     auto [result, ext] = do_get_extent(ref->get_paddr());
@@ -156,6 +159,7 @@ public:
 
   // Returns true if added, false if already added or weak
   bool maybe_add_to_read_set(CachedExtentRef ref) {
+    assert(ref->get_paddr().is_absolute());
     if (is_weak()) {
       return false;
     }
@@ -163,6 +167,8 @@ public:
   }
 
   void add_to_read_set(CachedExtentRef ref) {
+    assert(ref->get_paddr().is_absolute()
+           || ref->get_paddr().is_root());
     if (is_weak()) {
       return;
     }
@@ -173,6 +179,7 @@ public:
 
   void add_fresh_extent(
     CachedExtentRef ref) {
+    assert(ref->get_paddr().is_real_location());
     ceph_assert(!is_weak());
     if (ref->is_exist_clean()) {
       existing_block_stats.inc(ref);
@@ -255,6 +262,8 @@ public:
 
   void add_mutated_extent(CachedExtentRef ref) {
     ceph_assert(!is_weak());
+    assert(ref->get_paddr().is_absolute() ||
+           ref->get_paddr().is_root());
     assert(ref->is_exist_mutation_pending() ||
           read_set.count(ref->prior_instance->get_paddr()));
     mutated_block_list.push_back(ref);
@@ -273,6 +282,7 @@ public:
     assert(!is_retired_placeholder_type(extent.get_type()));
     assert(!is_root_type(extent.get_type()));
     assert(extent.get_paddr() == placeholder.get_paddr());
+    assert(extent.get_paddr().is_absolute());
     {
       auto where = read_set.find(placeholder.get_paddr());
       assert(where != read_set.end());
@@ -327,6 +337,7 @@ public:
   }
 
   bool is_stable_extent_retired(paddr_t paddr, extent_len_t len) {
+    assert(paddr.is_absolute());
     auto iter = retired_set.lower_bound(paddr);
     if (iter == retired_set.end()) {
       return false;