The HAVE_ prefix means that the code could be enabled. Add another
variable for HAVE_HARDLOCKUP_DETECTOR_ARCH without this prefix.
It will be set when it should be built. It will make it compatible
with the other hardlockup detectors.
The change allows to clean up dependencies of PPC_WATCHDOG
and HAVE_HARDLOCKUP_DETECTOR_PERF definitions for powerpc.
As a result HAVE_HARDLOCKUP_DETECTOR_PERF has the same dependencies
on arm, x86, powerpc architectures.
Signed-off-by: Petr Mladek <pmladek@suse.com>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
---
arch/powerpc/Kconfig | 5 ++---
include/linux/nmi.h | 2 +-
lib/Kconfig.debug | 9 +++++++++
3 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 539d1f03ff42..987e730740d7 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -90,8 +90,7 @@ config NMI_IPI
config PPC_WATCHDOG
bool
- depends on HARDLOCKUP_DETECTOR
- depends on HAVE_HARDLOCKUP_DETECTOR_ARCH
+ depends on HARDLOCKUP_DETECTOR_ARCH
default y
help
This is a placeholder when the powerpc hardlockup detector
@@ -240,7 +239,7 @@ config PPC
select HAVE_GCC_PLUGINS if GCC_VERSION >= 50200 # plugin support on gcc <= 5.1 is buggy on PPC
select HAVE_GENERIC_VDSO
select HAVE_HARDLOCKUP_DETECTOR_ARCH if PPC_BOOK3S_64 && SMP
- select HAVE_HARDLOCKUP_DETECTOR_PERF if PERF_EVENTS && HAVE_PERF_EVENTS_NMI && !HAVE_HARDLOCKUP_DETECTOR_ARCH
+ select HAVE_HARDLOCKUP_DETECTOR_PERF if PERF_EVENTS && HAVE_PERF_EVENTS_NMI
select HAVE_HW_BREAKPOINT if PERF_EVENTS && (PPC_BOOK3S || PPC_8xx)
select HAVE_IOREMAP_PROT
select HAVE_IRQ_TIME_ACCOUNTING
diff --git a/include/linux/nmi.h b/include/linux/nmi.h
index 515d6724f469..ec808ebd36ba 100644
--- a/include/linux/nmi.h
+++ b/include/linux/nmi.h
@@ -9,7 +9,7 @@
#include <asm/irq.h>
/* Arch specific watchdogs might need to share extra watchdog-related APIs. */
-#if defined(CONFIG_HAVE_HARDLOCKUP_DETECTOR_ARCH) || defined(CONFIG_HARDLOCKUP_DETECTOR_SPARC64)
+#if defined(CONFIG_HARDLOCKUP_DETECTOR_ARCH) || defined(CONFIG_HARDLOCKUP_DETECTOR_SPARC64)
#include <asm/nmi.h>
#endif
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index f285e9cf967a..2c4bb72e72ad 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1056,6 +1056,7 @@ config HARDLOCKUP_DETECTOR
depends on HAVE_HARDLOCKUP_DETECTOR_PERF || HAVE_HARDLOCKUP_DETECTOR_BUDDY || HAVE_HARDLOCKUP_DETECTOR_ARCH
imply HARDLOCKUP_DETECTOR_PERF
imply HARDLOCKUP_DETECTOR_BUDDY
+ imply HARDLOCKUP_DETECTOR_ARCH
select LOCKUP_DETECTOR
help
@@ -1101,6 +1102,14 @@ config HARDLOCKUP_DETECTOR_BUDDY
depends on !HAVE_HARDLOCKUP_DETECTOR_ARCH
select HARDLOCKUP_DETECTOR_COUNTS_HRTIMER
+config HARDLOCKUP_DETECTOR_ARCH
+ bool
+ depends on HARDLOCKUP_DETECTOR
+ depends on HAVE_HARDLOCKUP_DETECTOR_ARCH
+ help
+ The arch-specific implementation of the hardlockup detector will
+ be used.
+
#
# Both the "perf" and "buddy" hardlockup detectors count hrtimer
# interrupts. This config enables functions managing this common code.
--
2.35.3On Fri, Jun 16, 2023 at 05:06:18PM +0200, Petr Mladek wrote: > The HAVE_ prefix means that the code could be enabled. Add another > variable for HAVE_HARDLOCKUP_DETECTOR_ARCH without this prefix. > It will be set when it should be built. It will make it compatible > with the other hardlockup detectors. > > The change allows to clean up dependencies of PPC_WATCHDOG > and HAVE_HARDLOCKUP_DETECTOR_PERF definitions for powerpc. > > As a result HAVE_HARDLOCKUP_DETECTOR_PERF has the same dependencies > on arm, x86, powerpc architectures. > > Signed-off-by: Petr Mladek <pmladek@suse.com> > Reviewed-by: Douglas Anderson <dianders@chromium.org> > --- ... > --- a/include/linux/nmi.h > +++ b/include/linux/nmi.h > @@ -9,7 +9,7 @@ > #include <asm/irq.h> > > /* Arch specific watchdogs might need to share extra watchdog-related APIs. */ > -#if defined(CONFIG_HAVE_HARDLOCKUP_DETECTOR_ARCH) || defined(CONFIG_HARDLOCKUP_DETECTOR_SPARC64) > +#if defined(CONFIG_HARDLOCKUP_DETECTOR_ARCH) || defined(CONFIG_HARDLOCKUP_DETECTOR_SPARC64) This results in: arch/powerpc/platforms/pseries/mobility.c: In function 'pseries_migrate_partition': arch/powerpc/platforms/pseries/mobility.c:753:17: error: implicit declaration of function 'watchdog_hardlockup_set_timeout_pct'; did you mean 'watchdog_hardlockup_stop'? [-Werror=implicit-function-declaration] 753 | watchdog_hardlockup_set_timeout_pct(factor); with ppc64_defconfig -CONFIG_HARDLOCKUP_DETECTOR, because the dummy for watchdog_hardlockup_set_timeout_pct() is still defined in arch/powerpc/include/asm/nmi.h which is no longer included. Guenter > #include <asm/nmi.h> > #endif >
Hi, On Sat, Jul 1, 2023 at 7:40 AM Guenter Roeck <linux@roeck-us.net> wrote: > > On Fri, Jun 16, 2023 at 05:06:18PM +0200, Petr Mladek wrote: > > The HAVE_ prefix means that the code could be enabled. Add another > > variable for HAVE_HARDLOCKUP_DETECTOR_ARCH without this prefix. > > It will be set when it should be built. It will make it compatible > > with the other hardlockup detectors. > > > > The change allows to clean up dependencies of PPC_WATCHDOG > > and HAVE_HARDLOCKUP_DETECTOR_PERF definitions for powerpc. > > > > As a result HAVE_HARDLOCKUP_DETECTOR_PERF has the same dependencies > > on arm, x86, powerpc architectures. > > > > Signed-off-by: Petr Mladek <pmladek@suse.com> > > Reviewed-by: Douglas Anderson <dianders@chromium.org> > > --- > ... > > --- a/include/linux/nmi.h > > +++ b/include/linux/nmi.h > > @@ -9,7 +9,7 @@ > > #include <asm/irq.h> > > > > /* Arch specific watchdogs might need to share extra watchdog-related APIs. */ > > -#if defined(CONFIG_HAVE_HARDLOCKUP_DETECTOR_ARCH) || defined(CONFIG_HARDLOCKUP_DETECTOR_SPARC64) > > +#if defined(CONFIG_HARDLOCKUP_DETECTOR_ARCH) || defined(CONFIG_HARDLOCKUP_DETECTOR_SPARC64) > > This results in: > > arch/powerpc/platforms/pseries/mobility.c: In function 'pseries_migrate_partition': > arch/powerpc/platforms/pseries/mobility.c:753:17: error: implicit declaration of function 'watchdog_hardlockup_set_timeout_pct'; did you mean 'watchdog_hardlockup_stop'? [-Werror=implicit-function-declaration] > 753 | watchdog_hardlockup_set_timeout_pct(factor); > > with ppc64_defconfig -CONFIG_HARDLOCKUP_DETECTOR, because the dummy > for watchdog_hardlockup_set_timeout_pct() is still defined in > arch/powerpc/include/asm/nmi.h which is no longer included. Can you test with: https://lore.kernel.org/r/20230629124500.1.I55e2f4e7903d686c4484cb23c033c6a9e1a9d4c4@changeid Thanks!
On 7/1/23 09:08, Doug Anderson wrote: > Hi, > > On Sat, Jul 1, 2023 at 7:40 AM Guenter Roeck <linux@roeck-us.net> wrote: >> >> On Fri, Jun 16, 2023 at 05:06:18PM +0200, Petr Mladek wrote: >>> The HAVE_ prefix means that the code could be enabled. Add another >>> variable for HAVE_HARDLOCKUP_DETECTOR_ARCH without this prefix. >>> It will be set when it should be built. It will make it compatible >>> with the other hardlockup detectors. >>> >>> The change allows to clean up dependencies of PPC_WATCHDOG >>> and HAVE_HARDLOCKUP_DETECTOR_PERF definitions for powerpc. >>> >>> As a result HAVE_HARDLOCKUP_DETECTOR_PERF has the same dependencies >>> on arm, x86, powerpc architectures. >>> >>> Signed-off-by: Petr Mladek <pmladek@suse.com> >>> Reviewed-by: Douglas Anderson <dianders@chromium.org> >>> --- >> ... >>> --- a/include/linux/nmi.h >>> +++ b/include/linux/nmi.h >>> @@ -9,7 +9,7 @@ >>> #include <asm/irq.h> >>> >>> /* Arch specific watchdogs might need to share extra watchdog-related APIs. */ >>> -#if defined(CONFIG_HAVE_HARDLOCKUP_DETECTOR_ARCH) || defined(CONFIG_HARDLOCKUP_DETECTOR_SPARC64) >>> +#if defined(CONFIG_HARDLOCKUP_DETECTOR_ARCH) || defined(CONFIG_HARDLOCKUP_DETECTOR_SPARC64) >> >> This results in: >> >> arch/powerpc/platforms/pseries/mobility.c: In function 'pseries_migrate_partition': >> arch/powerpc/platforms/pseries/mobility.c:753:17: error: implicit declaration of function 'watchdog_hardlockup_set_timeout_pct'; did you mean 'watchdog_hardlockup_stop'? [-Werror=implicit-function-declaration] >> 753 | watchdog_hardlockup_set_timeout_pct(factor); >> >> with ppc64_defconfig -CONFIG_HARDLOCKUP_DETECTOR, because the dummy >> for watchdog_hardlockup_set_timeout_pct() is still defined in >> arch/powerpc/include/asm/nmi.h which is no longer included. > > Can you test with: > > https://lore.kernel.org/r/20230629124500.1.I55e2f4e7903d686c4484cb23c033c6a9e1a9d4c4@changeid > Ah, I didn't find that one. Sorry for the noise. Yes, that should work. It is a bit odd that including both linux/nmi.h and asm/nmi.h is required, but as it turns out that is actually quite common. Thanks, Guenter
On 7/1/23 09:08, Doug Anderson wrote: > Hi, > > On Sat, Jul 1, 2023 at 7:40 AM Guenter Roeck <linux@roeck-us.net> wrote: >> >> On Fri, Jun 16, 2023 at 05:06:18PM +0200, Petr Mladek wrote: >>> The HAVE_ prefix means that the code could be enabled. Add another >>> variable for HAVE_HARDLOCKUP_DETECTOR_ARCH without this prefix. >>> It will be set when it should be built. It will make it compatible >>> with the other hardlockup detectors. >>> >>> The change allows to clean up dependencies of PPC_WATCHDOG >>> and HAVE_HARDLOCKUP_DETECTOR_PERF definitions for powerpc. >>> >>> As a result HAVE_HARDLOCKUP_DETECTOR_PERF has the same dependencies >>> on arm, x86, powerpc architectures. >>> >>> Signed-off-by: Petr Mladek <pmladek@suse.com> >>> Reviewed-by: Douglas Anderson <dianders@chromium.org> >>> --- >> ... >>> --- a/include/linux/nmi.h >>> +++ b/include/linux/nmi.h >>> @@ -9,7 +9,7 @@ >>> #include <asm/irq.h> >>> >>> /* Arch specific watchdogs might need to share extra watchdog-related APIs. */ >>> -#if defined(CONFIG_HAVE_HARDLOCKUP_DETECTOR_ARCH) || defined(CONFIG_HARDLOCKUP_DETECTOR_SPARC64) >>> +#if defined(CONFIG_HARDLOCKUP_DETECTOR_ARCH) || defined(CONFIG_HARDLOCKUP_DETECTOR_SPARC64) >> >> This results in: >> >> arch/powerpc/platforms/pseries/mobility.c: In function 'pseries_migrate_partition': >> arch/powerpc/platforms/pseries/mobility.c:753:17: error: implicit declaration of function 'watchdog_hardlockup_set_timeout_pct'; did you mean 'watchdog_hardlockup_stop'? [-Werror=implicit-function-declaration] >> 753 | watchdog_hardlockup_set_timeout_pct(factor); >> >> with ppc64_defconfig -CONFIG_HARDLOCKUP_DETECTOR, because the dummy >> for watchdog_hardlockup_set_timeout_pct() is still defined in >> arch/powerpc/include/asm/nmi.h which is no longer included. > > Can you test with: > > https://lore.kernel.org/r/20230629124500.1.I55e2f4e7903d686c4484cb23c033c6a9e1a9d4c4@changeid > Sorry for the noise. I didn't find that one. Yes, that should work. It is a bit odd that including both linux/nmi.h and asm/nmi.h is required, but as it turns out that is actually quite common. Thanks, Guenter
Petr Mladek <pmladek@suse.com> writes:
> The HAVE_ prefix means that the code could be enabled. Add another
> variable for HAVE_HARDLOCKUP_DETECTOR_ARCH without this prefix.
> It will be set when it should be built. It will make it compatible
> with the other hardlockup detectors.
>
> The change allows to clean up dependencies of PPC_WATCHDOG
> and HAVE_HARDLOCKUP_DETECTOR_PERF definitions for powerpc.
>
> As a result HAVE_HARDLOCKUP_DETECTOR_PERF has the same dependencies
> on arm, x86, powerpc architectures.
>
> Signed-off-by: Petr Mladek <pmladek@suse.com>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> ---
> arch/powerpc/Kconfig | 5 ++---
> include/linux/nmi.h | 2 +-
> lib/Kconfig.debug | 9 +++++++++
> 3 files changed, 12 insertions(+), 4 deletions(-)
Something in this patch is breaking the powerpc g5_defconfig, I don't
immediately see what though.
../arch/powerpc/kernel/stacktrace.c: In function ‘handle_backtrace_ipi’:
../arch/powerpc/kernel/stacktrace.c:171:9: error: implicit declaration of function ‘nmi_cpu_backtrace’ [-Werror=implicit-function-declaration]
171 | nmi_cpu_backtrace(regs);
| ^~~~~~~~~~~~~~~~~
../arch/powerpc/kernel/stacktrace.c: In function ‘arch_trigger_cpumask_backtrace’:
../arch/powerpc/kernel/stacktrace.c:226:9: error: implicit declaration of function ‘nmi_trigger_cpumask_backtrace’; did you mean ‘arch_trigger_cpumask_backtrace’? [-Werror=implicit-function-declaration]
226 | nmi_trigger_cpumask_backtrace(mask, exclude_self, raise_backtrace_ipi);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| arch_trigger_cpumask_backtrace
cc1: all warnings being treated as errors
cheers
Hi,
On Wed, Jun 21, 2023 at 6:08 AM Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> Petr Mladek <pmladek@suse.com> writes:
> > The HAVE_ prefix means that the code could be enabled. Add another
> > variable for HAVE_HARDLOCKUP_DETECTOR_ARCH without this prefix.
> > It will be set when it should be built. It will make it compatible
> > with the other hardlockup detectors.
> >
> > The change allows to clean up dependencies of PPC_WATCHDOG
> > and HAVE_HARDLOCKUP_DETECTOR_PERF definitions for powerpc.
> >
> > As a result HAVE_HARDLOCKUP_DETECTOR_PERF has the same dependencies
> > on arm, x86, powerpc architectures.
> >
> > Signed-off-by: Petr Mladek <pmladek@suse.com>
> > Reviewed-by: Douglas Anderson <dianders@chromium.org>
> > ---
> > arch/powerpc/Kconfig | 5 ++---
> > include/linux/nmi.h | 2 +-
> > lib/Kconfig.debug | 9 +++++++++
> > 3 files changed, 12 insertions(+), 4 deletions(-)
>
> Something in this patch is breaking the powerpc g5_defconfig, I don't
> immediately see what though.
>
> ../arch/powerpc/kernel/stacktrace.c: In function ‘handle_backtrace_ipi’:
> ../arch/powerpc/kernel/stacktrace.c:171:9: error: implicit declaration of function ‘nmi_cpu_backtrace’ [-Werror=implicit-function-declaration]
> 171 | nmi_cpu_backtrace(regs);
> | ^~~~~~~~~~~~~~~~~
> ../arch/powerpc/kernel/stacktrace.c: In function ‘arch_trigger_cpumask_backtrace’:
> ../arch/powerpc/kernel/stacktrace.c:226:9: error: implicit declaration of function ‘nmi_trigger_cpumask_backtrace’; did you mean ‘arch_trigger_cpumask_backtrace’? [-Werror=implicit-function-declaration]
> 226 | nmi_trigger_cpumask_backtrace(mask, exclude_self, raise_backtrace_ipi);
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> | arch_trigger_cpumask_backtrace
> cc1: all warnings being treated as errors
Yeah, I can reproduce that.
The problem is that before ${SUBJECT} patch "include/linux/nmi.h"
would include <asm/nmi.h>. Now it won't.
There are a ton of different ways to fix this, but I think the one
that makes sense is to be consistent with other architectures and move
the "arch_trigger_cpumask_backtrace" definitions to asm/irq.h.
https://lore.kernel.org/r/20230621164809.1.Ice67126857506712559078e7de26d32d26e64631@changeid
-Doug
© 2016 - 2026 Red Hat, Inc.