Re: [PATCH] arm64: watchdog_hld: provide arm_pmu_irq_is_nmi stub

[Doug Anderson]

Hi,

On Mon, May 22, 2023 at 7:31 AM Doug Anderson <dianders@xxxxxxxxxxxx> wrote:
>
> Hi,
>
> On Mon, May 22, 2023 at 4:49 AM Arnd Bergmann <arnd@xxxxxxxxxx> wrote:
> >
> > From: Arnd Bergmann <arnd@xxxxxxxx>
> >
> > The newly added arch_perf_nmi_is_available() function fails to build
> > when CONFIG_ARM_PMU is disabled:
> >
> > arch/arm64/kernel/watchdog_hld.c: In function 'arch_perf_nmi_is_available':
> > arch/arm64/kernel/watchdog_hld.c:35:16: error: implicit declaration of function 'arm_pmu_irq_is_nmi' [-Werror=implicit-function-declaration]
> >    35 |         return arm_pmu_irq_is_nmi();
> >
> > As it turns out, there is only one caller for that function anyway,
> > in the same file as the __weak definition, and this can only be called
> > if CONFIG_ARM_PMU is also enabled.
> >
> > I tried a number of variants, but everything ended up with more
> > complexity from having both the __weak function and one or more
> > added #ifdef. Keeping it in watchdog_perf.c is a small layering
> > violation but otherwise the most robust.
>
> Sorry for the breakage!
>
> The intention here is that turning on CONFIG_HARDLOCKUP_DETECTOR_PERF
> doesn't make any sense if the PMU is not enabled. In that sense, maybe
> a better option would be to just fix this in Kconfig? What about going
> into `arch/arm64/Kconfig` and instead of:
>
> select HAVE_HARDLOCKUP_DETECTOR_PERF if PERF_EVENTS && HAVE_PERF_EVENTS_NMI
>
> We do:
>
> select HAVE_HARDLOCKUP_DETECTOR_PERF if PERF_EVENTS &&
> HAVE_PERF_EVENTS_NMI && HW_PERF_EVENTS
>
> It looks like HW_PERF_EVENTS is a synonym for ARM_PMU and that's the
> same symbol used to include the needed definition.
>
> Making that change seems to fix the compile error and has the added
> benefit that enabling CONFIG_HARDLOCKUP_DETECTOR will automatically
> choose the "buddy" backend when CONFIG_ARM_PMU isn't turned on.

Breadcrumbs: since I didn't see a patch this morning and I'd love to
get this resolved, I've posted the Kconfig fix:

https://lore.kernel.org/r/20230523073952.1.I60217a63acc35621e13f10be16c0cd7c363caf8c@changeid

Assuming people think that's OK, it should land instead of ${SUBJECT} patch.

-Doug