From dec87e89cf4fc8951e66a8299c2fc09d6c86eeb2 Mon Sep 17 00:00:00 2001
From: Yite Gu <yitegu0@gmail.com>
Date: Wed, 28 Aug 2024 17:48:40 +0800
Subject: [PATCH] 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 <yitegu0@gmail.com>
(cherry picked from commit 9c65adeb23fbb03b31bb0455b601af2c517baad5)
---
 src/blk/kernel/KernelDevice.cc | 22 ++++++++++------------
 src/blk/kernel/KernelDevice.h  |  3 +--
 2 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/src/blk/kernel/KernelDevice.cc b/src/blk/kernel/KernelDevice.cc
index ad7bfc4cf9467..9a56415ef594f 100644
--- a/src/blk/kernel/KernelDevice.cc
+++ b/src/blk/kernel/KernelDevice.cc
@@ -65,7 +65,6 @@ KernelDevice::KernelDevice(CephContext* cct, aio_callback_t cb, void *cbpriv, ai
     discard_callback(d_cb),
     discard_callback_priv(d_cbpriv),
     aio_stop(false),
-    discard_stop(false),
     aio_thread(this),
     injecting_crash(0)
 {
@@ -533,7 +532,7 @@ void KernelDevice::_aio_stop()
   }
 }
 
-void KernelDevice::_discard_update_threads()
+void KernelDevice::_discard_update_threads(bool discard_stop)
 {
   std::unique_lock l(discard_lock);
 
@@ -555,28 +554,27 @@ void KernelDevice::_discard_update_threads()
     }
   // Decrease? Signal threads after telling them to stop
   } else if (newcount < oldcount) {
+    std::vector<std::shared_ptr<DiscardThread>> discard_threads_to_stop;
     dout(10) << __func__ << " stopping " << (oldcount - newcount) << " existing discard threads" << dendl;
 
     // Signal the last threads to quit, and stop tracking them
-    for(uint64_t i = oldcount; i > newcount; i--)
-    {
+    for(uint64_t i = oldcount; i > newcount; i--) {
       discard_threads[i-1]->stop = true;
-      discard_threads[i-1]->detach();
+      discard_threads_to_stop.push_back(discard_threads[i-1]);
     }
-    discard_threads.resize(newcount);
-
     discard_cond.notify_all();
+    discard_threads.resize(newcount);
+    l.unlock();
+    for (auto &t : discard_threads_to_stop) {
+      t->join();
+    }
   }
 }
 
 void KernelDevice::_discard_stop()
 {
   dout(10) << __func__ << dendl;
-
-  discard_stop = true;
-  _discard_update_threads();
-  discard_drain();
-
+  _discard_update_threads(true);
   dout(10) << __func__ << " stopped" << dendl;
 }
 
diff --git a/src/blk/kernel/KernelDevice.h b/src/blk/kernel/KernelDevice.h
index 99098d7fe401a..e3ecaa036d088 100644
--- a/src/blk/kernel/KernelDevice.h
+++ b/src/blk/kernel/KernelDevice.h
@@ -52,7 +52,6 @@ private:
   aio_callback_t discard_callback;
   void *discard_callback_priv;
   bool aio_stop;
-  bool discard_stop;
 
   ceph::mutex discard_lock = ceph::make_mutex("KernelDevice::discard_lock");
   ceph::condition_variable discard_cond;
@@ -93,7 +92,7 @@ private:
   int _aio_start();
   void _aio_stop();
 
-  void _discard_update_threads();
+  void _discard_update_threads(bool discard_stop = false);
   void _discard_stop();
   bool _discard_started();
 
-- 
2.39.5