crimson/os/seastore/omap_manager: simplify maybe_init from tolerating duplicated...

Also added asserts around seen_by_users.

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

diff --git a/src/crimson/os/seastore/logical_child_node.h b/src/crimson/os/seastore/logical_child_node.h
index 7fb3b1eebc2527dd552c1450214182e5b58b7f38..1c5ef01ba0a12a47ee4011cd19a5bbd28add4655 100644 (file)
--- a/src/crimson/os/seastore/logical_child_node.h
+++ b/src/crimson/os/seastore/logical_child_node.h
@@ -40,6 +40,7 @@ public:
   }
 protected:
   void on_replace_prior() final {
+    assert(is_seen_by_users());
     lba_child_node_t::on_replace_prior();
     do_on_replace_prior();
   }

diff --git a/src/crimson/os/seastore/omap_manager/btree/omap_btree_node.h b/src/crimson/os/seastore/omap_manager/btree/omap_btree_node.h
index cbdacd28601e9393ad146aac831aed74d86b22b6..9a2ef3795749c8eef9c714a9b36b13bd81f1e50a 100644 (file)
--- a/src/crimson/os/seastore/omap_manager/btree/omap_btree_node.h
+++ b/src/crimson/os/seastore/omap_manager/btree/omap_btree_node.h
@@ -116,17 +116,14 @@ struct OMapNode : LogicalChildNode {
 
   virtual ~OMapNode() = default;
 
-  void init_range(std::string begin, std::string end) {
-    if (!this->end.empty()) {
-      // this is a duplicated init.
-      assert(end == this->end);
-      return;
-    }
-    if (begin == BEGIN_KEY && end == END_KEY) {
+  void init_range(std::string _begin, std::string _end) {
+    assert(begin.empty());
+    assert(end.empty());
+    if (_begin == BEGIN_KEY && _end == END_KEY) {
       root = true;
     }
-    this->begin = std::move(begin);
-    this->end = std::move(end);
+    begin = std::move(_begin);
+    end = std::move(_end);
   }
   const std::string &get_begin() const {
     return begin;

diff --git a/src/crimson/os/seastore/omap_manager/btree/omap_btree_node_impl.h b/src/crimson/os/seastore/omap_manager/btree/omap_btree_node_impl.h
index 32fb3f871464a95602b7355f426c7fa70ebd2449..3cd311610292a4e3b9f22a81ed3169f6bc619297 100644 (file)
--- a/src/crimson/os/seastore/omap_manager/btree/omap_btree_node_impl.h
+++ b/src/crimson/os/seastore/omap_manager/btree/omap_btree_node_impl.h
@@ -80,10 +80,15 @@ struct OMapInnerNode
     if (is_rewrite() && !is_btree_root()) {
       auto &prior = *get_prior_instance()->template cast<OMapInnerNode>();
       if (prior.base_child_t::has_parent_tracker()) {
+        assert(prior.is_seen_by_users());
        // unlike fixed-kv nodes, rewriting child nodes of the omap tree
        // won't affect parent nodes, so we have to manually take prior
        // instances' parent trackers here.
        this->child_node_t::take_parent_from_prior();
+      } else {
+        // dirty omap extent may not be accessed yet during rewrite,
+        // this means the extent may not be initalized yet as linked.
+        assert(!prior.is_seen_by_users());
       }
     }
   }
@@ -231,6 +236,7 @@ struct OMapInnerNode
     if (this->is_valid()
        && !this->is_pending()
        && !this->is_btree_root()
+       // dirty omap extent may not be accessed/linked yet
        && this->base_child_t::has_parent_tracker()) {
       this->child_node_t::destroy();
     }
@@ -282,10 +288,15 @@ struct OMapLeafNode
     if (is_rewrite() && !is_btree_root()) {
       auto &prior = *get_prior_instance()->template cast<OMapLeafNode>();
       if (prior.base_child_t::has_parent_tracker()) {
+        assert(prior.is_seen_by_users());
        // unlike fixed-kv nodes, rewriting child nodes of the omap tree
        // won't affect parent nodes, so we have to manually take prior
        // instances' parent trackers here.
        this->child_node_t::take_parent_from_prior();
+      } else {
+        // dirty omap extent may not be accessed yet during rewrite,
+        // this means the extent may not be initalized yet as linked.
+        assert(!prior.is_seen_by_users());
       }
     }
   }
@@ -303,6 +314,7 @@ struct OMapLeafNode
     if (this->is_valid()
        && !this->is_pending()
        && !this->is_btree_root()
+       // dirty omap extent may not be accessed/linked yet
        && this->base_child_t::has_parent_tracker()) {
       this->child_node_t::destroy();
     }
@@ -434,14 +446,15 @@ omap_load_extent(
     oc.t, laddr, size,
     [begin=std::move(begin), end=std::move(end), FNAME,
     oc, chp=std::move(chp)](T &extent) mutable {
+      assert(!extent.is_seen_by_users());
       extent.init_range(std::move(begin), std::move(end));
-      if (extent.T::base_child_t::is_parent_valid()
-         || extent.is_btree_root()) {
-       return;
+      if (extent.is_btree_root()) {
+        return;
       }
-      assert(chp);
       SUBDEBUGT(seastore_omap, "linking {} to {}",
-       oc.t, extent, *chp->get_parent());
+        oc.t, extent, *chp->get_parent());
+      assert(chp);
+      assert(!extent.T::base_child_t::is_parent_valid());
       chp->link_child(&extent);
     }
   ).handle_error_interruptible(