Skip to content

Commit 2c3a3e3

Browse files
committed
crimson/os/seastore/cached_extent: add "seen_by_users" to LogicalCachedExtent
This indicates whether the extent has been seen by user transactions. This is required to implement the accurate init callback which is guaranteed be called only once. Signed-off-by: Yingxin Cheng <[email protected]> Signed-off-by: Xuehan Xu <[email protected]>
1 parent 55e7b4c commit 2c3a3e3

File tree

5 files changed

+85
-11
lines changed

5 files changed

+85
-11
lines changed

src/crimson/os/seastore/cache.cc

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1151,13 +1151,19 @@ CachedExtentRef Cache::duplicate_for_write(
11511151
LOG_PREFIX(Cache::duplicate_for_write);
11521152
assert(i->is_fully_loaded());
11531153

1154+
#ifndef NDEBUG
1155+
if (i->is_logical()) {
1156+
assert(static_cast<LogicalCachedExtent&>(*i).has_laddr());
1157+
assert(static_cast<LogicalCachedExtent&>(*i).is_seen_by_users());
1158+
}
1159+
#endif
1160+
11541161
if (i->is_mutable()) {
11551162
return i;
11561163
}
11571164

11581165
if (i->is_exist_clean()) {
11591166
assert(i->is_logical());
1160-
assert(static_cast<LogicalCachedExtent&>(*i).has_laddr());
11611167
i->version++;
11621168
i->state = CachedExtent::extent_state_t::EXIST_MUTATION_PENDING;
11631169
i->last_committed_crc = i->calc_crc32c();
@@ -1188,10 +1194,11 @@ CachedExtentRef Cache::duplicate_for_write(
11881194
ret->version++;
11891195
ret->state = CachedExtent::extent_state_t::MUTATION_PENDING;
11901196
if (i->is_logical()) {
1191-
auto& lextent = static_cast<LogicalCachedExtent&>(*i);
1192-
assert(lextent.has_laddr());
1197+
auto& stable_extent = static_cast<LogicalCachedExtent&>(*i);
11931198
assert(ret->is_logical());
1194-
static_cast<LogicalCachedExtent&>(*ret).set_laddr(lextent.get_laddr());
1199+
auto& mutate_extent = static_cast<LogicalCachedExtent&>(*ret);
1200+
mutate_extent.set_laddr(stable_extent.get_laddr());
1201+
assert(mutate_extent.is_seen_by_users());
11951202
}
11961203

11971204
t.add_mutated_extent(ret);

src/crimson/os/seastore/cached_extent.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,8 @@ CachedExtent* CachedExtent::get_transactional_view(transaction_id_t tid) {
9191

9292
std::ostream &LogicalCachedExtent::print_detail(std::ostream &out) const
9393
{
94-
out << ", laddr=" << laddr;
94+
out << ", laddr=" << laddr
95+
<< ", seen=" << seen_by_users;
9596
return print_detail_l(out);
9697
}
9798

src/crimson/os/seastore/cached_extent.h

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -857,10 +857,12 @@ class CachedExtent
857857

858858
/// used to wait while in-progress commit completes
859859
std::optional<seastar::shared_promise<>> io_wait_promise;
860+
860861
void set_io_wait() {
861862
ceph_assert(!io_wait_promise);
862863
io_wait_promise = seastar::shared_promise<>();
863864
}
865+
864866
void complete_io() {
865867
ceph_assert(io_wait_promise);
866868
io_wait_promise->set_value();
@@ -1385,18 +1387,37 @@ class LBAMapping;
13851387
*/
13861388
class LogicalCachedExtent : public CachedExtent {
13871389
public:
1388-
template <typename... T>
1389-
LogicalCachedExtent(T&&... t) : CachedExtent(std::forward<T>(t)...) {}
1390+
using CachedExtent::CachedExtent;
1391+
1392+
LogicalCachedExtent(const LogicalCachedExtent& other)
1393+
: CachedExtent(other),
1394+
seen_by_users(other.seen_by_users) {}
1395+
1396+
LogicalCachedExtent(const LogicalCachedExtent& other, share_buffer_t s)
1397+
: CachedExtent(other, s),
1398+
seen_by_users(other.seen_by_users) {}
13901399

13911400
void on_rewrite(Transaction &t, CachedExtent &extent, extent_len_t off) final {
13921401
assert(get_type() == extent.get_type());
13931402
auto &lextent = (LogicalCachedExtent&)extent;
13941403
set_laddr((lextent.get_laddr() + off).checked_to_laddr());
1404+
seen_by_users = lextent.seen_by_users;
13951405
do_on_rewrite(t, lextent);
13961406
}
13971407

13981408
virtual void do_on_rewrite(Transaction &t, LogicalCachedExtent &extent) {}
13991409

1410+
bool is_seen_by_users() const {
1411+
return seen_by_users;
1412+
}
1413+
1414+
// handled under TransactionManager,
1415+
// user should not set this by themselves.
1416+
void set_seen_by_users() {
1417+
assert(!seen_by_users);
1418+
seen_by_users = true;
1419+
}
1420+
14001421
bool has_laddr() const {
14011422
return laddr != L_ADDR_NULL;
14021423
}
@@ -1461,6 +1482,12 @@ class LogicalCachedExtent : public CachedExtent {
14611482
// the logical address of the extent, and if shared,
14621483
// it is the intermediate_base, see BtreeLBAMapping comments.
14631484
laddr_t laddr = L_ADDR_NULL;
1485+
1486+
// whether the extent has been seen by users since OSD startup,
1487+
// otherwise, an extent can still be alive if it's dirty or pinned by lru.
1488+
//
1489+
// user must initialize the logical extent upon the first access.
1490+
bool seen_by_users = false;
14641491
};
14651492

14661493
using LogicalCachedExtentRef = TCachedExtentRef<LogicalCachedExtent>;

src/crimson/os/seastore/seastore_types.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2197,6 +2197,10 @@ constexpr bool is_valid_transaction(transaction_type_t type) {
21972197
return type < transaction_type_t::MAX;
21982198
}
21992199

2200+
constexpr bool is_user_transaction(transaction_type_t type) {
2201+
return type <= transaction_type_t::READ;
2202+
}
2203+
22002204
constexpr bool is_background_transaction(transaction_type_t type) {
22012205
return (type >= transaction_type_t::TRIM_DIRTY &&
22022206
type < transaction_type_t::MAX);

src/crimson/os/seastore/transaction_manager.h

Lines changed: 39 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,8 @@ class TransactionManager : public ExtentCallbackInterface {
260260
static_assert(is_logical_type(T::TYPE));
261261
assert(is_aligned(partial_off, get_block_size()));
262262
assert(is_aligned(partial_len, get_block_size()));
263+
// must be user-oriented required by maybe_init
264+
assert(is_user_transaction(t.get_src()));
263265

264266
extent_len_t direct_partial_off = partial_off;
265267
bool is_clone = pin->is_clone();
@@ -302,9 +304,12 @@ class TransactionManager : public ExtentCallbackInterface {
302304
return cache->read_extent_maybe_partial(
303305
t, std::move(extent), direct_partial_off, partial_len);
304306
}).si_then([maybe_init=std::move(maybe_init)](auto extent) {
305-
maybe_init(*extent);
306-
return extent;
307-
});
307+
if (!extent->is_seen_by_users()) {
308+
maybe_init(*extent);
309+
extent->set_seen_by_users();
310+
}
311+
return std::move(extent);
312+
});
308313
} else {
309314
return this->pin_to_extent<T>(
310315
t, std::move(std::get<0>(ret)),
@@ -381,6 +386,7 @@ class TransactionManager : public ExtentCallbackInterface {
381386
laddr_t laddr_hint,
382387
extent_len_t len,
383388
placement_hint_t placement_hint = placement_hint_t::HOT) {
389+
static_assert(is_logical_metadata_type(T::TYPE));
384390
LOG_PREFIX(TransactionManager::alloc_non_data_extent);
385391
SUBDEBUGT(seastore_tm, "{} hint {}~0x{:x} phint={} ...",
386392
t, T::TYPE, laddr_hint, len, placement_hint);
@@ -389,6 +395,9 @@ class TransactionManager : public ExtentCallbackInterface {
389395
len,
390396
placement_hint,
391397
INIT_GENERATION);
398+
// user must initialize the logical extent themselves.
399+
assert(is_user_transaction(t.get_src()));
400+
ext->set_seen_by_users();
392401
return lba_manager->alloc_extent(
393402
t,
394403
laddr_hint,
@@ -416,6 +425,7 @@ class TransactionManager : public ExtentCallbackInterface {
416425
laddr_t laddr_hint,
417426
extent_len_t len,
418427
placement_hint_t placement_hint = placement_hint_t::HOT) {
428+
static_assert(is_data_type(T::TYPE));
419429
LOG_PREFIX(TransactionManager::alloc_data_extents);
420430
SUBDEBUGT(seastore_tm, "{} hint {}~0x{:x} phint={} ...",
421431
t, T::TYPE, laddr_hint, len, placement_hint);
@@ -424,6 +434,11 @@ class TransactionManager : public ExtentCallbackInterface {
424434
len,
425435
placement_hint,
426436
INIT_GENERATION);
437+
// user must initialize the logical extent themselves
438+
assert(is_user_transaction(t.get_src()));
439+
for (auto& ext : exts) {
440+
ext->set_seen_by_users();
441+
}
427442
return lba_manager->alloc_extents(
428443
t,
429444
laddr_hint,
@@ -479,6 +494,10 @@ class TransactionManager : public ExtentCallbackInterface {
479494
LBAMappingRef &&pin,
480495
std::array<remap_entry, N> remaps) {
481496
static_assert(std::is_base_of_v<LogicalChildNode, T>);
497+
// data extents don't need maybe_init yet, currently,
498+
static_assert(is_data_type(T::TYPE));
499+
// must be user-oriented required by (the potential) maybe_init
500+
assert(is_user_transaction(t.get_src()));
482501

483502
#ifndef NDEBUG
484503
std::sort(remaps.begin(), remaps.end(),
@@ -550,7 +569,14 @@ class TransactionManager : public ExtentCallbackInterface {
550569
} else {
551570
auto ret = get_extent_if_linked<T>(t, pin->duplicate());
552571
if (ret.index() == 1) {
553-
return std::move(std::get<1>(ret));
572+
return std::get<1>(ret
573+
).si_then([](auto extent) {
574+
if (!extent->is_seen_by_users()) {
575+
// Note, no maybe_init available for data extents
576+
extent->set_seen_by_users();
577+
}
578+
return std::move(extent);
579+
});
554580
} else {
555581
// absent
556582
return base_iertr::make_ready_future<TCachedExtentRef<T>>();
@@ -571,6 +597,7 @@ class TransactionManager : public ExtentCallbackInterface {
571597
original_bptr = ext->get_bptr();
572598
}
573599
if (ext) {
600+
assert(ext->is_seen_by_users());
574601
cache->retire_extent(t, ext);
575602
} else {
576603
cache->retire_absent_extent_addr(t, original_paddr, original_len);
@@ -594,6 +621,8 @@ class TransactionManager : public ExtentCallbackInterface {
594621
remap_len,
595622
original_laddr,
596623
original_bptr);
624+
// user must initialize the logical extent themselves.
625+
extent->set_seen_by_users();
597626
extents.emplace_back(std::move(extent));
598627
}
599628
});
@@ -1034,6 +1063,8 @@ class TransactionManager : public ExtentCallbackInterface {
10341063
extent_len_t partial_len,
10351064
lextent_init_func_t<T> &&maybe_init) {
10361065
static_assert(is_logical_type(T::TYPE));
1066+
// must be user-oriented required by maybe_init
1067+
assert(is_user_transaction(t.get_src()));
10371068
using ret = pin_to_extent_ret<T>;
10381069
auto &pref = *pin;
10391070
auto direct_length = pref.is_indirect() ?
@@ -1062,6 +1093,7 @@ class TransactionManager : public ExtentCallbackInterface {
10621093
pref.link_child(&extent);
10631094
extent.maybe_set_intermediate_laddr(pref);
10641095
maybe_init(extent);
1096+
extent.set_seen_by_users();
10651097
}
10661098
).si_then([FNAME, &t, pin=std::move(pin), this](auto ref) mutable -> ret {
10671099
if (ref->is_fully_loaded()) {
@@ -1114,6 +1146,7 @@ class TransactionManager : public ExtentCallbackInterface {
11141146
SUBTRACET(seastore_tm, "getting absent extent from pin {} type {} ...",
11151147
t, *pin, type);
11161148
assert(is_logical_type(type));
1149+
assert(is_background_transaction(t.get_src()));
11171150
auto &pref = *pin;
11181151
laddr_t direct_key;
11191152
extent_len_t direct_length;
@@ -1140,6 +1173,8 @@ class TransactionManager : public ExtentCallbackInterface {
11401173
assert(!pref.get_parent()->is_pending());
11411174
pref.link_child(&lextent);
11421175
lextent.maybe_set_intermediate_laddr(pref);
1176+
// No change to extent::seen_by_user because this path is only
1177+
// for background cleaning.
11431178
}
11441179
).si_then([FNAME, &t, pin=std::move(pin), this](auto ref) {
11451180
auto crc = ref->calc_crc32c();

0 commit comments

Comments
 (0)