Re: [PATCH 4/7] watchdog/hardlockup: Enable HAVE_NMI_WATCHDOG only on sparc64
From: Doug Anderson <dianders@chromium.org>
Date: 2023-06-08 00:35:50
Also in:
linux-perf-users, lkml, sparclinux
Hi, On Wed, Jun 7, 2023 at 8:25 AM Petr Mladek [off-list ref] wrote:
quoted hunk ↗ jump to hunk
diff --git a/arch/Kconfig b/arch/Kconfig index 13c6e596cf9e..57f15babe188 100644 --- a/arch/Kconfig +++ b/arch/Kconfig@@ -404,10 +404,9 @@ config HAVE_NMI_WATCHDOG depends on HAVE_NMI bool help - The arch provides its own hardlockup detector implementation instead + Sparc64 provides its own hardlockup detector implementation instead of the generic perf one.
It's a little weird to document generic things with the specifics of the user. The exception, IMO, is when something is deprecated. Personally, it would sound less weird to me to say something like: The arch provides its own hardlockup detector implementation instead of the generic perf one. This is a deprecated thing to do and kept around until sparc64 provides a full hardlockup implementation or moves to generic code.
quoted hunk ↗ jump to hunk
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index d201f5d3876b..4b4aa0f941f9 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug@@ -1050,15 +1050,13 @@ config HAVE_HARDLOCKUP_DETECTOR_BUDDY # sparc64: has a custom implementation which is not using the common # hardlockup command line options and sysctl interface. # -# Note that HAVE_NMI_WATCHDOG is used to distinguish the sparc64 specific -# implementaion. It is automatically enabled also for other arch-specific -# variants which set HAVE_HARDLOCKUP_DETECTOR_ARCH. It makes the check -# of avaialable and supported variants quite tricky. +# Note that HAVE_NMI_WATCHDOG is set when the sparc64 specific implementation +# is used. # config HARDLOCKUP_DETECTOR bool "Detect Hard Lockups" - depends on DEBUG_KERNEL && !S390 - depends on ((HAVE_HARDLOCKUP_DETECTOR_PERF || HAVE_HARDLOCKUP_DETECTOR_BUDDY) && !HAVE_NMI_WATCHDOG) || HAVE_HARDLOCKUP_DETECTOR_ARCH + depends on DEBUG_KERNEL && !S390 && !HAVE_NMI_WATCHDOG + depends on HAVE_HARDLOCKUP_DETECTOR_PERF || HAVE_HARDLOCKUP_DETECTOR_BUDDY || HAVE_HARDLOCKUP_DETECTOR_ARCH
If you add the "!HAVE_NMI_WATCHDOG" as a dependency to HAVE_HARDLOCKUP_DETECTOR_BUDDY, as discussed in a previous patch, you can skip adding it here.
quoted hunk ↗ jump to hunk
imply HARDLOCKUP_DETECTOR_PERF imply HARDLOCKUP_DETECTOR_BUDDY select LOCKUP_DETECTOR@@ -1079,7 +1077,7 @@ config HARDLOCKUP_DETECTOR_PREFER_BUDDY bool "Prefer the buddy CPU hardlockup detector" depends on HARDLOCKUP_DETECTOR depends on HAVE_HARDLOCKUP_DETECTOR_PERF && HAVE_HARDLOCKUP_DETECTOR_BUDDY - depends on !HAVE_NMI_WATCHDOG + depends on !HAVE_HARLOCKUP_DETECTOR_ARCH
Don't need this. Architectures never are allowed to define HAVE_HARDLOCKUP_DETECTOR_PERF and HAVE_HARLOCKUP_DETECTOR_ARCH
quoted hunk ↗ jump to hunk
default n help Say Y here to prefer the buddy hardlockup detector over the perf one.@@ -1096,7 +1094,7 @@ config HARDLOCKUP_DETECTOR_PERF bool depends on HARDLOCKUP_DETECTOR depends on HAVE_HARDLOCKUP_DETECTOR_PERF && !HARDLOCKUP_DETECTOR_PREFER_BUDDY - depends on !HAVE_NMI_WATCHDOG + depends on !HAVE_HARDLOCKUP_DETECTOR_ARCH
Similarly, don't need this.
quoted hunk ↗ jump to hunk
select HARDLOCKUP_DETECTOR_COUNTS_HRTIMER config HARDLOCKUP_DETECTOR_BUDDY@@ -1104,7 +1102,7 @@ config HARDLOCKUP_DETECTOR_BUDDY depends on HARDLOCKUP_DETECTOR depends on HAVE_HARDLOCKUP_DETECTOR_BUDDY depends on !HAVE_HARDLOCKUP_DETECTOR_PERF || HARDLOCKUP_DETECTOR_PREFER_BUDDY - depends on !HAVE_NMI_WATCHDOG + depends on !HAVE_HARDLOCKUP_DETECTOR_ARCH
Similarly, don't need this. In general I don't object to splitting out HAVE_NMI_WATCHDOG from HAVE_HARDLOCKUP_DETECTOR_ARCH.