On Fri 2023-05-26 18:41:40, Douglas Anderson wrote:
> HAVE_HARDLOCKUP_DETECTOR_NON_ARCH is a mouthful and
> confusing. HAVE_HARDLOCKUP_DETECTOR_PERF_OR_BUDDY is even more of a
> mouthful, but probably less confusing. Rename the Kconfig names.
It is better. But I have an idea that might be even better.
> Signed-off-by: Douglas Anderson <dianders@xxxxxxxxxxxx>
> ---
>
> lib/Kconfig.debug | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index eb1edd5905bc..b9e162698a82 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -1058,7 +1058,7 @@ config HARDLOCKUP_DETECTOR_BUDDY
> # needs SMP). In either case, using the "non-arch" code conflicts with
> # the NMI watchdog code (which is sometimes used directly and sometimes used
> # by the arch-provided hardlockup detector).
The comment above still uses the term "no-arch" and tries to
explain the confusion around it.
> -config HAVE_HARDLOCKUP_DETECTOR_NON_ARCH
> +config HAVE_HARDLOCKUP_DETECTOR_PERF_OR_BUDDY
> bool
> depends on (HAVE_HARDLOCKUP_DETECTOR_PERF || SMP) && !HAVE_NMI_WATCHDOG
> default y
> @@ -1077,10 +1077,10 @@ config HARDLOCKUP_DETECTOR_PREFER_BUDDY
> an arch-specific hardlockup detector or if resources needed
> for the hardlockup detector are better used for other things.
>
> -# This will select the appropriate non-arch hardlockdup detector
> -config HARDLOCKUP_DETECTOR_NON_ARCH
> +# This will select the appropriate non-arch hardlockup detector
> +config HARDLOCKUP_DETECTOR_PERF_OR_BUDDY
> bool
> - depends on HAVE_HARDLOCKUP_DETECTOR_NON_ARCH
> + depends on HAVE_HARDLOCKUP_DETECTOR_PERF_OR_BUDDY
> select HARDLOCKUP_DETECTOR_BUDDY if !HAVE_HARDLOCKUP_DETECTOR_PERF || HARDLOCKUP_DETECTOR_PREFER_BUDDY
> select HARDLOCKUP_DETECTOR_PERF if HAVE_HARDLOCKUP_DETECTOR_PERF && !HARDLOCKUP_DETECTOR_PREFER_BUDDY
>
> @@ -1098,9 +1098,9 @@ config HARDLOCKUP_CHECK_TIMESTAMP
> config HARDLOCKUP_DETECTOR
> bool "Detect Hard Lockups"
> depends on DEBUG_KERNEL && !S390
> - depends on HAVE_HARDLOCKUP_DETECTOR_NON_ARCH || HAVE_HARDLOCKUP_DETECTOR_ARCH
> + depends on HAVE_HARDLOCKUP_DETECTOR_PERF_OR_BUDDY || HAVE_HARDLOCKUP_DETECTOR_ARCH
> select LOCKUP_DETECTOR
> - select HARDLOCKUP_DETECTOR_NON_ARCH if HAVE_HARDLOCKUP_DETECTOR_NON_ARCH
> + select HARDLOCKUP_DETECTOR_PERF_OR_BUDDY if HAVE_HARDLOCKUP_DETECTOR_PERF_OR_BUDDY
>
> help
> Say Y here to enable the kernel to act as a watchdog to detect
I am sorry but I am still confused by the logic. For me, it is far
from clear what combinations are possible, impossible, and optional.
Especially, the effect of HAVE_NMI_WATCHDOG and
HAVE_HARDLOCKUP_DETECTOR_ARCH is quite tricky.
I was playing with it and came up with a more straightforward solution
and found more possibilities how the simplify the logic. I am going
to prepare a patchset that would replace this patch.
Just to get the idea. I made the following changes:
+ define the values in logical order:
+ HAVE_*
+ HARDLOCKUP_DETECTOR y/n value
+ HARDLOCKUP_DETECTOR_PREFER_BUDDY y/n value
+ HARDLOCKUP_DETECTOR_PERF decision based on above
+ HARDLOCKUP_DETECTOR_BUDDY decision based on above
+ remove HAVE_HARDLOCKUP_DETECTOR_PERF_OR_BUDDY,
instead, explicitly define the dependencies on all HAVE_*
variables to make it clear what it possible
and what is not possible
+ remove HARDLOCKUP_DETECTOR_PERF_OR_BUDDY,
instead use "imply" in HARDLOCKUP_DETECTOR to trigger
re-evaluation of HARDLOCKUP_DETECTOR_PERF and
HARDLOCKUP_DETECTOR_BUDDY decisions
My current version has the following in lib/Kconfig.devel:
--- cut ---
config HAVE_HARDLOCKUP_DETECTOR_BUDDY
bool
depends on SMP
default y
#
# arch/ can define HAVE_NMI_WATCHDOG to provide their own hard
# lockup detector rather than the generic perf or buddy detector.
#
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
imply HARDLOCKUP_DETECTOR_PERF
imply HARDLOCKUP_DETECTOR_BUDDY
select LOCKUP_DETECTOR
help
Say Y here to enable the kernel to act as a watchdog to detect
hard lockups.
Hardlockups are bugs that cause the CPU to loop in kernel mode
for more than 10 seconds, without letting other interrupts have a
chance to run. The current stack trace is displayed upon detection
and the system will stay locked up.
#
# The architecture-specific variant is always used when available,
# see HAVE_NMI_WATCHDOG
#
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 && !HAVE_NMI_WATCHDOG
default n
help
Say Y here to prefer the buddy hardlockup detector over the perf one.
With the buddy detector, each CPU uses its softlockup hrtimer
to check that the next CPU is processing hrtimer interrupts by
verifying that a counter is increasing.
This hardlockup detector is useful on systems that don't have
an arch-specific hardlockup detector or if resources needed
for the hardlockup detector are better used for other things.
config HARDLOCKUP_DETECTOR_PERF
bool
depends on HARDLOCKUP_DETECTOR
depends on HAVE_HARDLOCKUP_DETECTOR_PERF && !HARDLOCKUP_DETECTOR_PREFER_BUDDY && !HAVE_NMI_WATCHDOG
select HARDLOCKUP_DETECTOR_COUNTS_HRTIMER
config HARDLOCKUP_DETECTOR_BUDDY
bool
depends on HARDLOCKUP_DETECTOR
depends on HAVE_HARDLOCKUP_DETECTOR_BUDDY
depends on HARDLOCKUP_DETECTOR_PREFER_BUDDY || !HAVE_HARDLOCKUP_DETECTOR_PERF
depends on !HAVE_NMI_WATCHDOG
select HARDLOCKUP_DETECTOR_COUNTS_HRTIMER
# Both the "perf" and "buddy" hardlockup detectors need counting hrtimer
# interrupts.
config HARDLOCKUP_DETECTOR_COUNTS_HRTIMER
bool
depends on HARDLOCKUP_DETECTOR_PERF || HARDLOCKUP_DETECTOR_BUDDY
select SOFTLOCKUP_DETECTOR
--- cut ---
Also I am going to break the dependency between HAVE_NMI_WATCHDOG and
HAVE_HADRDLOCKUP_DETECTOR_ARCH. HAVE_NMI_WATCHDOG is needed only
for the very special powerpc64 watchdog. I am going to make sure
that it will be used only there and it will not be needed for
sparc and arm. As a result, we would have 4 separate implementations:
+ HAVE_HARDLOCKUP_DETECTOR_BUDDY enabled on any SMP system
+ HAVE_HARDLOCKUP_DETECTOR_PERF enabled on architectures supporting
this perf-based solution
+ HAVE_HARDLOCKUP_DETECTOR_ARCH enabled on architectures which
need another solution instead of the perf interface;
they would support the usual HARDLOCKUP_DETECTOR command
line parameters and sysctl interface
+ HAVE_NMI_WATCHDOG enabled just on powerpc64; it is special
solution with its own command line parameters. Also it does
not support hardlockup sysctl interface. I think about
renaming it to HAVE_HARDLOCKUP_DETECTOR_POWERPC64 or
_CUSTOM.
Best Regards,
Petr
