Re: [PATCH v2 0/2] Update the watchdog period according to real CPU frequency

[Yicong Yang <yangyicong@huawei.com>]

On 2025/5/13 7:06, Andrew Morton wrote:
> On Mon, 12 May 2025 21:09:17 +0800 Yicong Yang <yangyicong@huawei.com> wrote:
> 
>> From: Yicong Yang <yangyicong@hisilicon.com>
>>
>> watchdog perf needs architecture to provide method for converting the watchdog
>> thresh to counter period. For arm64 we're using the max CPU frequency for
>> doing the conversion which is from cpufreq driver. But some cpufreq driver
>> are registered lately, for example cppc_cpufreq will be registered at late
>> initcall which is after the initialization of watchdog perf (initialized in
>> armv8_pmuv3 of device initcall). In such case the period of watchdog will not
>> be accurate enough. Fix this by registering a cpufreq notifier and update the
>> watchdog period once the cpufreq driver is initialized.
>>
> 
> Thanks.  Thoughts.
> 
> 1: What is the impact of this change?  Is the current code causing
>    problems?  If so, what are they?  How is the end-user experience
>    improved by this change?  Important info!
> 

This will make NMI watchdog (hardlockup detector) work more accurately. HARDLOCKUP_DETECTOR_PERF
is driven by the PMU sample interrupts of cpu cycle event with period related to the watchdog
threshold.  User will set the watchdog threshold in seconds (e.g. 10s by default) and for
HARDLOCKUP_DETECTOR_PERF we need to convert the seconds to cycles to setup the PMU counter.
The coversion method is provided by the arhitecture by implementing hw_nmi_get_sample_period().
For arm64 it's using max_cpufreq to do the conversion:
cycle_event_period = threshold(s) * max_cpufreq(hz)

Since arm64 doesn't have an arthictectural way to get the max_cpufreq, we use cpufreq_driver
to get it. If cpufreq_driver is not available, currently we use a safe max_cpufreq as 5GHz.
Without this patchset, if the cpufreq_driver is initialized after the hardlockup detector we'll
use 5GHz for calculating the event period. It's my case here as described in the coverletter.
That means in the default case (10s threshold) if the real max_cpufreq is 2.5GHz, the NMI watchdog
is actually working in a 20s period. With this patchset the period can be calibrated after
the cpufreq driver is initialized, much more accurate.

> 2: As far as I can tell, this patchset impacts arm64 only.  Do you
>    think that other architectures should implement this?

It's highly depends on the architecure's implementation of their HARDLOCKUP_DETECTOR_PERF (the
implementation of hw_nmi_get_sample_period()). If other architectures can gain the max_cpufreq
without cpufreq driver they don't have this problems (seems x86 implement in another way
and can gain the cpufreq by some arhitectural way).

> 
> 3: As far as I can tell, this patchset affects all cpufreq drivers
>    which use late_initcall() (on arm64, of course).  Is this correct?
> 

cpufreq drivers are not touched. we registered a notifier block to the cpufreq framework
so the changes to the cpufreq will notify us to see whether it's needed to modify the
event period.


> 4: It is asserted that we should use the *maximum* possible CPU
>    frequency for this calculation.  Why?  I assume this is because we
>    care about the minimum watchdog period?
> 

I think it may because it's impossbile to use the current frequency for calculating the counter
period as it may change at any time. Using maximum frequency will make the real period as close
as possible to the expected watchdog thresh in a simplest way.

> Can I assume that the ARM maintainers will be handling this?
> .

I think yes since it only affects the arm64's implementation of HARDLOCKUP_DETECTOR_PERF.

Thanks.