[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAD=FV=XX0dJ2jEaQ21M4Kas6pbJL0SSCxYhr8-1kqSTEiJP_UA@mail.gmail.com>
Date: Tue, 27 Jan 2026 20:08:00 -0800
From: Doug Anderson <dianders@...omium.org>
To: Qiliang Yuan <realwujing@...il.com>
Cc: Li Huafei <lihuafei1@...wei.com>, Andrew Morton <akpm@...ux-foundation.org>,
Ingo Molnar <mingo@...nel.org>, Jinchao Wang <wangjinchao600@...il.com>,
Yicong Yang <yangyicong@...ilicon.com>, Thorsten Blum <thorsten.blum@...ux.dev>,
linux-watchdog@...r.kernel.org, mm-commits@...r.kernel.org,
Shouxin Sun <sunshx@...natelecom.cn>, Junnan Zhang <zhangjn11@...natelecom.cn>,
Qiliang Yuan <yuanql9@...natelecom.cn>, Song Liu <song@...nel.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v6] watchdog/hardlockup: simplify perf event probe and
remove per-cpu dependency
Hi,
On Tue, Jan 27, 2026 at 6:32 PM Qiliang Yuan <realwujing@...il.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@...natelecom.cn>
> Signed-off-by: Junnan Zhang <zhangjn11@...natelecom.cn>
> Signed-off-by: Qiliang Yuan <yuanql9@...natelecom.cn>
> Cc: Song Liu <song@...nel.org>
> Cc: Douglas Anderson <dianders@...omium.org>
> Cc: Jinchao Wang <wangjinchao600@...il.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@...il.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@...omium.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
Powered by blists - more mailing lists