[1/7] watchdog/hardlockup: Sort hardlockup detector related config values a logical way
Commit Message
There are four possible variants of hardlockup detectors:
+ buddy: available when SMP is set.
+ perf: available when HAVE_HARDLOCKUP_DETECTOR_PERF is set.
+ arch-specific: available when HAVE_HARDLOCKUP_DETECTOR_ARCH is set.
+ sparc64 special variant: available when HAVE_NMI_WATCHDOG is set
and HAVE_HARDLOCKUP_DETECTOR_ARCH is not set.
Only one hardlockup detector can be compiled in. The selection is done
using quite complex dependencies between several CONFIG variables.
The following patches will try to make it more straightforward.
As a first step, reorder the definitions of the various CONFIG variables.
The logical order is:
1. HAVE_* variables define available variants. They are typically
defined in the arch/ config files.
2. HARDLOCKUP_DETECTOR y/n variable defines whether the hardlockup
detector is enabled at all.
3. HARDLOCKUP_DETECTOR_PREFER_BUDDY y/n variable defines whether
the buddy detector should be preferred over the perf one.
Note that the arch specific variants are always preferred when
available.
4. HARDLOCKUP_DETECTOR_PERF/BUDDY variables define whether the given
detector is enabled in the end.
5. HAVE_HARDLOCKUP_DETECTOR_NON_ARCH and HARDLOCKUP_DETECTOR_NON_ARCH
are temporary variables that are going to be removed in
a followup patch.
The patch just shuffles the definitions. It should not change the existing
behavior.
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
lib/Kconfig.debug | 112 +++++++++++++++++++++++-----------------------
1 file changed, 56 insertions(+), 56 deletions(-)
Comments
Hi,
On Wed, Jun 7, 2023 at 8:25 AM Petr Mladek <pmladek@suse.com> wrote:
>
> There are four possible variants of hardlockup detectors:
>
> + buddy: available when SMP is set.
>
> + perf: available when HAVE_HARDLOCKUP_DETECTOR_PERF is set.
>
> + arch-specific: available when HAVE_HARDLOCKUP_DETECTOR_ARCH is set.
>
> + sparc64 special variant: available when HAVE_NMI_WATCHDOG is set
> and HAVE_HARDLOCKUP_DETECTOR_ARCH is not set.
>
> Only one hardlockup detector can be compiled in. The selection is done
> using quite complex dependencies between several CONFIG variables.
> The following patches will try to make it more straightforward.
>
> As a first step, reorder the definitions of the various CONFIG variables.
> The logical order is:
>
> 1. HAVE_* variables define available variants. They are typically
> defined in the arch/ config files.
>
> 2. HARDLOCKUP_DETECTOR y/n variable defines whether the hardlockup
> detector is enabled at all.
>
> 3. HARDLOCKUP_DETECTOR_PREFER_BUDDY y/n variable defines whether
> the buddy detector should be preferred over the perf one.
> Note that the arch specific variants are always preferred when
> available.
>
> 4. HARDLOCKUP_DETECTOR_PERF/BUDDY variables define whether the given
> detector is enabled in the end.
>
> 5. HAVE_HARDLOCKUP_DETECTOR_NON_ARCH and HARDLOCKUP_DETECTOR_NON_ARCH
> are temporary variables that are going to be removed in
> a followup patch.
>
> The patch just shuffles the definitions. It should not change the existing
> behavior.
>
> Signed-off-by: Petr Mladek <pmladek@suse.com>
> ---
> lib/Kconfig.debug | 112 +++++++++++++++++++++++-----------------------
> 1 file changed, 56 insertions(+), 56 deletions(-)
I don't really have any strong opinions, so I'm fine with this. In
general I think the ordering I picked tried to match the existing
"style" which generally tried to list configs and then select them
below. To me the existing style makes more sense when thinking about
writing C code without having to put a pile of prototypes at the top
of your file: you define things higher in the file and reference them
below. For instance, the old style (before any of my patches) had:
config SOFTLOCKUP_DETECTOR:
... blah blah blah ...
config HARDLOCKUP_DETECTOR_PERF:
select SOFTLOCKUP_DETECTOR
config HARDLOCKUP_DETECTOR:
... blah blah blah ...
select LOCKUP_DETECTOR
select HARDLOCKUP_DETECTOR_PERF if HAVE_HARDLOCKUP_DETECTOR_PERF
Your new style seems to be the opposite of that.
-Doug
On Wed 2023-06-07 16:34:20, Doug Anderson wrote:
> Hi,
>
> On Wed, Jun 7, 2023 at 8:25 AM Petr Mladek <pmladek@suse.com> wrote:
> > Only one hardlockup detector can be compiled in. The selection is done
> > using quite complex dependencies between several CONFIG variables.
> > The following patches will try to make it more straightforward.
> >
> > As a first step, reorder the definitions of the various CONFIG variables.
> > The logical order is:
> >
> > 1. HAVE_* variables define available variants. They are typically
> > defined in the arch/ config files.
> >
> > 2. HARDLOCKUP_DETECTOR y/n variable defines whether the hardlockup
> > detector is enabled at all.
> >
> > 3. HARDLOCKUP_DETECTOR_PREFER_BUDDY y/n variable defines whether
> > the buddy detector should be preferred over the perf one.
> > Note that the arch specific variants are always preferred when
> > available.
> >
> > 4. HARDLOCKUP_DETECTOR_PERF/BUDDY variables define whether the given
> > detector is enabled in the end.
> >
> > 5. HAVE_HARDLOCKUP_DETECTOR_NON_ARCH and HARDLOCKUP_DETECTOR_NON_ARCH
> > are temporary variables that are going to be removed in
> > a followup patch.
> >
>
> I don't really have any strong opinions, so I'm fine with this. In
> general I think the ordering I picked tried to match the existing
> "style" which generally tried to list configs and then select them
> below. To me the existing style makes more sense when thinking about
> writing C code
I know. My motivation was the following:
1. Kconfig is not C. My view is that it is more like a menu. There is a
top level item. If the top level is enabled then there is a submenu
with a more detailed selection of various variants and options.
2. The current logic is quite complicated from my POV. And it was
even before your patchset. For example,
HAVE_HARDLOCKUP_DETECTOR_BUDDY is defined as:
config HAVE_HARDLOCKUP_DETECTOR_BUDDY
bool
depends on SMP
default y
One would expect that it would be enabled on SMP system.
But the final value depends on many other variables
which are defined using relatively complex conditions,
especially HARDLOCKUP_DETECTOR, HAVE_HARDLOCKUP_DETECTOR_NON_ARCH,
and HARDLOCKUP_DETECTOR_NON_ARCH.
Understanding the logic is even more complicated because Kconfig is
not indexed by cscope.
Important: The logic used at the end of the patchset actually
follows the C style. It defines how the various variables
depend on each other from top to bottom.
>
> config SOFTLOCKUP_DETECTOR:
> ... blah blah blah ...
This one is actually defined in the menu-like order:
config SOFTLOCKUP_DETECTOR
config BOOTPARAM_SOFTLOCKUP_PANIC
depends on SOFTLOCKUP_DETECTOR
It is because the custom option depends on the top level one.
This is exactly what I would like to achieve with HARDLOCKUP
variables in this patchset.
Best Regards,
Petr
@@ -1035,62 +1035,6 @@ config BOOTPARAM_SOFTLOCKUP_PANIC
Say N if unsure.
-# Both the "perf" and "buddy" hardlockup detectors count hrtimer
-# interrupts. This config enables functions managing this common code.
-config HARDLOCKUP_DETECTOR_COUNTS_HRTIMER
- bool
- select SOFTLOCKUP_DETECTOR
-
-config HARDLOCKUP_DETECTOR_PERF
- bool
- depends on HAVE_HARDLOCKUP_DETECTOR_PERF
- select HARDLOCKUP_DETECTOR_COUNTS_HRTIMER
-
-config HARDLOCKUP_DETECTOR_BUDDY
- bool
- depends on SMP
- select HARDLOCKUP_DETECTOR_COUNTS_HRTIMER
-
-# For hardlockup detectors you can have one directly provided by the arch
-# or use a "non-arch" one. If you're using a "non-arch" one that is
-# further divided the perf hardlockup detector (which, confusingly, needs
-# arch-provided perf support) and the buddy hardlockup detector (which just
-# 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).
-config HAVE_HARDLOCKUP_DETECTOR_NON_ARCH
- bool
- depends on (HAVE_HARDLOCKUP_DETECTOR_PERF || SMP) && !HAVE_NMI_WATCHDOG
- default y
-
-config HARDLOCKUP_DETECTOR_PREFER_BUDDY
- bool "Prefer the buddy CPU hardlockup detector"
- depends on HAVE_HARDLOCKUP_DETECTOR_PERF && SMP
- 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.
-
-# This will select the appropriate non-arch hardlockdup detector
-config HARDLOCKUP_DETECTOR_NON_ARCH
- bool
- depends on HAVE_HARDLOCKUP_DETECTOR_NON_ARCH
- 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
-
-#
-# Enables a timestamp based low pass filter to compensate for perf based
-# hard lockup detection which runs too fast due to turbo modes.
-#
-config HARDLOCKUP_CHECK_TIMESTAMP
- bool
-
#
# arch/ can define HAVE_HARDLOCKUP_DETECTOR_ARCH to provide their own hard
# lockup detector rather than the perf based detector.
@@ -1111,6 +1055,62 @@ config HARDLOCKUP_DETECTOR
chance to run. The current stack trace is displayed upon detection
and the system will stay locked up.
+config HARDLOCKUP_DETECTOR_PREFER_BUDDY
+ bool "Prefer the buddy CPU hardlockup detector"
+ depends on HAVE_HARDLOCKUP_DETECTOR_PERF && SMP
+ 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 HAVE_HARDLOCKUP_DETECTOR_PERF
+ select HARDLOCKUP_DETECTOR_COUNTS_HRTIMER
+
+config HARDLOCKUP_DETECTOR_BUDDY
+ bool
+ depends on SMP
+ select HARDLOCKUP_DETECTOR_COUNTS_HRTIMER
+
+# Both the "perf" and "buddy" hardlockup detectors count hrtimer
+# interrupts. This config enables functions managing this common code.
+config HARDLOCKUP_DETECTOR_COUNTS_HRTIMER
+ bool
+ select SOFTLOCKUP_DETECTOR
+
+# For hardlockup detectors you can have one directly provided by the arch
+# or use a "non-arch" one. If you're using a "non-arch" one that is
+# further divided the perf hardlockup detector (which, confusingly, needs
+# arch-provided perf support) and the buddy hardlockup detector (which just
+# 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).
+config HAVE_HARDLOCKUP_DETECTOR_NON_ARCH
+ bool
+ depends on (HAVE_HARDLOCKUP_DETECTOR_PERF || SMP) && !HAVE_NMI_WATCHDOG
+ default y
+
+# This will select the appropriate non-arch hardlockdup detector
+config HARDLOCKUP_DETECTOR_NON_ARCH
+ bool
+ depends on HAVE_HARDLOCKUP_DETECTOR_NON_ARCH
+ 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
+
+#
+# Enables a timestamp based low pass filter to compensate for perf based
+# hard lockup detection which runs too fast due to turbo modes.
+#
+config HARDLOCKUP_CHECK_TIMESTAMP
+ bool
+
config BOOTPARAM_HARDLOCKUP_PANIC
bool "Panic (Reboot) On Hard Lockups"
depends on HARDLOCKUP_DETECTOR