From 8ffe35e85f2e1d10fa8b238d54bbf89e3862830c Mon Sep 17 00:00:00 2001 From: Joshua Baergen Date: Tue, 2 Jul 2024 12:10:31 -0600 Subject: [PATCH] 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 --- src/blk/kernel/KernelDevice.cc | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/blk/kernel/KernelDevice.cc b/src/blk/kernel/KernelDevice.cc index 18466648e8eaa..057dd04cba898 100644 --- 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 &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("bdev_async_discard_threads"); + if (!cct->_conf.get_val("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. -- 2.39.5