Skip to content

Commit d595505

Browse files
committed
librbd: Reduce use of atomics in librbd throttling
Signed-off-by: Adam Lyon-Jones <[email protected]>
1 parent 492a769 commit d595505

File tree

2 files changed

+25
-16
lines changed

2 files changed

+25
-16
lines changed

src/librbd/io/QosImageDispatch.cc

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,8 @@ bool QosImageDispatch<I>::read(
129129
ldout(cct, 20) << "tid=" << tid << ", image_extents=" << image_extents
130130
<< dendl;
131131

132-
if (m_qos_exclude_ops & RBD_IO_OPERATION_READ) {
132+
if (m_qos_exclude_ops & RBD_IO_OPERATION_READ ||
133+
m_qos_enabled_flag == 0) {
133134
return false;
134135
}
135136

@@ -152,7 +153,8 @@ bool QosImageDispatch<I>::write(
152153
ldout(cct, 20) << "tid=" << tid << ", image_extents=" << image_extents
153154
<< dendl;
154155

155-
if (m_qos_exclude_ops & RBD_IO_OPERATION_WRITE) {
156+
if (m_qos_exclude_ops & RBD_IO_OPERATION_WRITE ||
157+
m_qos_enabled_flag == 0) {
156158
return false;
157159
}
158160

@@ -175,7 +177,8 @@ bool QosImageDispatch<I>::discard(
175177
ldout(cct, 20) << "tid=" << tid << ", image_extents=" << image_extents
176178
<< dendl;
177179

178-
if (m_qos_exclude_ops & RBD_IO_OPERATION_DISCARD) {
180+
if (m_qos_exclude_ops & RBD_IO_OPERATION_DISCARD ||
181+
m_qos_enabled_flag == 0) {
179182
return false;
180183
}
181184

@@ -198,7 +201,8 @@ bool QosImageDispatch<I>::write_same(
198201
ldout(cct, 20) << "tid=" << tid << ", image_extents=" << image_extents
199202
<< dendl;
200203

201-
if (m_qos_exclude_ops & RBD_IO_OPERATION_WRITE_SAME) {
204+
if (m_qos_exclude_ops & RBD_IO_OPERATION_WRITE_SAME ||
205+
m_qos_enabled_flag == 0) {
202206
return false;
203207
}
204208

@@ -222,7 +226,8 @@ bool QosImageDispatch<I>::compare_and_write(
222226
ldout(cct, 20) << "tid=" << tid << ", image_extents=" << image_extents
223227
<< dendl;
224228

225-
if (m_qos_exclude_ops & RBD_IO_OPERATION_COMPARE_AND_WRITE) {
229+
if (m_qos_exclude_ops & RBD_IO_OPERATION_COMPARE_AND_WRITE ||
230+
m_qos_enabled_flag == 0) {
226231
return false;
227232
}
228233

@@ -258,12 +263,12 @@ void QosImageDispatch<I>::handle_finished(int r, uint64_t tid) {
258263
}
259264

260265
template <typename I>
261-
bool QosImageDispatch<I>::set_throttle_flag(
262-
std::atomic<uint32_t>* image_dispatch_flags, uint32_t flag) {
266+
bool QosImageDispatch<I>::set_throttle_flags(
267+
std::atomic<uint32_t>* image_dispatch_flags, uint32_t flags) {
263268
uint32_t expected = image_dispatch_flags->load();
264269
uint32_t desired;
265270
do {
266-
desired = expected | flag;
271+
desired = expected | flags;
267272
} while (!image_dispatch_flags->compare_exchange_weak(expected, desired));
268273

269274
return ((desired & IMAGE_DISPATCH_FLAG_QOS_MASK) ==
@@ -278,7 +283,7 @@ bool QosImageDispatch<I>::needs_throttle(
278283
Context* on_dispatched) {
279284
auto cct = m_image_ctx->cct;
280285
auto extent_length = get_extent_length(image_extents);
281-
bool all_qos_flags_set = false;
286+
uint32_t flags_to_set = 0;
282287

283288
if (!read_op) {
284289
m_flush_tracker->start_io(tid);
@@ -292,7 +297,7 @@ bool QosImageDispatch<I>::needs_throttle(
292297
auto qos_enabled_flag = m_qos_enabled_flag;
293298
for (auto [flag, throttle] : m_throttles) {
294299
if ((qos_enabled_flag & flag) == 0) {
295-
all_qos_flags_set = set_throttle_flag(image_dispatch_flags, flag);
300+
flags_to_set |= flag;
296301
continue;
297302
}
298303

@@ -302,12 +307,16 @@ bool QosImageDispatch<I>::needs_throttle(
302307
Tag{image_dispatch_flags, on_dispatched}, flag)) {
303308
ldout(cct, 15) << "on_dispatched=" << on_dispatched << ", "
304309
<< "flag=" << flag << dendl;
305-
all_qos_flags_set = false;
306310
} else {
307-
all_qos_flags_set = set_throttle_flag(image_dispatch_flags, flag);
311+
flags_to_set |= flag;
308312
}
309313
}
310-
return !all_qos_flags_set;
314+
315+
// flags_to_set should never be zero because a single op cannot
316+
// activate all throttles (and m_throttles cannot be empty)
317+
ceph_assert(flags_to_set != 0);
318+
319+
return !set_throttle_flags(image_dispatch_flags, flags_to_set);
311320
}
312321

313322
template <typename I>
@@ -316,7 +325,7 @@ void QosImageDispatch<I>::handle_throttle_ready(Tag&& tag, uint64_t flag) {
316325
ldout(cct, 15) << "on_dispatched=" << tag.on_dispatched << ", "
317326
<< "flag=" << flag << dendl;
318327

319-
if (set_throttle_flag(tag.image_dispatch_flags, flag)) {
328+
if (set_throttle_flags(tag.image_dispatch_flags, flag)) {
320329
// timer_lock is held -- so dispatch from outside the timer thread
321330
m_image_ctx->asio_engine->post(tag.on_dispatched, 0);
322331
}

src/librbd/io/QosImageDispatch.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,8 +117,8 @@ class QosImageDispatch : public ImageDispatchInterface {
117117

118118
void handle_finished(int r, uint64_t tid);
119119

120-
bool set_throttle_flag(std::atomic<uint32_t>* image_dispatch_flags,
121-
uint32_t flag);
120+
bool set_throttle_flags(std::atomic<uint32_t>* image_dispatch_flags,
121+
uint32_t flags);
122122
bool needs_throttle(bool read_op, const Extents& image_extents, uint64_t tid,
123123
std::atomic<uint32_t>* image_dispatch_flags,
124124
DispatchResult* dispatch_result, Context** on_finish,

0 commit comments

Comments
 (0)