[PATCH v5 00/18] watchdog/hardlockup: Add the buddy hardlockup detector

Douglas Anderson
2023-05-19T17:21:10Z

This patch series adds the "buddy" hardlockup detector. In brief, the
buddy hardlockup detector can detect hardlockups without arch-level
support by having CPUs checkup on a "buddy" CPU periodically. All the
details are in the patch ("watchdog/hardlockup: detect hard lockups
using secondary (buddy) CPUs") and I encourage folks to reply to that
patch for general comments about this approach.

Note that in v3, this was a single patch [1]. It's now exploded into a
much larger series. The much larger series does a bunch of cleanup
that Petr requested in response to v3 [2] [3]. This new series ends up
sharing a lot more code with the perf lockup detector. It also tries
to bring a little sanity to some of the naming we had.

v5 of this series attempts to resolve comments made against v4. It
also fixes a bug that I had introduced between v3 and v4 where
watchdog_hardlockup_check() was reading watchdog_hardlockup_touch from
the wrong CPU. As somewhat expected there was debate about some of the
naming in the v4 responses. I've mostly tended to stick with Petr
Mladek's opinions here.

The fact that this series now touches across the whole lockup detector
codebase also caused conflicts with the series trying to add a arm64
perf-based hardlockup detector. That was a bit incovenient for me
since I was testing on arm64 and wanted to make sure that the perf
and buddy hardlockup detectors both compiled and worked. Because of
this, I've pulled the latest arm64 perf-based lockup detector patches
into my series. Specifically:
- Patches #1 - #3 of the arm64 perf-based lockup detector patches were
generic cleanups. I added them early in my series. IMO these should
just land.
- Patches #4 - #6 of the arm64 perf-based lockup detector patches were
less generic but still caused conflict with my series. I tacked them
to the end of my series after adapting them to my changes. However,
I don't really consider them part of this series and I'd be OK if
the series landed without them. For use cases I'm aware of the buddy
system is sufficient and I see no urgent need to land the arm64 perf
hardlockup support, though I also don't have any strong objections
to the patches.

I will note that this patch series currently has no conflicts with the
other patch series I posed recently adding support for pseudo-NMI
based backtraces [5] and the two patches series are meant to work
together.

Given the new design of this patch series, testing all combinations is
fairly difficult. I've attempted to make sure that all combinations of
CONFIG_ options are good, but it wouldn't surprise me if I missed
something. I apologize in advance and I'll do my best to fix any
problems that are found.

I'll also note that the CC list is pretty giant. Some of this is based
on get_maintainers and some of this is people I thought might be
interested. Now that this series is touching so many files, I've
stopped auto-CCing everyone that get_maintainers suggests. I'll reply
to v3 and point at this patch to make sure folks are aware of it, but
if I stopped CCing you and you want back on then please yell.

As far as I can tell, there's no true MAINTAINER listed for the
existing watchdog code. Assuming people don't hate this, maybe it
would go through Andrew Morton's tree? There is now some arch-specific
code for sparc and powerpc, but it's all still watchdog code so
hopefully that would still be fine to go through the same tree.

[1] https://lore.kernel.org/r/20230501082341.v3.1.I6bf789d21d0c3d75d382e7e51a804a7a51315f2c@changeid
[2] https://lore.kernel.org/r/ZFEqynvf5nqkzEvQ@alley
[3] https://lore.kernel.org/r/ZFErmshcrcikrSU1@alley
[4] https://lore.kernel.org/linux-arm-kernel/20220903093415.15850-1-lecopzer.chen@mediatek.com/
[5] https://lore.kernel.org/r/20230419225604.21204-1-dianders@chromium.org

Changes in v5:
- ("More properly prevent false ...") promoted to its own patch for v5.
- Add Nicholas's explanation of why this didn't break builds.
- Don't dump stack on the buddy CPU if we fail to backtrace the hung CPU.
- Fixed wrong __this_cpu to per_cpu (oops).
- Found a few more names / comments to change.
- Move side effect (timestamp check ordering) to its own patch.
- No longer rename touch_nmi_watchdog(), just add comments.
- Tried to make names more consistent as per v4 feedback.
- Use atomic_t for hrtimer_interrupts.
- watchdog_hardlockup_dumped_stacks => watchdog_hardlockup_all_cpu_dumped
- watchdog_hardlockup_interrupt_count() => watchdog_hardlockup_kick()
- watchdog_hardlockup_is_lockedup() => is_hardlockup()
- watchdog_hardlockup_perf.c => kernel/watchdog_hld.c in description.
- watchdog_hardlockup_processed => watchdog_hardlockup_warned
- watchdog_hardlockup_touch => watchdog_hardlockup_touched.

Changes in v4:
- ("Add a "cpu" param to watchdog_hardlockup_check()") new for v4.
- ("Add a weak function for an arch to detect ...") new for v4.
- ("Define dummy watchdog_update_hrtimer_threshold() ...") new for v4.
- ("Have the perf hardlockup use __weak ...") new for v4.
- ("Move perf hardlockup checking/panic ...") new for v4.
- ("Move perf hardlockup watchdog petting to watchdog.c") new for v4.
- ("Rename some "NMI watchdog" constants/function ...") new for v4.
- ("Rename touch_nmi_watchdog() to ...") new for v4.
- ("Rename watchdog_hld.c to watchdog_perf.c") new for v4.
- ("Style changes to watchdog_hardlockup_check ...") new for v4.
- Pulled ("Adapt the watchdog_hld interface ...") into my series for v4.
- Pulled ("Enable perf events based hard ...") into my series for v4.
- Pulled ("Ensure CPU-bound context when creating ...") into my series for v4.
- Pulled ("add hw_nmi_get_sample_period for ...") into my series for v4.
- Pulled ("change watchdog_nmi_enable() to void") into my series for v4.
- Pulled ("remove WATCHDOG_DEFAULT") into my series for v4.
- Reworked atop a whole pile of cleanups, as suggested by Petr.

Changes in v3:
- Added a note in commit message about the effect on idle.
- Cleaned up commit message pros/cons to be complete sentences.
- More cpu => CPU (in Kconfig and comments).
- No code changes other than comments.

Changes in v2:
- No code changes.
- Reworked description and Kconfig based on v1 discussion.
- cpu => CPU (in commit message).

Douglas Anderson (13):
watchdog/perf: Define dummy watchdog_update_hrtimer_threshold() on
correct config
watchdog/perf: More properly prevent false positives with turbo modes
watchdog/hardlockup: Add comments to touch_nmi_watchdog()
watchdog/perf: Rename watchdog_hld.c to watchdog_perf.c
watchdog/hardlockup: Move perf hardlockup checking/panic to common
watchdog.c
watchdog/hardlockup: Style changes to watchdog_hardlockup_check() /
is_hardlockup()
watchdog/hardlockup: Add a "cpu" param to watchdog_hardlockup_check()
watchdog/hardlockup: Move perf hardlockup watchdog petting to
watchdog.c
watchdog/hardlockup: Rename some "NMI watchdog" constants/function
watchdog/hardlockup: Have the perf hardlockup use __weak functions
more cleanly
watchdog/hardlockup: detect hard lockups using secondary (buddy) CPUs
watchdog/perf: Add a weak function for an arch to detect if perf can
use NMIs
arm64: Enable perf events based hard lockup detector

Lecopzer Chen (4):
watchdog: remove WATCHDOG_DEFAULT
watchdog/hardlockup: change watchdog_nmi_enable() to void
watchdog/perf: Adapt the watchdog_perf interface for async model
arm64: add hw_nmi_get_sample_period for preparation of lockup detector

Pingfan Liu (1):
watchdog/perf: Ensure CPU-bound context when creating hardlockup
detector event

arch/arm64/Kconfig | 2 +
arch/arm64/kernel/Makefile | 1 +
arch/arm64/kernel/watchdog_hld.c | 36 +++
arch/powerpc/include/asm/nmi.h | 4 +-
arch/powerpc/kernel/watchdog.c | 12 +-
arch/powerpc/platforms/pseries/mobility.c | 4 +-
arch/sparc/kernel/nmi.c | 10 +-
drivers/perf/arm_pmu.c | 5 +
drivers/perf/arm_pmuv3.c | 12 +-
include/linux/nmi.h | 73 +++--
include/linux/perf/arm_pmu.h | 2 +
kernel/Makefile | 3 +-
kernel/watchdog.c | 344 +++++++++++++++------
kernel/watchdog_buddy.c | 93 ++++++
kernel/{watchdog_hld.c => watchdog_perf.c} | 105 +++----
lib/Kconfig.debug | 52 +++-
16 files changed, 550 insertions(+), 208 deletions(-)
create mode 100644 arch/arm64/kernel/watchdog_hld.c
create mode 100644 kernel/watchdog_buddy.c
rename kernel/{watchdog_hld.c => watchdog_perf.c} (72%)