Skip to content

Commit 513369c

Browse files
committed
crimson/os/seastore: more accurate usages to is_pending() vs is_stable()
Generally, prefer is_pending() than !is_stable(), and is_stable() than !is_pending(). Signed-off-by: Yingxin Cheng <[email protected]>
1 parent 6f892fe commit 513369c

File tree

7 files changed

+28
-28
lines changed

7 files changed

+28
-28
lines changed

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1192,7 +1192,7 @@ class FixedKVBtree {
11921192
parent_pos=std::move(parent_pos)]
11931193
(internal_node_t &node) {
11941194
using tree_root_linker_t = TreeRootLinker<RootBlock, internal_node_t>;
1195-
assert(!node.is_pending());
1195+
assert(node.is_stable());
11961196
assert(!node.is_linked());
11971197
node.range = fixed_kv_node_meta_t<node_key_t>{begin, end, depth};
11981198
if (parent_pos) {
@@ -1205,7 +1205,7 @@ class FixedKVBtree {
12051205
auto &stable_root = (RootBlockRef&)*root_block->get_prior_instance();
12061206
tree_root_linker_t::link_root(stable_root, &node);
12071207
} else {
1208-
assert(!root_block->is_pending());
1208+
assert(root_block->is_stable());
12091209
tree_root_linker_t::link_root(root_block, &node);
12101210
}
12111211
}
@@ -1236,7 +1236,7 @@ class FixedKVBtree {
12361236
*ret);
12371237
// This can only happen during init_cached_extent
12381238
// or when backref extent being rewritten by gc space reclaiming
1239-
if (!ret->is_pending() && !ret->is_linked()) {
1239+
if (ret->is_stable() && !ret->is_linked()) {
12401240
assert(ret->has_delta() || is_backref_node(ret->get_type()));
12411241
init_internal(*ret);
12421242
}
@@ -1276,7 +1276,7 @@ class FixedKVBtree {
12761276
parent_pos=std::move(parent_pos)]
12771277
(leaf_node_t &node) {
12781278
using tree_root_linker_t = TreeRootLinker<RootBlock, leaf_node_t>;
1279-
assert(!node.is_pending());
1279+
assert(node.is_stable());
12801280
assert(!node.is_linked());
12811281
node.range = fixed_kv_node_meta_t<node_key_t>{begin, end, 1};
12821282
if (parent_pos) {
@@ -1289,7 +1289,7 @@ class FixedKVBtree {
12891289
auto &stable_root = (RootBlockRef&)*root_block->get_prior_instance();
12901290
tree_root_linker_t::link_root(stable_root, &node);
12911291
} else {
1292-
assert(!root_block->is_pending());
1292+
assert(root_block->is_stable());
12931293
tree_root_linker_t::link_root(root_block, &node);
12941294
}
12951295
}
@@ -1319,7 +1319,7 @@ class FixedKVBtree {
13191319
*ret);
13201320
// This can only happen during init_cached_extent
13211321
// or when backref extent being rewritten by gc space reclaiming
1322-
if (!ret->is_pending() && !ret->is_linked()) {
1322+
if (ret->is_stable() && !ret->is_linked()) {
13231323
assert(ret->has_delta() || is_backref_node(ret->get_type()));
13241324
init_leaf(*ret);
13251325
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ struct FixedKVInternalNode
182182
}
183183

184184
virtual ~FixedKVInternalNode() {
185-
if (this->is_valid() && !this->is_pending()) {
185+
if (this->is_stable()) {
186186
if (this->is_btree_root()) {
187187
this->root_node_t::destroy();
188188
} else {
@@ -571,7 +571,7 @@ struct FixedKVLeafNode
571571
}
572572

573573
virtual ~FixedKVLeafNode() {
574-
if (this->is_valid() && !this->is_pending()) {
574+
if (this->is_stable()) {
575575
if (this->is_btree_root()) {
576576
this->root_node_t::destroy();
577577
} else {

src/crimson/os/seastore/cached_extent.h

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -436,7 +436,8 @@ class CachedExtent
436436

437437
void rewrite(Transaction &t, CachedExtent &e, extent_len_t o) {
438438
assert(is_initial_pending());
439-
if (!e.is_pending()) {
439+
assert(e.is_valid());
440+
if (e.is_stable()) {
440441
set_prior_instance(&e);
441442
} else {
442443
assert(e.has_mutation());
@@ -550,7 +551,8 @@ class CachedExtent
550551
state == extent_state_t::EXIST_MUTATION_PENDING;
551552
}
552553

553-
/// Returns true if extent is part of an open transaction
554+
/// Returns true if extent is part of an open transaction,
555+
/// normally equivalent to !is_stable.
554556
bool is_pending() const {
555557
return is_mutable() || state == extent_state_t::EXIST_CLEAN;
556558
}
@@ -574,7 +576,8 @@ class CachedExtent
574576
return (has_mutation() || is_initial_pending()) && is_pending_io();
575577
}
576578

577-
/// Returns true if extent is stable and shared among transactions
579+
/// Returns true if extent is stable and shared among transactions,
580+
/// normally equivalent to !is_pending
578581
bool is_stable() const {
579582
return is_stable_written() || is_stable_writting();
580583
}

src/crimson/os/seastore/lba/btree_lba_manager.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -490,7 +490,7 @@ _init_cached_extent(
490490
iter.get_key() == logn->get_laddr() &&
491491
iter.get_val().pladdr.is_paddr() &&
492492
iter.get_val().pladdr.get_paddr() == logn->get_paddr()) {
493-
assert(!iter.get_leaf_node()->is_pending());
493+
assert(iter.get_leaf_node()->is_stable());
494494
iter.get_leaf_node()->link_child(logn.get(), iter.get_leaf_pos());
495495
logn->set_laddr(iter.get_key());
496496
ceph_assert(iter.get_val().len == e->get_length());

src/crimson/os/seastore/linked_tree_node.h

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -206,8 +206,8 @@ class BaseChildNode {
206206
virtual key_t node_begin() const = 0;
207207
protected:
208208
parent_tracker_ref<ParentT> parent_tracker;
209-
virtual bool valid() const = 0;
210-
virtual bool pending() const = 0;
209+
virtual bool _is_valid() const = 0;
210+
virtual bool _is_stable() const = 0;
211211
template <typename, typename, typename>
212212
friend class ParentNode;
213213
};
@@ -360,8 +360,8 @@ class ParentNode {
360360
assert(pos < me.get_size());
361361
assert(pos < children.capacity());
362362
assert(child);
363-
ceph_assert(!me.is_pending());
364-
assert(child->valid() && !child->pending());
363+
ceph_assert(me.is_stable());
364+
assert(child->_is_stable());
365365
assert(!children[pos]);
366366
ceph_assert(is_valid_child_ptr(child));
367367
update_child_ptr(pos, child);
@@ -460,7 +460,7 @@ class ParentNode {
460460

461461
void on_rewrite(Transaction &t, T &foreign_extent) {
462462
auto &me = down_cast();
463-
if (!foreign_extent.is_pending()) {
463+
if (foreign_extent.is_stable()) {
464464
foreign_extent.add_copy_dest(t, &me);
465465
copy_sources.emplace(&foreign_extent);
466466
} else {
@@ -508,7 +508,7 @@ class ParentNode {
508508
T &src)
509509
{
510510
ceph_assert(dest.is_initial_pending());
511-
if (!src.is_pending()) {
511+
if (src.is_stable()) {
512512
src.add_copy_dest(t, &dest);
513513
dest.copy_sources.emplace(&src);
514514
} else if (src.is_mutation_pending()) {
@@ -713,7 +713,7 @@ class ParentNode {
713713
for (auto it = children.begin();
714714
it != children.begin() + down_cast().get_size();
715715
it++) {
716-
if (is_valid_child_ptr(*it) && (*it)->valid()) {
716+
if (is_valid_child_ptr(*it) && (*it)->_is_valid()) {
717717
return false;
718718
}
719719
}
@@ -1041,11 +1041,11 @@ class ChildNode : public BaseChildNode<ParentT, key_t> {
10411041
assert(iter.get_key() == me.get_begin());
10421042
return iter.get_offset();
10431043
}
1044-
bool valid() const final {
1044+
bool _is_valid() const final {
10451045
return down_cast().is_valid();
10461046
}
1047-
bool pending() const final {
1048-
return down_cast().is_pending();
1047+
bool _is_stable() const final {
1048+
return down_cast().is_stable();
10491049
}
10501050
key_t node_begin() const final {
10511051
return down_cast().get_begin();

src/crimson/os/seastore/logical_child_node.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,7 @@ class LogicalChildNode : public LogicalCachedExtent,
2121
LogicalChildNode(T&&... t) : LogicalCachedExtent(std::forward<T>(t)...) {}
2222

2323
virtual ~LogicalChildNode() {
24-
if (this->is_valid() &&
25-
!this->is_pending()) {
24+
if (this->is_stable()) {
2625
lba_child_node_t::destroy();
2726
}
2827
}

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

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -250,8 +250,7 @@ struct OMapInnerNode
250250
}
251251

252252
~OMapInnerNode() {
253-
if (this->is_valid()
254-
&& !this->is_pending()
253+
if (this->is_stable()
255254
&& !this->is_btree_root()
256255
// dirty omap extent may not be accessed/linked yet
257256
&& this->base_child_t::has_parent_tracker()) {
@@ -352,8 +351,7 @@ struct OMapLeafNode
352351
}
353352

354353
~OMapLeafNode() {
355-
if (this->is_valid()
356-
&& !this->is_pending()
354+
if (this->is_stable()
357355
&& !this->is_btree_root()
358356
// dirty omap extent may not be accessed/linked yet
359357
&& this->base_child_t::has_parent_tracker()) {

0 commit comments

Comments
 (0)