Skip to content

Commit 9c65ade

Browse files
committed
blk/KernelDevice: using join() to wait thread end is more safe
Using join() to wait discard thread end is more safe, it can ensure that resource releases are sequential, to avoid potential race conditions. Signed-off-by: Yite Gu <[email protected]>
1 parent 327d209 commit 9c65ade

File tree

2 files changed

+11
-14
lines changed

2 files changed

+11
-14
lines changed

src/blk/kernel/KernelDevice.cc

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,6 @@ KernelDevice::KernelDevice(CephContext* cct, aio_callback_t cb, void *cbpriv, ai
6565
discard_callback(d_cb),
6666
discard_callback_priv(d_cbpriv),
6767
aio_stop(false),
68-
discard_stop(false),
6968
aio_thread(this),
7069
injecting_crash(0)
7170
{
@@ -548,7 +547,7 @@ void KernelDevice::_aio_stop()
548547
}
549548
}
550549

551-
void KernelDevice::_discard_update_threads()
550+
void KernelDevice::_discard_update_threads(bool discard_stop)
552551
{
553552
std::unique_lock l(discard_lock);
554553

@@ -570,28 +569,27 @@ void KernelDevice::_discard_update_threads()
570569
}
571570
// Decrease? Signal threads after telling them to stop
572571
} else if (newcount < oldcount) {
572+
std::vector<std::shared_ptr<DiscardThread>> discard_threads_to_stop;
573573
dout(10) << __func__ << " stopping " << (oldcount - newcount) << " existing discard threads" << dendl;
574574

575575
// Signal the last threads to quit, and stop tracking them
576-
for(uint64_t i = oldcount; i > newcount; i--)
577-
{
576+
for(uint64_t i = oldcount; i > newcount; i--) {
578577
discard_threads[i-1]->stop = true;
579-
discard_threads[i-1]->detach();
578+
discard_threads_to_stop.push_back(discard_threads[i-1]);
580579
}
581-
discard_threads.resize(newcount);
582-
583580
discard_cond.notify_all();
581+
discard_threads.resize(newcount);
582+
l.unlock();
583+
for (auto &t : discard_threads_to_stop) {
584+
t->join();
585+
}
584586
}
585587
}
586588

587589
void KernelDevice::_discard_stop()
588590
{
589591
dout(10) << __func__ << dendl;
590-
591-
discard_stop = true;
592-
_discard_update_threads();
593-
discard_drain();
594-
592+
_discard_update_threads(true);
595593
dout(10) << __func__ << " stopped" << dendl;
596594
}
597595

src/blk/kernel/KernelDevice.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,6 @@ class KernelDevice : public BlockDevice,
5858
aio_callback_t discard_callback;
5959
void *discard_callback_priv;
6060
bool aio_stop;
61-
bool discard_stop;
6261
std::unique_ptr<PerfCounters> logger;
6362

6463
ceph::mutex discard_lock = ceph::make_mutex("KernelDevice::discard_lock");
@@ -100,7 +99,7 @@ class KernelDevice : public BlockDevice,
10099
int _aio_start();
101100
void _aio_stop();
102101

103-
void _discard_update_threads();
102+
void _discard_update_threads(bool discard_stop = false);
104103
void _discard_stop();
105104
bool _discard_started();
106105

0 commit comments

Comments
 (0)