blk/kernel: improve DiscardThread life cycle.

This will eliminate a potential race between thread startup and its
removal.

Relates-to: https://tracker.ceph.com/issues/71800
Signed-off-by: Igor Fedotov <igor.fedotov@croit.io>
(cherry picked from commit 69369b151b96ca74bffb9d72f4c249f48fde2845)

diff --git a/src/blk/kernel/KernelDevice.cc b/src/blk/kernel/KernelDevice.cc
index 0b5f44bd652003debf2956f66ccc6f54844ca0c1..bebca263db8307ff73117bd1f9efe2c6aae2baf4 100644 (file)
--- a/src/blk/kernel/KernelDevice.cc
+++ b/src/blk/kernel/KernelDevice.cc
@@ -571,12 +571,12 @@ void KernelDevice::_discard_update_threads(bool discard_stop)
     for(uint64_t i = oldcount; i < newcount; i++)
     {
       // All threads created with the same name
-      discard_threads.emplace_back(new DiscardThread(this, i));
+      discard_threads.emplace_back(new DiscardThread(this));
       discard_threads.back()->create("bstore_discard");
     }
   // Decrease? Signal threads after telling them to stop
   } else if (newcount < oldcount) {
-    std::vector<std::shared_ptr<DiscardThread>> discard_threads_to_stop;
+    std::vector<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
@@ -587,8 +587,9 @@ void KernelDevice::_discard_update_threads(bool discard_stop)
     discard_cond.notify_all();
     discard_threads.resize(newcount);
     l.unlock();
-    for (auto &t : discard_threads_to_stop) {
+    for (auto t : discard_threads_to_stop) {
       t->join();
+      delete t;
     }
   }
   logger->set(l_blk_kernel_discard_threads, discard_threads.size());
@@ -758,9 +759,9 @@ void KernelDevice::swap_discard_queued(interval_set<uint64_t>& other)
   discard_queued.swap(other);
 }
 
-void KernelDevice::_discard_thread(uint64_t tid)
+void KernelDevice::_discard_thread(DiscardThread* thr)
 {
-  dout(10) << __func__ << " thread " << tid << " start" << dendl;
+  dout(10) << __func__ << " thread " << thr << " start" << dendl;
 
   // Thread-local list of processing discards
   interval_set<uint64_t> discard_processing;
@@ -768,10 +769,6 @@ void KernelDevice::_discard_thread(uint64_t tid)
   std::unique_lock l(discard_lock);
   discard_cond.notify_all();
 
-  // Keeps the shared pointer around until erased from the vector
-  // and until we leave this function
-  auto thr = discard_threads[tid];
-
   while (true) {
     ceph_assert(discard_processing.empty());
     if (discard_queued.empty()) {
@@ -822,7 +819,7 @@ void KernelDevice::_discard_thread(uint64_t tid)
     }
   }
 
-  dout(10) << __func__ << " thread " << tid << " finish" << dendl;
+  dout(10) << __func__ << " thread " << thr << " finish" << dendl;
 }
 
 // this is private and is expected that the caller checks that discard

diff --git a/src/blk/kernel/KernelDevice.h b/src/blk/kernel/KernelDevice.h
index 29e91ab5f6dc8999257ec0081c2440076c99ec64..3c7a82a54fd3d9672b51597749da092cd168a6a4 100644 (file)
--- a/src/blk/kernel/KernelDevice.h
+++ b/src/blk/kernel/KernelDevice.h
@@ -78,15 +78,15 @@ private:
 
   struct DiscardThread : public Thread {
     KernelDevice *bdev;
-    const uint64_t id;
     bool stop = false;
-    explicit DiscardThread(KernelDevice *b, uint64_t id) : bdev(b), id(id) {}
+    explicit DiscardThread(KernelDevice *b) : bdev(b) {
+    }
     void *entry() override {
-      bdev->_discard_thread(id);
+      bdev->_discard_thread(this);
       return NULL;
     }
   };
-  std::vector<std::shared_ptr<DiscardThread>> discard_threads;
+  std::vector<DiscardThread*> discard_threads;
 
   std::atomic_int injecting_crash;
 
@@ -94,7 +94,7 @@ private:
   virtual void  _pre_close() { }  // hook for child implementations
 
   void _aio_thread();
-  void _discard_thread(uint64_t tid);
+  void _discard_thread(DiscardThread* thr);
   bool _queue_discard(interval_set<uint64_t> &to_release);
   bool try_discard(interval_set<uint64_t> &to_release, bool async = true) override;