blk/KernelDevice: React to bdev_enable_discard changes in handle_conf_change()

This fixes two issues that were introduced by 755f3e03b5bf547cfd940b52a53833f66285062b:
1. After an OSD boots, discard threads were not stopped when
   bdev_enable_discard was set to false, whereas that was the intent of
   that commit.
2. If bdev_enable_discard or bdev_async_discard_threads are configured
   with a mask that can't be evaluated at OSD boot (e.g. a device
   class), then async discard won't be enabled until a later config
   change to bdev_async_discard_threads.

Fixes: https://tracker.ceph.com/issues/66817
Signed-off-by: Joshua Baergen <jbaergen@digitalocean.com>

diff --git a/src/blk/kernel/KernelDevice.cc b/src/blk/kernel/KernelDevice.cc
index 18466648e8eaa3b8edb6a8b90762eb4986d417af..057dd04cba8986ea9fbdba94d1e7aeefd389a6af 100644 (file)
--- a/src/blk/kernel/KernelDevice.cc
+++ b/src/blk/kernel/KernelDevice.cc
@@ -1511,6 +1511,7 @@ const char** KernelDevice::get_tracked_conf_keys() const
 {
   static const char* KEYS[] = {
     "bdev_async_discard_threads",
+    "bdev_enable_discard",
     NULL
   };
   return KEYS;
@@ -1519,11 +1520,16 @@ const char** KernelDevice::get_tracked_conf_keys() const
 void KernelDevice::handle_conf_change(const ConfigProxy& conf,
                             const std::set <std::string> &changed)
 {
-  if (changed.count("bdev_async_discard_threads")) {
+  if (changed.count("bdev_async_discard_threads") || changed.count("bdev_enable_discard")) {
     std::unique_lock l(discard_lock);
 
     uint64_t oldval = target_discard_threads;
     uint64_t newval = cct->_conf.get_val<uint64_t>("bdev_async_discard_threads");
+    if (!cct->_conf.get_val<bool>("bdev_enable_discard")) {
+      // We don't want these threads running if discard has been disabled (this is consistent with
+      // KernelDevice::open())
+      newval = 0;
+    }
 
     target_discard_threads = newval;
 
@@ -1537,8 +1543,8 @@ void KernelDevice::handle_conf_change(const ConfigProxy& conf,
         discard_threads.emplace_back(new DiscardThread(this, i));
         discard_threads.back()->create("bstore_discard");
       }
-    } else {
-      // Decrease? Signal threads after telling them to stop
+    // Decrease? Signal threads after telling them to stop
+    } else if (newval < oldval) {
       dout(10) << __func__ << " stopping " << (oldval - newval) << " existing discard threads" << dendl;
 
       // Decreasing to zero is exactly the same as disabling async discard.