Skip to content

Commit 1675ce8

Browse files
Matan-Bcyx1231st
andcommitted
crimson/osd/object_context_loader: get_or_load to support atomicity
make use of try_lock in order to support atomicity when called in ObjectContext::_with_lock() Co-authored-by: Yingxin Cheng <[email protected]> Signed-off-by: Matan Breizman <[email protected]>
1 parent f63d76a commit 1675ce8

File tree

2 files changed

+52
-14
lines changed

2 files changed

+52
-14
lines changed

src/crimson/osd/object_context_loader.cc

Lines changed: 40 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -150,20 +150,46 @@ using crimson::common::local_conf;
150150
dpp, obc->get_oid(),
151151
obc->fully_loaded,
152152
obc->invalidated_by_interval_change);
153-
return interruptor::with_lock(obc->loading_mutex,
154-
[this, obc, existed, FNAME] {
155-
if (existed) {
156-
ceph_assert(obc->is_valid() && obc->is_loaded());
157-
DEBUGDPP("cache hit on {}", dpp, obc->get_oid());
158-
return load_obc_iertr::make_ready_future<ObjectContextRef>(obc);
159-
} else {
160-
DEBUGDPP("cache miss on {}", dpp, obc->get_oid());
161-
return obc->template with_promoted_lock<State, IOInterruptCondition>(
162-
[obc, this] {
163-
return load_obc(obc);
164-
});
165-
}
166-
});
153+
if (existed) {
154+
// obc is already loaded - avoid loading_mutex usage
155+
DEBUGDPP("cache hit on {}", dpp, obc->get_oid());
156+
return get_obc(obc, existed);
157+
}
158+
// See ObjectContext::_with_lock(),
159+
// this function must be able to support atomicity before
160+
// acquiring the lock
161+
if (obc->loading_mutex.try_lock()) {
162+
return _get_or_load_obc<State>(obc, existed
163+
).finally([obc]{
164+
obc->loading_mutex.unlock();
165+
});
166+
} else {
167+
return interruptor::with_lock(obc->loading_mutex,
168+
[this, obc, existed, FNAME] {
169+
// Previous user already loaded the obc
170+
DEBUGDPP("{} finished waiting for loader, cache hit on {}",
171+
dpp, FNAME, obc->get_oid());
172+
return get_obc(obc, existed);
173+
});
174+
}
175+
}
176+
177+
template<RWState::State State>
178+
ObjectContextLoader::load_obc_iertr::future<ObjectContextRef>
179+
ObjectContextLoader::_get_or_load_obc(ObjectContextRef obc,
180+
bool existed)
181+
{
182+
LOG_PREFIX(ObjectContextLoader::_get_or_load_obc);
183+
if (existed) {
184+
DEBUGDPP("cache hit on {}", dpp, obc->get_oid());
185+
return get_obc(obc, existed);
186+
} else {
187+
DEBUGDPP("cache miss on {}", dpp, obc->get_oid());
188+
return obc->template with_promoted_lock<State, IOInterruptCondition>(
189+
[obc, this] {
190+
return load_obc(obc);
191+
});
192+
}
167193
}
168194

169195
ObjectContextLoader::load_obc_iertr::future<>

src/crimson/osd/object_context_loader.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,18 @@ class ObjectContextLoader {
8080
get_or_load_obc(ObjectContextRef obc,
8181
bool existed);
8282

83+
template<RWState::State State>
84+
load_obc_iertr::future<ObjectContextRef>
85+
_get_or_load_obc(ObjectContextRef obc,
86+
bool existed);
87+
88+
static inline load_obc_iertr::future<ObjectContextRef>
89+
get_obc(ObjectContextRef obc,
90+
bool existed) {
91+
ceph_assert(existed && obc->is_valid() && obc->is_loaded());
92+
return load_obc_iertr::make_ready_future<ObjectContextRef>(obc);
93+
}
94+
8395
load_obc_iertr::future<ObjectContextRef>
8496
load_obc(ObjectContextRef obc);
8597
};

0 commit comments

Comments
 (0)