| Message ID | 20230607152432.5435-4-pmladek@suse.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | watchdog/hardlockup: Cleanup configuration of hardlockup detectors | expand |
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>
On Wed 2023-06-07 16:35:19, Doug Anderson wrote: > 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/ Will fix in v2. > > This looks right and is a nice cleanup. > > Reviewed-by: Douglas Anderson <dianders@chromium.org> Thanks a lot. Best Regards, Petr
diff --git a/arch/powerpc/include/asm/nmi.h b/arch/powerpc/include/asm/nmi.h index 43bfd4de868f..ce25318c3902 100644 --- a/arch/powerpc/include/asm/nmi.h +++ b/arch/powerpc/include/asm/nmi.h @@ -3,11 +3,9 @@ #define _ASM_NMI_H #ifdef CONFIG_PPC_WATCHDOG -extern void arch_touch_nmi_watchdog(void); long soft_nmi_interrupt(struct pt_regs *regs); void watchdog_hardlockup_set_timeout_pct(u64 pct); #else -static inline void arch_touch_nmi_watchdog(void) {} static inline void watchdog_hardlockup_set_timeout_pct(u64 pct) {} #endif diff --git a/arch/sparc/include/asm/nmi.h b/arch/sparc/include/asm/nmi.h index 90ee7863d9fe..920dc23f443f 100644 --- a/arch/sparc/include/asm/nmi.h +++ b/arch/sparc/include/asm/nmi.h @@ -8,7 +8,6 @@ void nmi_adjust_hz(unsigned int new_hz); extern atomic_t nmi_active; -void arch_touch_nmi_watchdog(void); void start_nmi_watchdog(void *unused); void stop_nmi_watchdog(void *unused); diff --git a/include/linux/nmi.h b/include/linux/nmi.h index b5d0b7ab52fb..b9e816bde14a 100644 --- a/include/linux/nmi.h +++ b/include/linux/nmi.h @@ -7,6 +7,8 @@ #include <linux/sched.h> #include <asm/irq.h> + +/* Arch specific watchdogs might need to share extra watchdog-related APIs. */ #if defined(CONFIG_HAVE_NMI_WATCHDOG) #include <asm/nmi.h> #endif @@ -89,12 +91,17 @@ extern unsigned int hardlockup_panic; static inline void hardlockup_detector_disable(void) {} #endif -#if defined(CONFIG_HARDLOCKUP_DETECTOR_COUNTS_HRTIMER) +/* Sparc64 has special implemetantion that is always enabled. */ +#if defined(CONFIG_HARDLOCKUP_DETECTOR) || \ + (defined(CONFIG_HAVE_NMI_WATCHDOG) && !defined(CONFIG_HAVE_HARDLOCKUP_DETECTOR_ARCH)) void arch_touch_nmi_watchdog(void); +#else +static inline void arch_touch_nmi_watchdog(void) { } +#endif + +#if defined(CONFIG_HARDLOCKUP_DETECTOR_COUNTS_HRTIMER) void watchdog_hardlockup_touch_cpu(unsigned int cpu); void watchdog_hardlockup_check(unsigned int cpu, struct pt_regs *regs); -#elif !defined(CONFIG_HAVE_NMI_WATCHDOG) -static inline void arch_touch_nmi_watchdog(void) { } #endif #if defined(CONFIG_HARDLOCKUP_DETECTOR_PERF)
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. 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(-)