From: Yingxin Cheng <yingxin.cheng@intel.com>
Date: Wed, 19 May 2021 08:26:10 +0000 (+0800)
Subject: crimson/onode-staged-tree: free resources when call submit_transaction()
X-Git-Tag: v17.1.0~1880^2~7
X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=46e48390acb01bb171fea8d407fd0f54f766222a;p=ceph-ci.git

crimson/onode-staged-tree: free resources when call submit_transaction()

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

diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/node.cc b/src/crimson/os/seastore/onode_manager/staged-fltree/node.cc
index 2ca3ea2bd89..ee520bdcd76 100644
--- a/src/crimson/os/seastore/onode_manager/staged-fltree/node.cc
+++ b/src/crimson/os/seastore/onode_manager/staged-fltree/node.cc
@@ -2062,6 +2062,10 @@ void LeafNode::validate_cursor(const tree_cursor_t& cursor) const
   assert(this == cursor.get_leaf_node().get());
   assert(cursor.is_tracked());
   assert(!impl->is_extent_retired());
+
+  // We need to make sure user has freed all the cursors before submitting the
+  // according transaction. Otherwise the below checks will have undefined
+  // behaviors.
   auto [key, p_value_header] = get_kv(cursor.get_position());
   auto magic = p_value_header->magic;
   assert(key.compare_to(cursor.get_key_view(magic)) == MatchKindCMP::EQ);
diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/tree.h b/src/crimson/os/seastore/onode_manager/staged-fltree/tree.h
index 17f14dd149b..db7920c33f9 100644
--- a/src/crimson/os/seastore/onode_manager/staged-fltree/tree.h
+++ b/src/crimson/os/seastore/onode_manager/staged-fltree/tree.h
@@ -23,6 +23,9 @@
  * - Runs above seastore block and transaction layer;
  * - Specially optimized for onode key structures and seastore
  *   delta/transaction semantics;
+ *
+ * Note: User should not hold any Cursor/Value when call
+ * submit_transaction() because of validations implemented in ~tree_cursor_t().
  */
 
 namespace crimson::os::seastore::onode {
@@ -68,6 +71,11 @@ class Btree {
       }
     }
 
+    /// Invalidate the Cursor before submitting transaction.
+    void invalidate() {
+      p_cursor.reset();
+    }
+
     // XXX: return key_view_t to avoid unecessary ghobject_t constructions
     ghobject_t get_ghobj() const {
       assert(!is_end());
diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/value.cc b/src/crimson/os/seastore/onode_manager/staged-fltree/value.cc
index 18eebf71824..cd4bba0e393 100644
--- a/src/crimson/os/seastore/onode_manager/staged-fltree/value.cc
+++ b/src/crimson/os/seastore/onode_manager/staged-fltree/value.cc
@@ -36,6 +36,11 @@ bool Value::is_tracked() const
   return p_cursor->is_tracked();
 }
 
+void Value::invalidate()
+{
+  p_cursor.reset();
+}
+
 eagain_future<> Value::extend(Transaction& t, value_size_t extend_size)
 {
   assert(is_tracked());
diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/value.h b/src/crimson/os/seastore/onode_manager/staged-fltree/value.h
index fe7126f7a98..95f675d33ad 100644
--- a/src/crimson/os/seastore/onode_manager/staged-fltree/value.h
+++ b/src/crimson/os/seastore/onode_manager/staged-fltree/value.h
@@ -171,6 +171,9 @@ class Value {
   /// Returns whether the Value is still tracked in tree.
   bool is_tracked() const;
 
+  /// Invalidate the Value before submitting transaction.
+  void invalidate();
+
   /// Returns the value payload size.
   value_size_t get_payload_size() const {
     assert(is_tracked());
diff --git a/src/crimson/os/seastore/seastore.cc b/src/crimson/os/seastore/seastore.cc
index e286a173b80..cbbf8db70e8 100644
--- a/src/crimson/os/seastore/seastore.cc
+++ b/src/crimson/os/seastore/seastore.cc
@@ -605,6 +605,10 @@ seastar::future<> SeaStore::do_transaction(
       }).safe_then([this, &ctx] {
 	return onode_manager->write_dirty(*ctx.transaction, ctx.onodes);
       }).safe_then([this, &ctx] {
+        // There are some validations in onode tree during onode value
+        // destruction in debug mode, which need to be done before calling
+        // submit_transaction().
+        ctx.onodes.clear();
 	return transaction_manager->submit_transaction(std::move(ctx.transaction));
       }).safe_then([&ctx]() {
 	for (auto i : {