Hi, On Thu, Jun 8, 2023 at 4:02 AM Petr Mladek <pmladek@xxxxxxxx> wrote: > > > > 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 || HAVE_HARDLOCKUP_DETECTOR_BUDDY) && !HAVE_NMI_WATCHDOG) || HAVE_HARDLOCKUP_DETECTOR_ARCH > > > > Adding the dependency to buddy (see ablove) would simplify the above > > to just this: > > > > depends on HAVE_HARDLOCKUP_DETECTOR_PERF || > > HAVE_HARDLOCKUP_DETECTOR_BUDDY || HAVE_HARDLOCKUP_DETECTOR_ARCH > > This is exactly what I do not want. It would just move the check > somewhere else. But it would make the logic harder to understand. Hmmm. To me, it felt easier to understand by moving this into the "HAVE_HARDLOCKUP_DETECTOR_BUDDY". To me it was pretty easy to say "if an architecture defined its own arch-specific watchdog then buddy can't be enabled" and that felt like it fit cleanly within the "HAVE_HARDLOCKUP_DETECTOR_BUDDY" definition. It got rid of _a lot_ of other special cases / checks elsewhere and felt quite a bit cleaner to me. I only had to think about the conflict between the "buddy" and "nmi" watchdogs once when I understood "HAVE_HARDLOCKUP_DETECTOR_BUDDY". > > As per above, it's simply a responsibility of architectures not to > > define that they have both "perf" if they have the NMI watchdog, so > > it's just buddy to worry about. > > Where is this documented, please? > Is it safe to assume this? It's not well documented and I agree that it could be improved. Right now, HAVE_NMI_WATCHDOG is documented to say that the architecture "defines its own arch_touch_nmi_watchdog()". Looking before my patches, you can see that "kernel/watchdog_hld.c" (the "perf" detector code) unconditionally defines arch_touch_nmi_watchdog(). That would give you a linker error. > I would personally prefer to ensure this by the config check. > It is even better than documentation because nobody reads > documentation ;-) Sure. IMO this should be documented as close as possible to the root of the problem. Make "HAVE_NMI_WATCHDOG" depend on "!HAVE_HARDLOCKUP_DETECTOR_PERF". That expresses that an architecture is not allowed to declare that it has both.
