Re: [PATCH 3/7] watchdog/hardlockup: Declare arch_touch_nmi_watchdog() only in linux/nmi.h
From: Doug Anderson <dianders@chromium.org>
Date: 2023-06-07 23:40:59
Also in:
linux-perf-users, lkml, sparclinux
Hi, On Wed, Jun 7, 2023 at 8:25 AM Petr Mladek [off-list ref] 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>