Skip to content

Commit 84092fa

Browse files
committed
crimson/os/seastore/omap_manager: simplify maybe_init from tolerating duplicated calls
Also added asserts around seen_by_users. Signed-off-by: Yingxin Cheng <[email protected]>
1 parent 2c3a3e3 commit 84092fa

File tree

3 files changed

+25
-14
lines changed

3 files changed

+25
-14
lines changed

src/crimson/os/seastore/logical_child_node.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ class LogicalChildNode : public LogicalCachedExtent,
4040
}
4141
protected:
4242
void on_replace_prior() final {
43+
assert(is_seen_by_users());
4344
lba_child_node_t::on_replace_prior();
4445
do_on_replace_prior();
4546
}

src/crimson/os/seastore/omap_manager/btree/omap_btree_node.h

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -116,17 +116,14 @@ struct OMapNode : LogicalChildNode {
116116

117117
virtual ~OMapNode() = default;
118118

119-
void init_range(std::string begin, std::string end) {
120-
if (!this->end.empty()) {
121-
// this is a duplicated init.
122-
assert(end == this->end);
123-
return;
124-
}
125-
if (begin == BEGIN_KEY && end == END_KEY) {
119+
void init_range(std::string _begin, std::string _end) {
120+
assert(begin.empty());
121+
assert(end.empty());
122+
if (_begin == BEGIN_KEY && _end == END_KEY) {
126123
root = true;
127124
}
128-
this->begin = std::move(begin);
129-
this->end = std::move(end);
125+
begin = std::move(_begin);
126+
end = std::move(_end);
130127
}
131128
const std::string &get_begin() const {
132129
return begin;

src/crimson/os/seastore/omap_manager/btree/omap_btree_node_impl.h

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -80,10 +80,15 @@ struct OMapInnerNode
8080
if (is_rewrite() && !is_btree_root()) {
8181
auto &prior = *get_prior_instance()->template cast<OMapInnerNode>();
8282
if (prior.base_child_t::has_parent_tracker()) {
83+
assert(prior.is_seen_by_users());
8384
// unlike fixed-kv nodes, rewriting child nodes of the omap tree
8485
// won't affect parent nodes, so we have to manually take prior
8586
// instances' parent trackers here.
8687
this->child_node_t::take_parent_from_prior();
88+
} else {
89+
// dirty omap extent may not be accessed yet during rewrite,
90+
// this means the extent may not be initalized yet as linked.
91+
assert(!prior.is_seen_by_users());
8792
}
8893
}
8994
}
@@ -231,6 +236,7 @@ struct OMapInnerNode
231236
if (this->is_valid()
232237
&& !this->is_pending()
233238
&& !this->is_btree_root()
239+
// dirty omap extent may not be accessed/linked yet
234240
&& this->base_child_t::has_parent_tracker()) {
235241
this->child_node_t::destroy();
236242
}
@@ -282,10 +288,15 @@ struct OMapLeafNode
282288
if (is_rewrite() && !is_btree_root()) {
283289
auto &prior = *get_prior_instance()->template cast<OMapLeafNode>();
284290
if (prior.base_child_t::has_parent_tracker()) {
291+
assert(prior.is_seen_by_users());
285292
// unlike fixed-kv nodes, rewriting child nodes of the omap tree
286293
// won't affect parent nodes, so we have to manually take prior
287294
// instances' parent trackers here.
288295
this->child_node_t::take_parent_from_prior();
296+
} else {
297+
// dirty omap extent may not be accessed yet during rewrite,
298+
// this means the extent may not be initalized yet as linked.
299+
assert(!prior.is_seen_by_users());
289300
}
290301
}
291302
}
@@ -303,6 +314,7 @@ struct OMapLeafNode
303314
if (this->is_valid()
304315
&& !this->is_pending()
305316
&& !this->is_btree_root()
317+
// dirty omap extent may not be accessed/linked yet
306318
&& this->base_child_t::has_parent_tracker()) {
307319
this->child_node_t::destroy();
308320
}
@@ -434,14 +446,15 @@ omap_load_extent(
434446
oc.t, laddr, size,
435447
[begin=std::move(begin), end=std::move(end), FNAME,
436448
oc, chp=std::move(chp)](T &extent) mutable {
449+
assert(!extent.is_seen_by_users());
437450
extent.init_range(std::move(begin), std::move(end));
438-
if (extent.T::base_child_t::is_parent_valid()
439-
|| extent.is_btree_root()) {
440-
return;
451+
if (extent.is_btree_root()) {
452+
return;
441453
}
442-
assert(chp);
443454
SUBDEBUGT(seastore_omap, "linking {} to {}",
444-
oc.t, extent, *chp->get_parent());
455+
oc.t, extent, *chp->get_parent());
456+
assert(chp);
457+
assert(!extent.T::base_child_t::is_parent_valid());
445458
chp->link_child(&extent);
446459
}
447460
).handle_error_interruptible(

0 commit comments

Comments
 (0)