Re: [RFC PATCH 14/23] watchdog/hardlockup: Decouple the hardlockup detector from perf
On Thu, Jun 14, 2018 at 11:41:44AM +1000, Nicholas Piggin wrote: > On Wed, 13 Jun 2018 18:19:01 -0700 > Ricardo Neri wrote: > > > On Wed, Jun 13, 2018 at 10:43:24AM +0200, Peter Zijlstra wrote: > > > On Tue, Jun 12, 2018 at 05:57:34PM -0700, Ricardo Neri wrote: > > > > The current default implementation of the hardlockup detector assumes > > > > that > > > > it is implemented using perf events. > > > > > > The sparc and powerpc things are very much not using perf. > > > > Isn't it true that the current hardlockup detector > > (under kernel/watchdog_hld.c) is based on perf? > > arch/powerpc/kernel/watchdog.c is a powerpc implementation that uses > the kernel/watchdog_hld.c framework. > > > As far as I understand, > > this hardlockup detector is constructed using perf events for architectures > > that don't provide an NMI watchdog. Perhaps I can be more specific and say > > that this synthetized detector is based on perf. > > The perf detector is like that, but we want NMI watchdogs to share > the watchdog_hld code as much as possible even for arch specific NMI > watchdogs, so that kernel and user interfaces and behaviour are > consistent. > > Other arch watchdogs like sparc are a little older so they are not > using HLD. You don't have to change those for your series, but it > would be good to bring them into the fold if possible at some time. > IIRC sparc was slightly non-trivial because it has some differences > in sysctl or cmdline APIs that we don't want to break. > > But powerpc at least needs to be updated if you change hld apis. I will look into updating at least the powerpc implementation as part of these changes. Thanks and BR, Ricardo ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 14/23] watchdog/hardlockup: Decouple the hardlockup detector from perf
On Wed, 13 Jun 2018 18:19:01 -0700 Ricardo Neri wrote: > On Wed, Jun 13, 2018 at 10:43:24AM +0200, Peter Zijlstra wrote: > > On Tue, Jun 12, 2018 at 05:57:34PM -0700, Ricardo Neri wrote: > > > The current default implementation of the hardlockup detector assumes that > > > it is implemented using perf events. > > > > The sparc and powerpc things are very much not using perf. > > Isn't it true that the current hardlockup detector > (under kernel/watchdog_hld.c) is based on perf? arch/powerpc/kernel/watchdog.c is a powerpc implementation that uses the kernel/watchdog_hld.c framework. > As far as I understand, > this hardlockup detector is constructed using perf events for architectures > that don't provide an NMI watchdog. Perhaps I can be more specific and say > that this synthetized detector is based on perf. The perf detector is like that, but we want NMI watchdogs to share the watchdog_hld code as much as possible even for arch specific NMI watchdogs, so that kernel and user interfaces and behaviour are consistent. Other arch watchdogs like sparc are a little older so they are not using HLD. You don't have to change those for your series, but it would be good to bring them into the fold if possible at some time. IIRC sparc was slightly non-trivial because it has some differences in sysctl or cmdline APIs that we don't want to break. But powerpc at least needs to be updated if you change hld apis. Thanks, Nick ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 14/23] watchdog/hardlockup: Decouple the hardlockup detector from perf
On Wed, Jun 13, 2018 at 10:43:24AM +0200, Peter Zijlstra wrote: > On Tue, Jun 12, 2018 at 05:57:34PM -0700, Ricardo Neri wrote: > > The current default implementation of the hardlockup detector assumes that > > it is implemented using perf events. > > The sparc and powerpc things are very much not using perf. Isn't it true that the current hardlockup detector (under kernel/watchdog_hld.c) is based on perf? As far as I understand, this hardlockup detector is constructed using perf events for architectures that don't provide an NMI watchdog. Perhaps I can be more specific and say that this synthetized detector is based on perf. On a side note, I saw that powerpc might use a perf-based hardlockup detector if it has perf events [1]. Please let me know if my understanding is not correct. Thanks and BR, Ricardo [1]. https://elixir.bootlin.com/linux/v4.17/source/arch/powerpc/Kconfig#L218 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 14/23] watchdog/hardlockup: Decouple the hardlockup detector from perf
On Tue, Jun 12, 2018 at 05:57:34PM -0700, Ricardo Neri wrote: > The current default implementation of the hardlockup detector assumes that > it is implemented using perf events. The sparc and powerpc things are very much not using perf. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[RFC PATCH 14/23] watchdog/hardlockup: Decouple the hardlockup detector from perf
The current default implementation of the hardlockup detector assumes that
it is implemented using perf events. However, the hardlockup detector can
be driven by other sources of non-maskable interrupts (e.g., a properly
configured timer).
Put in a separate file all the code that is specific to perf: create and
manage events, stop and start the detector. This perf-specific code is put
in the new file watchdog_hld_perf.c
The code generic code used to monitor the timers' thresholds, check
timestamps and detect hardlockups remains in watchdog_hld.c
Functions and variables are simply relocated to a new file. No functional
changes were made.
Cc: Ashok Raj
Cc: Andi Kleen
Cc: Tony Luck
Cc: Borislav Petkov
Cc: Jacob Pan
Cc: Don Zickus
Cc: Nicholas Piggin
Cc: Michael Ellerman
Cc: Frederic Weisbecker
Cc: Babu Moger
Cc: "David S. Miller"
Cc: Benjamin Herrenschmidt
Cc: Paul Mackerras
Cc: Mathieu Desnoyers
Cc: Masami Hiramatsu
Cc: Peter Zijlstra
Cc: Andrew Morton
Cc: Philippe Ombredanne
Cc: Colin Ian King
Cc: "Luis R. Rodriguez"
Cc: "Ravi V. Shankar"
Cc: x...@kernel.org
Cc: sparcli...@vger.kernel.org
Cc: linuxppc-...@lists.ozlabs.org
Cc: iommu@lists.linux-foundation.org
Signed-off-by: Ricardo Neri
---
kernel/Makefile| 2 +-
kernel/watchdog_hld.c | 162
kernel/watchdog_hld_perf.c | 182 +
3 files changed, 183 insertions(+), 163 deletions(-)
create mode 100644 kernel/watchdog_hld_perf.c
diff --git a/kernel/Makefile b/kernel/Makefile
index f85ae5d..0a0d86d 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -85,7 +85,7 @@ obj-$(CONFIG_FAIL_FUNCTION) += fail_function.o
obj-$(CONFIG_KGDB) += debug/
obj-$(CONFIG_DETECT_HUNG_TASK) += hung_task.o
obj-$(CONFIG_LOCKUP_DETECTOR) += watchdog.o
-obj-$(CONFIG_HARDLOCKUP_DETECTOR_PERF) += watchdog_hld.o
+obj-$(CONFIG_HARDLOCKUP_DETECTOR_PERF) += watchdog_hld.o watchdog_hld_perf.o
obj-$(CONFIG_SECCOMP) += seccomp.o
obj-$(CONFIG_RELAY) += relay.o
obj-$(CONFIG_SYSCTL) += utsname_sysctl.o
diff --git a/kernel/watchdog_hld.c b/kernel/watchdog_hld.c
index 28a00c3..96615a2 100644
--- a/kernel/watchdog_hld.c
+++ b/kernel/watchdog_hld.c
@@ -22,12 +22,8 @@
static DEFINE_PER_CPU(bool, hard_watchdog_warn);
static DEFINE_PER_CPU(bool, watchdog_nmi_touch);
-static DEFINE_PER_CPU(struct perf_event *, watchdog_ev);
-static DEFINE_PER_CPU(struct perf_event *, dead_event);
-static struct cpumask dead_events_mask;
static unsigned long hardlockup_allcpu_dumped;
-static atomic_t watchdog_cpus = ATOMIC_INIT(0);
void arch_touch_nmi_watchdog(void)
{
@@ -98,14 +94,6 @@ static inline bool watchdog_check_timestamp(void)
}
#endif
-static struct perf_event_attr wd_hw_attr = {
- .type = PERF_TYPE_HARDWARE,
- .config = PERF_COUNT_HW_CPU_CYCLES,
- .size = sizeof(struct perf_event_attr),
- .pinned = 1,
- .disabled = 1,
-};
-
void inspect_for_hardlockups(struct pt_regs *regs)
{
if (__this_cpu_read(watchdog_nmi_touch) == true) {
@@ -155,153 +143,3 @@ void inspect_for_hardlockups(struct pt_regs *regs)
__this_cpu_write(hard_watchdog_warn, false);
return;
}
-
-/* Callback function for perf event subsystem */
-static void watchdog_overflow_callback(struct perf_event *event,
- struct perf_sample_data *data,
- struct pt_regs *regs)
-{
- /* Ensure the watchdog never gets throttled */
- event->hw.interrupts = 0;
- inspect_for_hardlockups(regs);
-}
-
-static int hardlockup_detector_event_create(void)
-{
- unsigned int cpu = smp_processor_id();
- struct perf_event_attr *wd_attr;
- struct perf_event *evt;
-
- wd_attr = _hw_attr;
- wd_attr->sample_period = hw_nmi_get_sample_period(watchdog_thresh);
-
- /* Try to register using hardware perf events */
- evt = perf_event_create_kernel_counter(wd_attr, cpu, NULL,
- watchdog_overflow_callback,
NULL);
- if (IS_ERR(evt)) {
- pr_info("Perf event create on CPU %d failed with %ld\n", cpu,
- PTR_ERR(evt));
- return PTR_ERR(evt);
- }
- this_cpu_write(watchdog_ev, evt);
- return 0;
-}
-
-/**
- * hardlockup_detector_perf_enable - Enable the local event
- */
-static void hardlockup_detector_perf_enable(void)
-{
- if (hardlockup_detector_event_create())
- return;
-
- /* use original value for check */
- if (!atomic_fetch_inc(_cpus))
- pr_info("Enabled. Permanently consumes one hw-PMU counter.\n");
-
- perf_event_enable(this_cpu_read(watchdog_ev));
-}
-
-/**
- * hardlockup_detector_perf_disable - Disable the local event
- */
-static void hardlockup_detector_perf_disable(void)
-{
- struct perf_event *event = this_cpu_read(watchdog_ev);
-
-
