Re: [PATCH 3/7] watchdog/hardlockup: Declare arch_touch_nmi_watchdog() only in linux/nmi.h

Doug Anderson
2023-06-07T23:40:59Z

Hi,

On Wed, Jun 7, 2023 at 8:25 AM Petr Mladek <pmladek@suse.com> wrote:
>
> arch_touch_nmi_watchdog() needs a different implementation for various
> hardlockup detector implementations. And it does nothing when
> any hardlockup detector is not build at all.

s/build/built/

> arch_touch_nmi_watchdog() has to be declared in linux/nmi.h. It is done
> directly in this header file for the perf and buddy detectors. And it
> is done in the included asm/linux.h for arch specific detectors.
>
> The reason probably is that the arch specific variants build the code
> using another conditions. For example, powerpc64/sparc64 builds the code
> when CONFIG_PPC_WATCHDOG is enabled.
>
> Another reason might be that these architectures define more functions
> in asm/nmi.h anyway.
>
> However the generic code actually knows the information. The config
> variables HAVE_NMI_WATCHDOG and HAVE_HARDLOCKUP_DETECTOR_ARCH are used
> to decide whether to build the buddy detector.
>
> In particular, CONFIG_HARDLOCKUP_DETECTOR is set only when a generic
> or arch-specific hardlockup detector is built. The only exception
> is sparc64 which ignores the global HARDLOCKUP_DETECTOR switch.
>
> The information about sparc64 is a bit complicated. The hardlockup
> detector is built there when CONFIG_HAVE_NMI_WATCHDOG is set and
> CONFIG_HAVE_HARDLOCKUP_DETECTOR_ARCH is not set.
>
> People might wonder whether this change really makes things easier.
> The motivation is:
>
> + The current logic in linux/nmi.h is far from obvious.
> For example, arch_touch_nmi_watchdog() is defined as {} when
> neither CONFIG_HARDLOCKUP_DETECTOR_COUNTS_HRTIMER nor
> CONFIG_HAVE_NMI_WATCHDOG is defined.
>
> + The change synchronizes the checks in lib/Kconfig.debug and
> in the generic code.
>
> + It is a step that will help cleaning HAVE_NMI_WATCHDOG related
> checks.
>
> The change should not change the existing behavior.
>
> Signed-off-by: Petr Mladek <pmladek@suse.com>
> ---
> arch/powerpc/include/asm/nmi.h | 2 --
> arch/sparc/include/asm/nmi.h | 1 -
> include/linux/nmi.h | 13 ++++++++++---
> 3 files changed, 10 insertions(+), 6 deletions(-)

This looks right and is a nice cleanup.

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