watchdog_hld cleanup and async model for arm64
v3 cover watchdog_hld cleanup and async model for arm64
Hard lockup detector is helpful to diagnose unpaired irq enable/disable. But the current watchdog framework can not cope with arm64 hw perf event easily.
On arm64, when lockup_detector_init()->watchdog_nmi_probe(), PMU is not ready until device_initcall(armv8_pmu_driver_init). And it is deeply integrated with the driver model and cpuhp. Hence it is hard to push the initialization of armv8_pmu_driver_init() before smp_init().
But it is easy to take an opposite approach by enabling watchdog_hld to get the capability of PMU async.
The async model is achieved by expanding watchdog_nmi_probe() with -EBUSY, and a re-initializing work_struct which waits on a wait_queue_head.
In this series, [1-2/4] are trivial cleanup. [3-4/4] is for this async model.
v2 -> v3:
check the delay work waken up and flush the work before __initdata is free. improve the commit log of [4/4]
rebase to v5.15-rc5
v1 > v2:
uplift the async model from hard lockup layer to watchdog layter.
The benefit is simpler code, the drawback is re-initialize means wasted alloc/free.
Cc: Sumit Garg <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Marc Zyngier <[email protected]>
Cc: Kees Cook <[email protected]>
Cc: Masahiro Yamada <[email protected]>
Cc: Sami Tolvanen <[email protected]>
Cc: Petr Mladek <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Wang Qing <[email protected]>
Cc: "Peter Zijlstra (Intel)" <[email protected]>
Cc: Santosh Sivaraj <[email protected]>
To: [email protected]
To: [email protected]
*** BLURB HERE ***
Pingfan Liu (3):
kernel/watchdog: trival cleanups
kernel/watchdog_hld: Ensure CPU-bound context when creating hardlockup
detector event
kernel/watchdog: Adapt the watchdog_hld interface for async model
Sumit Garg (1):
arm64: Enable perf events based hard lockup detector
arch/arm64/Kconfig | 2 ++ arch/arm64/kernel/Makefile | 1 + arch/arm64/kernel/perf_event.c | 11 ++++-- arch/arm64/kernel/watchdog_hld.c | 36 +++++++++++++++++++ arch/sparc/kernel/nmi.c | 8 ++--- drivers/perf/arm_pmu.c | 5 +++ include/linux/nmi.h | 11 +++++- include/linux/perf/arm_pmu.h | 2 ++ kernel/watchdog.c | 62 ++++++++++++++++++++++++++++---- kernel/watchdog_hld.c | 5 ++- 10 files changed, 129 insertions(+), 14 deletions(-) create mode 100644 arch/arm64/kernel/watchdog_hld.c
Signature · 1 line
2.31.1
PATCH v3 1/4 kernel/watchdog: trival cleanups
No reference to WATCHDOG_DEFAULT, remove it.
And nobody cares about the return value of watchdog_nmi_enable(), changing its prototype to void.
--- arch/sparc/kernel/nmi.c | 8 ++++---- include/linux/nmi.h | 2 +- kernel/watchdog.c | 5 +---- 3 files changed, 6 insertions(+), 9 deletions(-)
arch/sparc/kernel/nmi.c +4 -4
@@ -282,11 +282,11 @@ __setup("nmi_watchdog=", setup_nmi_watchdog);* sparc specific NMI watchdog enable function. * Enables watchdog if it is not enabled already. */ -int watchdog_nmi_enable (unsigned int cpu) +void watchdog_nmi_enable (unsigned int cpu) {if (atomic_read (&nmi_active) ==-1 ) {pr_warn ("NMI watchdog cannot be enabled or disabled\n" ); -return -1 ; +return ; }/*
@@ -295,11 +295,11 @@ int watchdog_nmi_enable(unsigned int cpu)* process first. */ if (!nmi_init_done) -return 0 ; +return ;smp_call_function_single (cpu, start_nmi_watchdog,NULL ,1 ); -return 0 ; +return ; }/* * sparc specific NMI watchdog disable function.
include/linux/nmi.h +1 -1
@@ -119,7 +119,7 @@ static inline int hardlockup_detector_perf_init(void) { return 0; }void watchdog_nmi_stop (void );void watchdog_nmi_start (void );int watchdog_nmi_probe (void ); -int watchdog_nmi_enable (unsigned int cpu); +void watchdog_nmi_enable (unsigned int cpu);void watchdog_nmi_disable (unsigned int cpu);/**
kernel/watchdog.c +1 -4
@@ -30,10 +30,8 @@static DEFINE_MUTEX (watchdog_mutex);#if defined(CONFIG_HARDLOCKUP_DETECTOR ) || defined(CONFIG_HAVE_NMI_WATCHDOG ) -# define WATCHDOG_DEFAULT (SOFT_WATCHDOG_ENABLED | NMI_WATCHDOG_ENABLED)# define NMI_WATCHDOG_DEFAULT 1#else -# define WATCHDOG_DEFAULT (SOFT_WATCHDOG_ENABLED)# define NMI_WATCHDOG_DEFAULT 0#endif
@@ -95,10 +93,9 @@ __setup("nmi_watchdog=", hardlockup_panic_setup);* softlockup watchdog start and stop. The arch must select the * SOFTLOCKUP_DETECTOR Kconfig. */ -int __weak watchdog_nmi_enable (unsigned int cpu) +void __weak watchdog_nmi_enable (unsigned int cpu) {hardlockup_detector_perf_enable (); -return 0 ; }void __weak watchdog_nmi_disable (unsigned int cpu)
Signature · 1 line
2.31.1
PATCH v3 2/4 kernel/watchdog_hld: Ensure CPU-bound context when creating hardlockup detector event
In reply to Pingfan Liuhardlockup_detector_event_create() should create perf_event on the current CPU. Preemption could not get disabled because perf_event_create_kernel_counter() allocates memory. Instead, the CPU locality is achieved by processing the code in a per-CPU bound kthread.
Add a check to prevent mistakes when calling the code in another code path.
--- kernel/watchdog_hld.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
kernel/watchdog_hld.c +4 -1
@@ -165,10 +165,13 @@ static void watchdog_overflow_callback(struct perf_event *event,static int hardlockup_detector_event_create (void ) { -unsigned int cpu =smp_processor_id (); +unsigned int cpu;struct perf_event_attr *wd_attr;struct perf_event *evt; +/* This function plans to execute in cpu bound kthread */ +WARN_ON (!is_percpu_thread ()); + cpu =raw_smp_processor_id (); wd_attr = &wd_hw_attr; wd_attr->sample_period =hw_nmi_get_sample_period (watchdog_thresh);
Signature · 1 line
2.31.1
PATCH v3 3/4 kernel/watchdog: Adapt the watchdog_hld interface for async model
In reply to Pingfan LiuWhen lockup_detector_init()->watchdog_nmi_probe(), PMU may be not ready yet. E.g. on arm64, PMU is not ready until device_initcall(armv8_pmu_driver_init). And it is deeply integrated with the driver model and cpuhp. Hence it is hard to push this initialization before smp_init().
But it is easy to take an opposite approach by enabling watchdog_hld to get the capability of PMU async.
The async model is achieved by expanding watchdog_nmi_probe() with -EBUSY, and a re-initializing work_struct which waits on a wait_queue_head.
--- include/linux/nmi.h | 9 +++++++ kernel/watchdog.c | 57 +++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 64 insertions(+), 2 deletions(-)
include/linux/nmi.h +9 -0
@@ -118,6 +118,15 @@ static inline int hardlockup_detector_perf_init(void) { return 0; }void watchdog_nmi_stop (void );void watchdog_nmi_start (void ); + +enum hld_detector_state { +DELAY_INIT_NOP , +DELAY_INIT_WAIT , +DELAY_INIT_READY +}; + +extern enum hld_detector_state detector_delay_init_state; +extern struct wait_queue_head hld_detector_wait;int watchdog_nmi_probe (void );void watchdog_nmi_enable (unsigned int cpu);void watchdog_nmi_disable (unsigned int cpu);
kernel/watchdog.c +55 -2
77 lines not on this page · raw message
Signature · 1 line
2.31.1
PATCH v3 4/4 arm64: Enable perf events based hard lockup detector
In reply to Pingfan LiuFrom: Sumit Garg <[email protected]>
With the recent feature added to enable perf events to use pseudo NMIs as interrupts on platforms which support GICv3 or later, its now been possible to enable hard lockup detector (or NMI watchdog) on arm64 platforms. So enable corresponding support.
One thing to note here is that normally lockup detector is initialized just after the early initcalls but PMU on arm64 comes up much later as device_initcall(). To cope with that, overriding watchdog_nmi_probe() to let the watchdog framework know PMU not ready, and inform the framework to re-initialize lockup detection once PMU has been initialized.
[1]: http://lore.kernel.org/linux-arm-kernel/[email protected]
(Pingfan: adapt it to watchdog_hld async model based on [1])
--- arch/arm64/Kconfig | 2 ++ arch/arm64/kernel/Makefile | 1 + arch/arm64/kernel/perf_event.c | 11 ++++++++-- arch/arm64/kernel/watchdog_hld.c | 36 ++++++++++++++++++++++++++++++++ drivers/perf/arm_pmu.c | 5 +++++ include/linux/perf/arm_pmu.h | 2 ++ 6 files changed, 55 insertions(+), 2 deletions(-) create mode 100644 arch/arm64/kernel/watchdog_hld.c
arch/arm64/Kconfig +2 -0
@@ -189,6 +189,8 @@ config ARM64 select HAVE_NMI select HAVE_PATA_PLATFORM select HAVE_PERF_EVENTS + select HAVE_PERF_EVENTS_NMI if ARM64_PSEUDO_NMI + select HAVE_HARDLOCKUP_DETECTOR_PERF if PERF_EVENTS && HAVE_PERF_EVENTS_NMI select HAVE_PERF_REGS select HAVE_PERF_USER_STACK_DUMP select HAVE_REGS_AND_STACK_ACCESS_API
arch/arm64/kernel/Makefile +1 -0
@@ -46,6 +46,7 @@ obj-$(CONFIG_MODULES) += module.o obj-$(CONFIG_ARM64_MODULE_PLTS) += module-plts.o obj-$(CONFIG_PERF_EVENTS) += perf_regs.o perf_callchain.o obj-$(CONFIG_HW_PERF_EVENTS) += perf_event.o +obj-$(CONFIG_HARDLOCKUP_DETECTOR_PERF) += watchdog_hld.o obj-$(CONFIG_HAVE_HW_BREAKPOINT) += hw_breakpoint.o obj-$(CONFIG_CPU_PM) += sleep.o suspend.o obj-$(CONFIG_CPU_IDLE) += cpuidle.o
arch/arm64/kernel/perf_event.c +9 -2
@@ -23,6 +23,7 @@#include <linux/platform_device.h> #include <linux/sched_clock.h> #include <linux/smp.h> +#include <linux/nmi.h> /* ARMv8 Cortex-A53 specific event types. */ #define ARMV8_A53_PERFCTR_PREF_LINEFILL 0xC2
@@ -1284,10 +1285,16 @@ static struct platform_driver armv8_pmu_driver = {static int __init armv8_pmu_driver_init (void ) { +int ret; +if (acpi_disabled) -return platform_driver_register (&armv8_pmu_driver); + ret =platform_driver_register (&armv8_pmu_driver);else -return arm_pmu_acpi_probe (armv8_pmuv3_init); + ret =arm_pmu_acpi_probe (armv8_pmuv3_init); + + detector_delay_init_state =DELAY_INIT_READY ; +wake_up (&hld_detector_wait); +return ret; }device_initcall (armv8_pmu_driver_init)
arch/arm64/kernel/watchdog_hld.c new file +36 -0
@@ -0,0 +1,36 @@ +// SPDX-License-Identifier: GPL-2.0 +#include <linux/nmi.h> +#include <linux/cpufreq.h> +#include <linux/perf/arm_pmu.h> + +/* +* Safe maximum CPU frequency in case a particular platform doesn't implement +* cpufreq driver. Although, architecture doesn't put any restrictions on +* maximum frequency but 5 GHz seems to be safe maximum given the available +* Arm CPUs in the market which are clocked much less than 5 GHz. On the other +* hand, we can't make it much higher as it would lead to a large hard-lockup +* detection timeout on parts which are running slower (eg. 1GHz on +* Developerbox) and doesn't possess a cpufreq driver. +*/ +#define SAFE_MAX_CPU_FREQ 5000000000UL // 5 GHz +u64 hw_nmi_get_sample_period (int watchdog_thresh) +{ +unsigned int cpu =smp_processor_id (); +unsigned long max_cpu_freq; + + max_cpu_freq =cpufreq_get_hw_max_freq (cpu) *1000UL ; +if (!max_cpu_freq) + max_cpu_freq =SAFE_MAX_CPU_FREQ ; + +return (u64 )max_cpu_freq * watchdog_thresh; +} + +int __init watchdog_nmi_probe (void ) +{ +if (detector_delay_init_state !=DELAY_INIT_READY ) +return -EBUSY ; +else if (!arm_pmu_irq_is_nmi ()) +return -ENODEV ; + +return hardlockup_detector_perf_init (); +}
drivers/perf/arm_pmu.c +5 -0
@@ -697,6 +697,11 @@ static int armpmu_get_cpu_irq(struct arm_pmu *pmu, int cpu)return per_cpu (hw_events->irq, cpu); } +bool arm_pmu_irq_is_nmi (void ) +{ +return has_nmi; +} +/* * PMU hardware loses all context when a CPU goes offline. * When a CPU is hotplugged back in, since some hardware registers are
include/linux/perf/arm_pmu.h +2 -0
@@ -169,6 +169,8 @@ void kvm_host_pmu_init(struct arm_pmu *pmu);#define kvm_host_pmu_init (x) do { } while(0)#endif +bool arm_pmu_irq_is_nmi (void ); +/* Internal functions only for core arm_pmu code */ struct arm_pmu *armpmu_alloc (void );struct arm_pmu *armpmu_alloc_atomic (void );
Signature · 1 line
2.31.1
kernel test robot
build botRe: [PATCHv3 4/4] arm64: Enable perf events based hard lockup detector
I love your patch! Yet something to improve:
Hi Pingfan,
I love your patch! Yet something to improve:
[auto build test ERROR on soc/for-next]
[also build test ERROR on kvmarm/next linus/master v5.15-rc7] [cannot apply to arm64/for-next/core arm/for-next xilinx-xlnx/master] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Pingfan-Liu/watchdog_hld-cleanup-and-async-model-for-arm64/20211014-104443 base: https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git for-next config: arm64-randconfig-p001-20211016 (attached as .config) compiler: aarch64-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/4c319c41e49036be58068395965bd33a5de02396
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Pingfan-Liu/watchdog_hld-cleanup-and-async-model-for-arm64/20211014-104443 git checkout 4c319c41e49036be58068395965bd33a5de02396
# save the attached .config to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=arm64 SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
All errors (new ones prefixed by >>):
aarch64-linux-ld: arch/arm64/kernel/perf_event.o: in function `armv8_pmu_driver_init':
>> perf_event.c:(.init.text+0x20): undefined reference to `detector_delay_init_state' aarch64-linux-ld: arch/arm64/kernel/perf_event.o: relocation R_AARCH64_ADR_PREL_PG_HI21 against symbol `detector_delay_init_state' which may bind externally can not be used when making a shared object; recompile with -fPIC perf_event.c:(.init.text+0x20): dangerous relocation: unsupported relocation >> aarch64-linux-ld: perf_event.c:(.init.text+0x30): undefined reference to `detector_delay_init_state' >> aarch64-linux-ld: perf_event.c:(.init.text+0x34): undefined reference to `hld_detector_wait'
aarch64-linux-ld: arch/arm64/kernel/perf_event.o: relocation R_AARCH64_ADR_PREL_PG_HI21 against symbol `hld_detector_wait' which may bind externally can not be used when making a shared object; recompile with -fPIC perf_event.c:(.init.text+0x34): dangerous relocation: unsupported relocation
aarch64-linux-ld: perf_event.c:(.init.text+0x3c): undefined reference to `hld_detector_wait'
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
kernel test robot
build botRe: [PATCHv3 4/4] arm64: Enable perf events based hard lockup detector
In reply to Pingfan LiuI love your patch! Yet something to improve:
Hi Pingfan,
I love your patch! Yet something to improve:
[auto build test ERROR on soc/for-next]
[also build test ERROR on kvmarm/next linus/master v5.15-rc7 next-20211025] [cannot apply to arm64/for-next/core arm/for-next xilinx-xlnx/master] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Pingfan-Liu/watchdog_hld-cleanup-and-async-model-for-arm64/20211014-104443 base: https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git for-next config: arm64-randconfig-r012-20211025 (attached as .config) compiler: aarch64-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/4c319c41e49036be58068395965bd33a5de02396
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Pingfan-Liu/watchdog_hld-cleanup-and-async-model-for-arm64/20211014-104443 git checkout 4c319c41e49036be58068395965bd33a5de02396
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross ARCH=arm64
If you fix the issue, kindly add following tag as appropriate
All errors (new ones prefixed by >>):
arch/arm64/kernel/watchdog_hld.c: In function 'watchdog_nmi_probe':
>> arch/arm64/kernel/watchdog_hld.c:32:19: error: implicit declaration of function 'arm_pmu_irq_is_nmi' [-Werror=implicit-function-declaration]
32 | else if (!arm_pmu_irq_is_nmi())
| ^~~~~~~~~~~~~~~~~~cc1: some warnings being treated as errors
vim +/arm_pmu_irq_is_nmi +32 arch/arm64/kernel/watchdog_hld.c
27
28 int __init watchdog_nmi_probe(void)
29 {
30 if (detector_delay_init_state != DELAY_INIT_READY)
31 return -EBUSY;
> 32 else if (!arm_pmu_irq_is_nmi())---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
Hi Pingfan,
Is this thread sill in progress?
We are looking for the upstream solution for ARM64 Hardlockup detector.
I'd appreciate it if someone keep working on it,
if not, I can take over it.
thanks!
-Lecopzer
Quoted 4 lines · Lecopzer Chen
Hi Pingfan,
Is this thread sill in progress?
No, I am working on other topic at present, and this is not in my queue in near future.
Quoted 5 lines
We are looking for the upstream solution for ARM64 Hardlockup detector.
I'd appreciate it if someone keep working on it,
if not, I can take over it.
Be my guest, and hope you have great work. We badly wants hardlock up detector on arm64
Best Regards,
Pingfan