| Message ID | 20230526184139.10.I821fe7609e57608913fe05abd8f35b343e7a9aae@changeid |
|---|---|
| State | New |
| Headers | show |
| Series | watchdog: Cleanup / fixes after buddy series v5 reviews | expand |
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@chromium.org> > --- > > 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
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). -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
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. Signed-off-by: Douglas Anderson <dianders@chromium.org> --- lib/Kconfig.debug | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)