Skip to content

Commit 550b2b8

Browse files
committed
crimson/common/smp_helpers: fix reactor_map_seq
Copy f into reactor_map_seq which would be kept alive due to this method being a coroutine. That way, we can ensure the lambdas passed to each core that are capturing f by reference would be safe. Alternatively, we can also copy f by using it's copy ctor and pass a copy to each shard: co_await crimson::submit_to(core, F(f)) However, avoiding the copy is possible here due to the sequential traversal. Note, seastar's invoke_on_all do copy each callback to every shard and is running the invocation in parallel. The above would have fixed f's captures to be invalid and result in a segfaults on diffrent shards. Fixes: https://tracker.ceph.com/issues/71457 Signed-off-by: Matan Breizman <[email protected]>
1 parent c45e18b commit 550b2b8

File tree

1 file changed

+9
-25
lines changed

1 file changed

+9
-25
lines changed

src/crimson/common/smp_helpers.h

Lines changed: 9 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -51,33 +51,17 @@ auto proxy_method_on_core(
5151
*
5252
* Invokes f on each reactor sequentially, Caller may assume that
5353
* f will not be invoked concurrently on multiple cores.
54+
* f is copied here and is kept alive due to coroutine parameter copying.
5455
*/
5556
template <typename F>
56-
auto reactor_map_seq(F &&f) {
57-
using ret_type = decltype(f());
58-
if constexpr (is_errorated_future_v<ret_type>) {
59-
auto ret = crimson::do_for_each(
60-
seastar::smp::all_cpus().begin(),
61-
seastar::smp::all_cpus().end(),
62-
[f=std::move(f)](auto core) mutable {
63-
return seastar::smp::submit_to(
64-
core,
65-
[&f] {
66-
return std::invoke(f);
67-
});
68-
});
69-
return ret_type(ret);
70-
} else {
71-
return seastar::do_for_each(
72-
seastar::smp::all_cpus().begin(),
73-
seastar::smp::all_cpus().end(),
74-
[f=std::move(f)](auto core) mutable {
75-
return seastar::smp::submit_to(
76-
core,
77-
[&f] {
78-
return std::invoke(f);
79-
});
80-
});
57+
auto reactor_map_seq(F f) -> decltype(seastar::futurize_invoke(f)) {
58+
for (auto core: seastar::smp::all_cpus()) {
59+
using ret_type = decltype(f());
60+
if constexpr (is_errorated_future_v<ret_type>) {
61+
co_await crimson::submit_to(core, [&f] { return seastar::futurize_invoke(f);});
62+
} else {
63+
co_await seastar::smp::submit_to(core, [&f] { return seastar::futurize_invoke(f);});
64+
}
8165
}
8266
}
8367

0 commit comments

Comments
 (0)