[PATCH v9] watchdog/hardlockup: simplify perf event probe and remove per-cpu dependency

Qiliang Yuan posted 1 patch 1 week, 2 days ago
kernel/watchdog_perf.c | 50 +++++++++++++++++++++++-------------------
1 file changed, 28 insertions(+), 22 deletions(-)
[PATCH v9] watchdog/hardlockup: simplify perf event probe and remove per-cpu dependency
Posted by Qiliang Yuan 1 week, 2 days ago
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.

Signed-off-by: Shouxin Sun <sunshx@chinatelecom.cn>
Signed-off-by: Junnan Zhang <zhangjn11@chinatelecom.cn>
Signed-off-by: Qiliang Yuan <yuanql9@chinatelecom.cn>
Signed-off-by: Qiliang Yuan <realwujing@gmail.com>
Cc: Douglas Anderson <dianders@chromium.org>
Cc: Song Liu <song@kernel.org>
Cc: Jinchao Wang <wangjinchao600@gmail.com>
---
v9:
- remove unused `#include <linux/cpu.h>`
v8:
- Add Signed-off-by with work email.
v7:
- Update Signed-off-by to match author email.
- Remove redundant cpu_hotplug_disable() in probe path.
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:
- Refine description: clarify that the retry path uses worker threads
  without PF_PERCPU_THREAD (though mainline is safe due to system_percpu_wq).
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 | 50 +++++++++++++++++++++++-------------------
 1 file changed, 28 insertions(+), 22 deletions(-)

diff --git a/kernel/watchdog_perf.c b/kernel/watchdog_perf.c
index d3ca70e3c256..cf05775a96d3 100644
--- a/kernel/watchdog_perf.c
+++ b/kernel/watchdog_perf.c
@@ -118,18 +118,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 +136,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 +145,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 +258,30 @@ 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 event is released immediately.
+	 */
+	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;
 	}
+
 	return ret;
 }
 
-- 
2.51.0
Re: [PATCH v9] watchdog/hardlockup: simplify perf event probe and remove per-cpu dependency
Posted by Doug Anderson 1 week, 1 day ago
Hi,

On Wed, Jan 28, 2026 at 6:26 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.
>
> Signed-off-by: Shouxin Sun <sunshx@chinatelecom.cn>
> Signed-off-by: Junnan Zhang <zhangjn11@chinatelecom.cn>
> Signed-off-by: Qiliang Yuan <yuanql9@chinatelecom.cn>
> Signed-off-by: Qiliang Yuan <realwujing@gmail.com>
> Cc: Douglas Anderson <dianders@chromium.org>
> Cc: Song Liu <song@kernel.org>
> Cc: Jinchao Wang <wangjinchao600@gmail.com>
> ---
> v9:
> - remove unused `#include <linux/cpu.h>`
> v8:
> - Add Signed-off-by with work email.
> v7:
> - Update Signed-off-by to match author email.
> - Remove redundant cpu_hotplug_disable() in probe path.
> 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:
> - Refine description: clarify that the retry path uses worker threads
>   without PF_PERCPU_THREAD (though mainline is safe due to system_percpu_wq).
> 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 | 50 +++++++++++++++++++++++-------------------
>  1 file changed, 28 insertions(+), 22 deletions(-)

Since I previously gave you my Reviewed-by tag and the changes you
made were minor (and exactly what I requested when I gave you my tag),
you should have kept my Reviewed-by tag in your v9. In general,
knowing whether to keep someone's Reviewed-by or Tested-by tag when
sending a new version is a bit of a judgement call, but in this case
it should have been pretty clear. Generally, if you remove someone's
tag that was given before it's good to mention "after the cut" why you
removed it. If you are unsure about keeping a tag but still decide to
keep it, you can still call attention to that fact "after the cut".

In any case:

Reviewed-by: Douglas Anderson <dianders@chromium.org>

No need to send a v10 adding that tag unless other changes are needed.
Andrew will add it when landing.

-Doug