Skip to content

Commit 7b889ea

Browse files
committed
crimson/os/seastore: introduce strict paddr type checks in cache and transaction
Signed-off-by: Yingxin Cheng <[email protected]>
1 parent 59907c3 commit 7b889ea

File tree

3 files changed

+29
-0
lines changed

3 files changed

+29
-0
lines changed

src/crimson/os/seastore/cache.cc

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -731,11 +731,14 @@ void Cache::add_extent(CachedExtentRef ref)
731731
assert(ref->is_valid());
732732
assert(ref->user_hint == PLACEMENT_HINT_NULL);
733733
assert(ref->rewrite_generation == NULL_GENERATION);
734+
assert(ref->get_paddr().is_absolute() ||
735+
ref->get_paddr().is_root());
734736
extents_index.insert(*ref);
735737
}
736738

737739
void Cache::mark_dirty(CachedExtentRef ref)
738740
{
741+
assert(ref->get_paddr().is_absolute());
739742
if (ref->is_dirty()) {
740743
assert(ref->primary_ref_list_hook.is_linked());
741744
return;
@@ -754,6 +757,8 @@ void Cache::add_to_dirty(
754757
assert(!ref->primary_ref_list_hook.is_linked());
755758
ceph_assert(ref->get_modify_time() != NULL_TIME);
756759
assert(ref->is_fully_loaded());
760+
assert(ref->get_paddr().is_absolute() ||
761+
ref->get_paddr().is_root());
757762

758763
// Note: next might not be at extent_state_t::DIRTY,
759764
// also see CachedExtent::is_stable_writting()
@@ -783,6 +788,8 @@ void Cache::remove_from_dirty(
783788
assert(ref->is_dirty());
784789
ceph_assert(ref->primary_ref_list_hook.is_linked());
785790
assert(ref->is_fully_loaded());
791+
assert(ref->get_paddr().is_absolute() ||
792+
ref->get_paddr().is_root());
786793

787794
auto extent_length = ref->get_length();
788795
stats.dirty_bytes -= extent_length;
@@ -864,9 +871,12 @@ void Cache::remove_extent(
864871
const Transaction::src_t* p_src)
865872
{
866873
assert(ref->is_valid());
874+
assert(ref->get_paddr().is_absolute() ||
875+
ref->get_paddr().is_root());
867876
if (ref->is_dirty()) {
868877
remove_from_dirty(ref, p_src);
869878
} else if (!ref->is_placeholder()) {
879+
assert(ref->get_paddr().is_absolute());
870880
lru.remove_from_lru(*ref);
871881
}
872882
extents_index.erase(*ref);
@@ -889,6 +899,7 @@ void Cache::commit_replace_extent(
889899
CachedExtentRef prev)
890900
{
891901
assert(next->get_paddr() == prev->get_paddr());
902+
assert(next->get_paddr().is_absolute() || next->get_paddr().is_root());
892903
assert(next->version == prev->version + 1);
893904
extents_index.replace(*next, *prev);
894905

@@ -2006,6 +2017,7 @@ Cache::replay_delta(
20062017
if (is_root_type(delta.type)) {
20072018
TRACE("replay root delta at {} {}, remove extent ... -- {}, prv_root={}",
20082019
journal_seq, record_base, delta, *root);
2020+
ceph_assert(delta.paddr.is_root());
20092021
remove_extent(root, nullptr);
20102022
root->apply_delta_and_adjust_crc(record_base, delta.bl);
20112023
root->dirty_from_or_retired_at = journal_seq;
@@ -2019,6 +2031,7 @@ Cache::replay_delta(
20192031
return replay_delta_ertr::make_ready_future<std::pair<bool, CachedExtentRef>>(
20202032
std::make_pair(true, root));
20212033
} else {
2034+
ceph_assert(delta.paddr.is_absolute());
20222035
auto _get_extent_if_cached = [this](paddr_t addr)
20232036
-> get_extent_ertr::future<CachedExtentRef> {
20242037
// replay is not included by the cache hit metrics

src/crimson/os/seastore/cache.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,7 @@ class Cache : public ExtentTransViewRetriever {
255255
});
256256
}
257257

258+
assert(paddr.is_absolute());
258259
// get_extent_ret::ABSENT from transaction
259260
ret = query_cache(paddr);
260261
if (!ret) {
@@ -590,6 +591,7 @@ class Cache : public ExtentTransViewRetriever {
590591
// Interfaces only for tests.
591592
public:
592593
CachedExtentRef test_query_cache(paddr_t offset) {
594+
assert(offset.is_absolute());
593595
return query_cache(offset);
594596
}
595597

@@ -678,6 +680,7 @@ class Cache : public ExtentTransViewRetriever {
678680
const Transaction::src_t* p_src
679681
) {
680682
LOG_PREFIX(Cache::do_get_caching_extent);
683+
assert(offset.is_absolute());
681684
auto cached = query_cache(offset);
682685
if (!cached) {
683686
// partial read
@@ -1484,6 +1487,7 @@ class Cache : public ExtentTransViewRetriever {
14841487
const Transaction::src_t* p_src,
14851488
cache_hint_t hint)
14861489
{
1490+
assert(ext.get_paddr().is_absolute());
14871491
if (hint == CACHE_HINT_NOCACHE && is_logical_type(ext.get_type())) {
14881492
return;
14891493
}
@@ -1919,6 +1923,7 @@ class Cache : public ExtentTransViewRetriever {
19191923
assert(!extent->is_range_loaded(offset, length));
19201924
assert(is_aligned(offset, get_block_size()));
19211925
assert(is_aligned(length, get_block_size()));
1926+
assert(extent->get_paddr().is_absolute());
19221927
extent->set_io_wait();
19231928
auto old_length = extent->get_loaded_length();
19241929
load_ranges_t to_read = extent->load_ranges(offset, length);

src/crimson/os/seastore/transaction.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@ class Transaction {
107107
RETIRED
108108
};
109109
get_extent_ret get_extent(paddr_t addr, CachedExtentRef *out) {
110+
assert(addr.is_real_location() || addr.is_root());
110111
auto [result, ext] = do_get_extent(addr);
111112
// placeholder in read-set must be in the retired-set
112113
// at the same time, user should not see a placeholder.
@@ -119,12 +120,14 @@ class Transaction {
119120
}
120121

121122
void add_absent_to_retired_set(CachedExtentRef ref) {
123+
assert(ref->get_paddr().is_absolute());
122124
bool added = do_add_to_read_set(ref);
123125
ceph_assert(added);
124126
add_present_to_retired_set(ref);
125127
}
126128

127129
void add_present_to_retired_set(CachedExtentRef ref) {
130+
assert(ref->get_paddr().is_real_location());
128131
assert(!is_weak());
129132
#ifndef NDEBUG
130133
auto [result, ext] = do_get_extent(ref->get_paddr());
@@ -156,13 +159,16 @@ class Transaction {
156159

157160
// Returns true if added, false if already added or weak
158161
bool maybe_add_to_read_set(CachedExtentRef ref) {
162+
assert(ref->get_paddr().is_absolute());
159163
if (is_weak()) {
160164
return false;
161165
}
162166
return do_add_to_read_set(ref);
163167
}
164168

165169
void add_to_read_set(CachedExtentRef ref) {
170+
assert(ref->get_paddr().is_absolute()
171+
|| ref->get_paddr().is_root());
166172
if (is_weak()) {
167173
return;
168174
}
@@ -173,6 +179,7 @@ class Transaction {
173179

174180
void add_fresh_extent(
175181
CachedExtentRef ref) {
182+
assert(ref->get_paddr().is_real_location());
176183
ceph_assert(!is_weak());
177184
if (ref->is_exist_clean()) {
178185
existing_block_stats.inc(ref);
@@ -255,6 +262,8 @@ class Transaction {
255262

256263
void add_mutated_extent(CachedExtentRef ref) {
257264
ceph_assert(!is_weak());
265+
assert(ref->get_paddr().is_absolute() ||
266+
ref->get_paddr().is_root());
258267
assert(ref->is_exist_mutation_pending() ||
259268
read_set.count(ref->prior_instance->get_paddr()));
260269
mutated_block_list.push_back(ref);
@@ -273,6 +282,7 @@ class Transaction {
273282
assert(!is_retired_placeholder_type(extent.get_type()));
274283
assert(!is_root_type(extent.get_type()));
275284
assert(extent.get_paddr() == placeholder.get_paddr());
285+
assert(extent.get_paddr().is_absolute());
276286
{
277287
auto where = read_set.find(placeholder.get_paddr());
278288
assert(where != read_set.end());
@@ -327,6 +337,7 @@ class Transaction {
327337
}
328338

329339
bool is_stable_extent_retired(paddr_t paddr, extent_len_t len) {
340+
assert(paddr.is_absolute());
330341
auto iter = retired_set.lower_bound(paddr);
331342
if (iter == retired_set.end()) {
332343
return false;

0 commit comments

Comments
 (0)