Skip to content

Commit 7aa972d

Browse files
committed
crimson/os/seastore/btree: check for reserved ptrs when determining
children stability Fixes: https://tracker.ceph.com/issues/64957 Signed-off-by: Xuehan Xu <[email protected]>
1 parent bb726d3 commit 7aa972d

File tree

4 files changed

+21
-6
lines changed

4 files changed

+21
-6
lines changed

src/crimson/os/seastore/btree/fixed_kv_btree.h

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,6 @@
1515
#include "crimson/os/seastore/btree/btree_range_pin.h"
1616
#include "crimson/os/seastore/root_block.h"
1717

18-
#define RESERVATION_PTR reinterpret_cast<ChildableCachedExtent*>(0x1)
19-
2018
namespace crimson::os::seastore::lba_manager::btree {
2119
struct lba_map_val_t;
2220
}
@@ -25,6 +23,12 @@ namespace crimson::os::seastore {
2523

2624
bool is_valid_child_ptr(ChildableCachedExtent* child);
2725

26+
bool is_reserved_ptr(ChildableCachedExtent* child);
27+
28+
inline ChildableCachedExtent* get_reserved_ptr() {
29+
return (ChildableCachedExtent*)0x1;
30+
}
31+
2832
template <typename T>
2933
phy_tree_root_t& get_phy_tree_root(root_t& r);
3034

src/crimson/os/seastore/btree/fixed_kv_node.cc

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,11 @@
66
namespace crimson::os::seastore {
77

88
bool is_valid_child_ptr(ChildableCachedExtent* child) {
9-
return child != nullptr && child != RESERVATION_PTR;
9+
return child != nullptr && child != get_reserved_ptr();
10+
}
11+
12+
bool is_reserved_ptr(ChildableCachedExtent* child) {
13+
return child == get_reserved_ptr();
1014
}
1115

1216
} // namespace crimson::os::seastore

src/crimson/os/seastore/btree/fixed_kv_node.h

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ struct FixedKVNode : ChildableCachedExtent {
134134
// copy sources when committing this lba node, because
135135
// we rely on pointers' "nullness" to avoid copying
136136
// pointers for updated values
137-
children[offset] = RESERVATION_PTR;
137+
children[offset] = get_reserved_ptr();
138138
}
139139
}
140140

@@ -431,7 +431,7 @@ struct FixedKVNode : ChildableCachedExtent {
431431
if (!child) {
432432
child = source.children[foreign_it.get_offset()];
433433
// child can be either valid if present, nullptr if absent,
434-
// or RESERVATION_PTR.
434+
// or reserved ptr.
435435
}
436436
foreign_it++;
437437
local_it++;
@@ -972,6 +972,7 @@ struct FixedKVLeafNode
972972
get_child_ret_t<LogicalCachedExtent>
973973
get_logical_child(op_context_t<NODE_KEY> c, uint16_t pos) final {
974974
auto child = this->children[pos];
975+
ceph_assert(!is_reserved_ptr(child));
975976
if (is_valid_child_ptr(child)) {
976977
ceph_assert(child->is_logical());
977978
return c.cache.template get_extent_viewable_by_trans<
@@ -996,9 +997,13 @@ struct FixedKVLeafNode
996997
// children are considered stable if any of the following case is true:
997998
// 1. The child extent is absent in cache
998999
// 2. The child extent is stable
1000+
//
1001+
// For reserved mappings, the return values are undefined.
9991002
bool is_child_stable(uint16_t pos) const final {
10001003
auto child = this->children[pos];
1001-
if (is_valid_child_ptr(child)) {
1004+
if (is_reserved_ptr(child)) {
1005+
return true;
1006+
} else if (is_valid_child_ptr(child)) {
10021007
ceph_assert(child->is_logical());
10031008
return child->is_stable();
10041009
} else if (this->is_pending()) {

src/crimson/os/seastore/cached_extent.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1062,6 +1062,8 @@ class PhysicalNodeMapping {
10621062
child_pos->link_child(c);
10631063
}
10641064

1065+
// For reserved mappings, the return values are
1066+
// undefined although it won't crash
10651067
virtual bool is_stable() const = 0;
10661068
virtual bool is_clone() const = 0;
10671069
bool is_zero_reserved() const {

0 commit comments

Comments
 (0)