Re: [External] Re: [PATCH v4 1/2] watchdog: move arm64 watchdog_hld into common code

[Will Deacon <will@kernel.org>]

On Fri, Nov 07, 2025 at 10:42:25AM +0800, yunhui cui wrote:
> On Mon, Nov 3, 2025 at 9:44 PM Will Deacon <will@kernel.org> wrote:
> >
> > On Tue, Oct 14, 2025 at 11:14:24AM +0800, Yunhui Cui wrote:
> > > @@ -306,3 +307,85 @@ void __init hardlockup_config_perf_event(const char *str)
> > >       wd_hw_attr.type = PERF_TYPE_RAW;
> > >       wd_hw_attr.config = config;
> > >  }
> > > +
> > > +#ifdef CONFIG_WATCHDOG_PERF_ADJUST_PERIOD
> > > +/*
> > > + * Safe maximum CPU frequency in case a particular platform doesn't implement
> > > + * cpufreq driver. Although, architecture doesn't put any restrictions on
> > > + * maximum frequency but 5 GHz seems to be safe maximum given the available
> > > + * CPUs in the market which are clocked much less than 5 GHz. On the other
> > > + * hand, we can't make it much higher as it would lead to a large hard-lockup
> > > + * detection timeout on parts which are running slower (eg. 1GHz on
> > > + * Developerbox) and doesn't possess a cpufreq driver.
> > > + */
> > > +#define SAFE_MAX_CPU_FREQ    5000000000UL // 5 GHz
> > > +__weak u64 hw_nmi_get_sample_period(int watchdog_thresh)
> > > +{
> > > +     unsigned int cpu = smp_processor_id();
> > > +     unsigned long max_cpu_freq;
> > > +
> > > +     max_cpu_freq = cpufreq_get_hw_max_freq(cpu) * 1000UL;
> > > +     if (!max_cpu_freq)
> > > +             max_cpu_freq = SAFE_MAX_CPU_FREQ;
> > > +
> > > +     return (u64)max_cpu_freq * watchdog_thresh;
> > > +}
> >
> > Why does this function become __weak? Neither arm64 nor riscv override
> > it afaict.
> 
> Would you say there’s any particular issue here? If some architectures
> might need to override the hw_nmi_get_sample_period() function later
> on, wouldn’t __weak be a more reasonable choice?

__weak is pretty brittle (it can depend on link order if you have multiple
targets) and I suspect it prevents inlining when LTO isn't enabled. It's
cleaner and more robust for architectures to provide their hooks by
#defining the symbol, as is done commonly in other parts of the kernel.

But in this particular case, it's completely unnecessary because there
isn't an architectural override and so this function should simply be
static.

Will