From 7aa972def9f53fd9ed2f708aa87ce83362765cbf Mon Sep 17 00:00:00 2001
From: Xuehan Xu <xuxuehan@qianxin.com>
Date: Mon, 18 Mar 2024 10:54:24 +0800
Subject: [PATCH] crimson/os/seastore/btree: check for reserved ptrs when
 determining children stability

Fixes: https://tracker.ceph.com/issues/64957
Signed-off-by: Xuehan Xu <xuxuehan@qianxin.com>
---
 src/crimson/os/seastore/btree/fixed_kv_btree.h |  8 ++++++--
 src/crimson/os/seastore/btree/fixed_kv_node.cc |  6 +++++-
 src/crimson/os/seastore/btree/fixed_kv_node.h  | 11 ++++++++---
 src/crimson/os/seastore/cached_extent.h        |  2 ++
 4 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/src/crimson/os/seastore/btree/fixed_kv_btree.h b/src/crimson/os/seastore/btree/fixed_kv_btree.h
index 2d758ef4be2d8..f15682621fb77 100644
--- a/src/crimson/os/seastore/btree/fixed_kv_btree.h
+++ b/src/crimson/os/seastore/btree/fixed_kv_btree.h
@@ -15,8 +15,6 @@
 #include "crimson/os/seastore/btree/btree_range_pin.h"
 #include "crimson/os/seastore/root_block.h"
 
-#define RESERVATION_PTR reinterpret_cast<ChildableCachedExtent*>(0x1)
-
 namespace crimson::os::seastore::lba_manager::btree {
 struct lba_map_val_t;
 }
@@ -25,6 +23,12 @@ namespace crimson::os::seastore {
 
 bool is_valid_child_ptr(ChildableCachedExtent* child);
 
+bool is_reserved_ptr(ChildableCachedExtent* child);
+
+inline ChildableCachedExtent* get_reserved_ptr() {
+  return (ChildableCachedExtent*)0x1;
+}
+
 template <typename T>
 phy_tree_root_t& get_phy_tree_root(root_t& r);
 
diff --git a/src/crimson/os/seastore/btree/fixed_kv_node.cc b/src/crimson/os/seastore/btree/fixed_kv_node.cc
index 00aceab92b382..94783a0109105 100644
--- a/src/crimson/os/seastore/btree/fixed_kv_node.cc
+++ b/src/crimson/os/seastore/btree/fixed_kv_node.cc
@@ -6,7 +6,11 @@
 namespace crimson::os::seastore {
 
 bool is_valid_child_ptr(ChildableCachedExtent* child) {
-  return child != nullptr && child != RESERVATION_PTR;
+  return child != nullptr && child != get_reserved_ptr();
+}
+
+bool is_reserved_ptr(ChildableCachedExtent* child) {
+  return child == get_reserved_ptr();
 }
 
 } // namespace crimson::os::seastore
diff --git a/src/crimson/os/seastore/btree/fixed_kv_node.h b/src/crimson/os/seastore/btree/fixed_kv_node.h
index 544727f30aac8..de3569c0c5bcf 100644
--- a/src/crimson/os/seastore/btree/fixed_kv_node.h
+++ b/src/crimson/os/seastore/btree/fixed_kv_node.h
@@ -134,7 +134,7 @@ struct FixedKVNode : ChildableCachedExtent {
       // copy sources when committing this lba node, because
       // we rely on pointers' "nullness" to avoid copying
       // pointers for updated values
-      children[offset] = RESERVATION_PTR;
+      children[offset] = get_reserved_ptr();
     }
   }
 
@@ -431,7 +431,7 @@ struct FixedKVNode : ChildableCachedExtent {
 	if (!child) {
 	  child = source.children[foreign_it.get_offset()];
 	  // child can be either valid if present, nullptr if absent,
-	  // or RESERVATION_PTR.
+	  // or reserved ptr.
 	}
 	foreign_it++;
 	local_it++;
@@ -972,6 +972,7 @@ struct FixedKVLeafNode
   get_child_ret_t<LogicalCachedExtent>
   get_logical_child(op_context_t<NODE_KEY> c, uint16_t pos) final {
     auto child = this->children[pos];
+    ceph_assert(!is_reserved_ptr(child));
     if (is_valid_child_ptr(child)) {
       ceph_assert(child->is_logical());
       return c.cache.template get_extent_viewable_by_trans<
@@ -996,9 +997,13 @@ struct FixedKVLeafNode
   // children are considered stable if any of the following case is true:
   // 1. The child extent is absent in cache
   // 2. The child extent is stable
+  //
+  // For reserved mappings, the return values are undefined.
   bool is_child_stable(uint16_t pos) const final {
     auto child = this->children[pos];
-    if (is_valid_child_ptr(child)) {
+    if (is_reserved_ptr(child)) {
+      return true;
+    } else if (is_valid_child_ptr(child)) {
       ceph_assert(child->is_logical());
       return child->is_stable();
     } else if (this->is_pending()) {
diff --git a/src/crimson/os/seastore/cached_extent.h b/src/crimson/os/seastore/cached_extent.h
index 35753d101bd25..18ddbd95796c3 100644
--- a/src/crimson/os/seastore/cached_extent.h
+++ b/src/crimson/os/seastore/cached_extent.h
@@ -1062,6 +1062,8 @@ public:
     child_pos->link_child(c);
   }
 
+  // For reserved mappings, the return values are
+  // undefined although it won't crash
   virtual bool is_stable() const = 0;
   virtual bool is_clone() const = 0;
   bool is_zero_reserved() const {
-- 
2.39.5