kernel/watchdog_perf.c | 56 +++++++++++++++++++++++++----------------- 1 file changed, 34 insertions(+), 22 deletions(-)
Simplify the hardlockup detector's probe path and remove its implicit
dependency on pinned per-cpu execution.
Refactor hardlockup_detector_event_create() to be stateless. Return
the created perf_event pointer to the caller instead of directly
modifying the per-cpu 'watchdog_ev' variable. This allows the probe
path to safely manage a temporary event without the risk of leaving
stale pointers should task migration occur.
Use cpu_hotplug_disable() during the probe to ensure the target CPU
remains stable throughout the availability check.
Signed-off-by: Shouxin Sun <sunshx@chinatelecom.cn>
Signed-off-by: Junnan Zhang <zhangjn11@chinatelecom.cn>
Signed-off-by: Qiliang Yuan <yuanql9@chinatelecom.cn>
Cc: Song Liu <song@kernel.org>
Cc: Douglas Anderson <dianders@chromium.org>
Cc: Jinchao Wang <wangjinchao600@gmail.com>
---
v6:
- Change title to "simplify/cleanup" and remove "Fixes" tag since the issue
is not reproducible on mainline.
- Rewrite commit message in imperative mood.
- Clarify that mainline is safe while this improves robustness.
- v5 link: https://lore.kernel.org/all/20260127022238.1182079-1-realwujing@gmail.com/
v5:
- Refine description: clarify that the retry path uses worker threads
without PF_PERCPU_THREAD (though mainline is safe due to system_percpu_wq).
- v4 link: https://lore.kernel.org/all/20260124070814.806828-1-realwujing@gmail.com/
v4:
- Add cpu_hotplug_disable() in watchdog_hardlockup_probe() to stabilize
the probe CPU.
- Update description to explain the relevance of 4.19 logs.
v3:
- Refactor hardlockup_detector_event_create() to be stateless by returning
the event pointer instead of directly assigning to per-cpu variables.
- Restore PMU cycle fallback and unify the enable/probe paths.
v2:
- Add Cc: stable@vger.kernel.org.
v1:
- Avoid 'watchdog_ev' in probe path by manually creating and releasing a
local perf event.
kernel/watchdog_perf.c | 56 +++++++++++++++++++++++++-----------------
1 file changed, 34 insertions(+), 22 deletions(-)
diff --git a/kernel/watchdog_perf.c b/kernel/watchdog_perf.c
index d3ca70e3c256..887b61c65c1b 100644
--- a/kernel/watchdog_perf.c
+++ b/kernel/watchdog_perf.c
@@ -17,6 +17,7 @@
#include <linux/atomic.h>
#include <linux/module.h>
#include <linux/sched/debug.h>
+#include <linux/cpu.h>
#include <asm/irq_regs.h>
#include <linux/perf_event.h>
@@ -118,18 +119,11 @@ static void watchdog_overflow_callback(struct perf_event *event,
watchdog_hardlockup_check(smp_processor_id(), regs);
}
-static int hardlockup_detector_event_create(void)
+static struct perf_event *hardlockup_detector_event_create(unsigned int cpu)
{
- unsigned int cpu;
struct perf_event_attr *wd_attr;
struct perf_event *evt;
- /*
- * Preemption is not disabled because memory will be allocated.
- * Ensure CPU-locality by calling this in per-CPU kthread.
- */
- WARN_ON(!is_percpu_thread());
- cpu = raw_smp_processor_id();
wd_attr = &wd_hw_attr;
wd_attr->sample_period = hw_nmi_get_sample_period(watchdog_thresh);
@@ -143,14 +137,7 @@ static int hardlockup_detector_event_create(void)
watchdog_overflow_callback, NULL);
}
- if (IS_ERR(evt)) {
- pr_debug("Perf event create on CPU %d failed with %ld\n", cpu,
- PTR_ERR(evt));
- return PTR_ERR(evt);
- }
- WARN_ONCE(this_cpu_read(watchdog_ev), "unexpected watchdog_ev leak");
- this_cpu_write(watchdog_ev, evt);
- return 0;
+ return evt;
}
/**
@@ -159,17 +146,26 @@ static int hardlockup_detector_event_create(void)
*/
void watchdog_hardlockup_enable(unsigned int cpu)
{
+ struct perf_event *evt;
+
WARN_ON_ONCE(cpu != smp_processor_id());
- if (hardlockup_detector_event_create())
+ evt = hardlockup_detector_event_create(cpu);
+ if (IS_ERR(evt)) {
+ pr_debug("Perf event create on CPU %d failed with %ld\n", cpu,
+ PTR_ERR(evt));
return;
+ }
/* use original value for check */
if (!atomic_fetch_inc(&watchdog_cpus))
pr_info("Enabled. Permanently consumes one hw-PMU counter.\n");
+ WARN_ONCE(this_cpu_read(watchdog_ev), "unexpected watchdog_ev leak");
+ this_cpu_write(watchdog_ev, evt);
+
watchdog_init_timestamp();
- perf_event_enable(this_cpu_read(watchdog_ev));
+ perf_event_enable(evt);
}
/**
@@ -263,19 +259,35 @@ bool __weak __init arch_perf_nmi_is_available(void)
*/
int __init watchdog_hardlockup_probe(void)
{
+ struct perf_event *evt;
+ unsigned int cpu;
int ret;
if (!arch_perf_nmi_is_available())
return -ENODEV;
- ret = hardlockup_detector_event_create();
+ if (!hw_nmi_get_sample_period(watchdog_thresh))
+ return -EINVAL;
- if (ret) {
+ /*
+ * Test hardware PMU availability by creating a temporary perf event.
+ * The requested CPU is arbitrary; preemption is not disabled, so
+ * raw_smp_processor_id() is used. Surround with cpu_hotplug_disable()
+ * to ensure the arbitrarily chosen CPU remains online during the check.
+ * The event is released immediately.
+ */
+ cpu_hotplug_disable();
+ cpu = raw_smp_processor_id();
+ evt = hardlockup_detector_event_create(cpu);
+ if (IS_ERR(evt)) {
pr_info("Perf NMI watchdog permanently disabled\n");
+ ret = PTR_ERR(evt);
} else {
- perf_event_release_kernel(this_cpu_read(watchdog_ev));
- this_cpu_write(watchdog_ev, NULL);
+ perf_event_release_kernel(evt);
+ ret = 0;
}
+ cpu_hotplug_enable();
+
return ret;
}
--
2.51.0
Hi,
On Tue, Jan 27, 2026 at 6:32 PM Qiliang Yuan <realwujing@gmail.com> wrote:
>
> Simplify the hardlockup detector's probe path and remove its implicit
> dependency on pinned per-cpu execution.
>
> Refactor hardlockup_detector_event_create() to be stateless. Return
> the created perf_event pointer to the caller instead of directly
> modifying the per-cpu 'watchdog_ev' variable. This allows the probe
> path to safely manage a temporary event without the risk of leaving
> stale pointers should task migration occur.
>
> Use cpu_hotplug_disable() during the probe to ensure the target CPU
> remains stable throughout the availability check.
>
> Signed-off-by: Shouxin Sun <sunshx@chinatelecom.cn>
> Signed-off-by: Junnan Zhang <zhangjn11@chinatelecom.cn>
> Signed-off-by: Qiliang Yuan <yuanql9@chinatelecom.cn>
> Cc: Song Liu <song@kernel.org>
> Cc: Douglas Anderson <dianders@chromium.org>
> Cc: Jinchao Wang <wangjinchao600@gmail.com>
One last note is that your Signed-off-by tags don't match. When I
apply your patch, I see:
Author: Qiliang Yuan <realwujing@gmail.com>
...but your Signed-off-by is your "@chinatelecom.cn" address. That's
generally not okay. You need to do something to make those match. My
guess is that locally you're using git-send-email and everything looks
good on your end, but then you use your gmail account to login to a
SMTP server to send your patch. Gmail silently rewrites your patch to
be sent from the Gmail address you logged in with.
You'll need to work on your email setup to fix that. Andrew can
correct me if I'm wrong, but I think he can't really commit your patch
until you've resent it with that fix.
> @@ -263,19 +259,35 @@ bool __weak __init arch_perf_nmi_is_available(void)
> */
> int __init watchdog_hardlockup_probe(void)
> {
> + struct perf_event *evt;
> + unsigned int cpu;
> int ret;
>
> if (!arch_perf_nmi_is_available())
> return -ENODEV;
>
> - ret = hardlockup_detector_event_create();
> + if (!hw_nmi_get_sample_period(watchdog_thresh))
> + return -EINVAL;
>
> - if (ret) {
> + /*
> + * Test hardware PMU availability by creating a temporary perf event.
> + * The requested CPU is arbitrary; preemption is not disabled, so
> + * raw_smp_processor_id() is used. Surround with cpu_hotplug_disable()
> + * to ensure the arbitrarily chosen CPU remains online during the check.
> + * The event is released immediately.
> + */
> + cpu_hotplug_disable();
Given our newest understanding, the cpu_hotplug_disable() isn't truly
needed anymore, but I'm still good keeping it. Perhaps we can
eventually make the caller not need to queue the work on the system
workqueue.
With your Signed-off-by fixed, I'd be happy with:
Reviewed-by: Douglas Anderson <dianders@chromium.org>
NOTE: I am not currently set up to test this patch (either through the
main probe path or the retry path) so I haven't personally validated
it. That being said, the code looks right to me. :-P
-Doug
Hi Doug, Just a quick follow-up to my previous reply. I've just sent out v8 which addresses the Signed-off-by issues and some final cleanups. On Tue, Jan 27, 2026 at 8:08 PM Doug Anderson <dianders@chromium.org> wrote: > > One last note is that your Signed-off-by tags don't match. When I > apply your patch, I see: > > Author: Qiliang Yuan <realwujing@gmail.com> > > ...but your Signed-off-by is your "@chinatelecom.cn" address. That's > generally not okay. You need to do something to make those match. Regarding the Signed-off-by tags, I've included both in v8: - Signed-off-by: Qiliang Yuan <realwujing@gmail.com> (Personal email) - Signed-off-by: Qiliang Yuan <yuanql9@chinatelecom.cn> (Work email) My work email often has trouble receiving external mailing list replies, so I've included both to ensure I don't miss any feedback and to properly attribute the work. The v8 version should have everything matching correctly now. v8 Link: https://lore.kernel.org/all/20260128115358.1757147-1-realwujing@gmail.com/ Thanks! Best regards, Qiliang
Hi Doug, Just a quick follow-up to my previous reply. I've just sent out v8 which addresses the Signed-off-by issues and some final cleanups. On Tue, Jan 27, 2026 at 8:08 PM Doug Anderson <dianders@chromium.org> wrote: > One last note is that your Signed-off-by tags don't match. When I > apply your patch, I see: > > Author: Qiliang Yuan <realwujing@gmail.com> > > ...but your Signed-off-by is your "@chinatelecom.cn" address. That's > generally not okay. You need to do something to make those match. Regarding the Signed-off-by tags, I've included both in v8: - Signed-off-by: Qiliang Yuan <realwujing@gmail.com> (Personal email) - Signed-off-by: Qiliang Yuan <yuanql9@chinatelecom.cn> (Work email) My work email often has trouble receiving external mailing list replies, so I've included both to ensure I don't miss any feedback and to properly attribute the work. The v8 version should have everything matching correctly now. I've already sent v8 twice today, but for some reason, they are still not visible on lore.kernel.org. Here are the Message-IDs from those three attempts for your reference: - <20260128115358.1757147-1-realwujing@gmail.com> - <20260128122330.1761777-1-realwujing@gmail.com> - <20260128125639.1767682-1-realwujing@gmail.com> Therefore, I cannot provide a lore link for v8 yet. You may use the v7 version from lore as well, I have already added "Signed-off-by: Qiliang Yuan <realwujing@gmail.com>" to match the author's identity in v7, but please help to include "Signed-off-by: Qiliang Yuan <yuanql9@chinatelecom.cn>". Thanks! Best regards, Qiliang
Hi Doug, On Tue, Jan 27, 2026 at 8:08 PM Doug Anderson <dianders@chromium.org> wrote: > One last note is that your Signed-off-by tags don't match. When I > apply your patch, I see: > > Author: Qiliang Yuan <realwujing@gmail.com> > > ...but your Signed-off-by is your "@chinatelecom.cn" address. That's > generally not okay. You need to do something to make those match. Thank you for pointing this out. I've updated my git configuration and the patch to ensure the Signed-off-by matches the Author's Gmail address. > Given our newest understanding, the cpu_hotplug_disable() isn't truly > needed anymore, but I'm still good keeping it. Since the probe is now fully stateless and no longer touches per-cpu variables, I've opted to remove the redundant cpu_hotplug_disable() in v7 to make the code cleaner, as you suggested it wasn't strictly necessary. v7 Link: https://lore.kernel.org/all/20260128060833.1715622-1-realwujing@gmail.com/ Thanks again for your detailed review and guidance! Best regards, Qiliang
© 2016 - 2026 Red Hat, Inc.