mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v4] watchdog: Allow nmi watchdog to use "ref-cycles" event
@ 2023-05-18  0:25 Song Liu
  2023-05-18  6:44 ` Yonghong Song
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Song Liu @ 2023-05-18  0:25 UTC (permalink / raw)
  To: linux-kernel; +Cc: kernel-team, Song Liu, Andrew Morton, Peter Zijlstra

NMI watchdog permanently consumes one hardware counters per CPU on the
system. For systems that use many hardware counters, this causes more
aggressive time multiplexing of perf events.

OTOH, some CPUs (mostly Intel) support "ref-cycles" event, which is rarely
used. Add kernel cmdline arg nmi_watchdog=ref-cycles to configure the
watchdog to use "ref-cycles" event instead of "cycles".

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Song Liu <song@kernel.org>

---
Changes in v4:
Fix compile error for !CONFIG_HARDLOCKUP_DETECTOR_PERF. (kernel test bot)

Changes in v3:

Pivot the design to use kernel arg nmi_watchdog=ref-cycles (Peter)
---
 Documentation/admin-guide/kernel-parameters.txt | 5 +++--
 include/linux/nmi.h                             | 2 ++
 kernel/watchdog.c                               | 2 ++
 kernel/watchdog_hld.c                           | 9 +++++++++
 4 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 9e5bab29685f..d378e23dad7c 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -3593,10 +3593,12 @@
 			Format: [state][,regs][,debounce][,die]
 
 	nmi_watchdog=	[KNL,BUGS=X86] Debugging features for SMP kernels
-			Format: [panic,][nopanic,][num]
+			Format: [panic,][nopanic,][ref-cycles][num]
 			Valid num: 0 or 1
 			0 - turn hardlockup detector in nmi_watchdog off
 			1 - turn hardlockup detector in nmi_watchdog on
+			ref-cycles - configure the watchdog with perf event
+			             "ref-cycles" instead of "cycles"
 			When panic is specified, panic when an NMI watchdog
 			timeout occurs (or 'nopanic' to not panic on an NMI
 			watchdog, if CONFIG_BOOTPARAM_HARDLOCKUP_PANIC is set)
@@ -7097,4 +7099,3 @@
 				memory, and other data can't be written using
 				xmon commands.
 			off	xmon is disabled.
-
diff --git a/include/linux/nmi.h b/include/linux/nmi.h
index 048c0b9aa623..edfd1bcce0f6 100644
--- a/include/linux/nmi.h
+++ b/include/linux/nmi.h
@@ -102,12 +102,14 @@ extern void hardlockup_detector_perf_disable(void);
 extern void hardlockup_detector_perf_enable(void);
 extern void hardlockup_detector_perf_cleanup(void);
 extern int hardlockup_detector_perf_init(void);
+extern void hardlockup_config_perf_event(const char *str);
 #else
 static inline void hardlockup_detector_perf_stop(void) { }
 static inline void hardlockup_detector_perf_restart(void) { }
 static inline void hardlockup_detector_perf_disable(void) { }
 static inline void hardlockup_detector_perf_enable(void) { }
 static inline void hardlockup_detector_perf_cleanup(void) { }
+static inline void hardlockup_config_perf_event(const char *str) { }
 # if !defined(CONFIG_HAVE_NMI_WATCHDOG)
 static inline int hardlockup_detector_perf_init(void) { return -ENODEV; }
 static inline void arch_touch_nmi_watchdog(void) {}
diff --git a/kernel/watchdog.c b/kernel/watchdog.c
index 8e61f21e7e33..fed4f0be8e1a 100644
--- a/kernel/watchdog.c
+++ b/kernel/watchdog.c
@@ -81,6 +81,8 @@ static int __init hardlockup_panic_setup(char *str)
 		nmi_watchdog_user_enabled = 0;
 	else if (!strncmp(str, "1", 1))
 		nmi_watchdog_user_enabled = 1;
+	else if (!strncmp(str, "ref-cycles", 10))
+		hardlockup_config_perf_event(str);
 	return 1;
 }
 __setup("nmi_watchdog=", hardlockup_panic_setup);
diff --git a/kernel/watchdog_hld.c b/kernel/watchdog_hld.c
index 247bf0b1582c..4deca58ba6ed 100644
--- a/kernel/watchdog_hld.c
+++ b/kernel/watchdog_hld.c
@@ -294,3 +294,12 @@ int __init hardlockup_detector_perf_init(void)
 	}
 	return ret;
 }
+
+/**
+ * hardlockup_config_perf_event - Overwrite config of wd_hw_attr
+ */
+void __init hardlockup_config_perf_event(const char *str)
+{
+	if (!strncmp(str, "ref-cycles", 10))
+		wd_hw_attr.config = PERF_COUNT_HW_REF_CPU_CYCLES;
+}
-- 
2.34.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v4] watchdog: Allow nmi watchdog to use "ref-cycles" event
  2023-05-18  0:25 [PATCH v4] watchdog: Allow nmi watchdog to use "ref-cycles" event Song Liu
@ 2023-05-18  6:44 ` Yonghong Song
  2023-05-18 20:12   ` Song Liu
  2023-05-19 16:59 ` Song Liu
  2023-06-02 22:47 ` Peter Zijlstra
  2 siblings, 1 reply; 9+ messages in thread
From: Yonghong Song @ 2023-05-18  6:44 UTC (permalink / raw)
  To: Song Liu, linux-kernel; +Cc: kernel-team, Andrew Morton, Peter Zijlstra



On 5/17/23 5:25 PM, Song Liu wrote:
> NMI watchdog permanently consumes one hardware counters per CPU on the
> system. For systems that use many hardware counters, this causes more
> aggressive time multiplexing of perf events.
> 
> OTOH, some CPUs (mostly Intel) support "ref-cycles" event, which is rarely
> used. Add kernel cmdline arg nmi_watchdog=ref-cycles to configure the
> watchdog to use "ref-cycles" event instead of "cycles".

Maybe list some example how this new option will used?

> 
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Song Liu <song@kernel.org>
> 
> ---
> Changes in v4:
> Fix compile error for !CONFIG_HARDLOCKUP_DETECTOR_PERF. (kernel test bot)
> 
> Changes in v3:
> 
> Pivot the design to use kernel arg nmi_watchdog=ref-cycles (Peter)
> ---
>   Documentation/admin-guide/kernel-parameters.txt | 5 +++--
>   include/linux/nmi.h                             | 2 ++
>   kernel/watchdog.c                               | 2 ++
>   kernel/watchdog_hld.c                           | 9 +++++++++
>   4 files changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 9e5bab29685f..d378e23dad7c 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -3593,10 +3593,12 @@
>   			Format: [state][,regs][,debounce][,die]
>   
>   	nmi_watchdog=	[KNL,BUGS=X86] Debugging features for SMP kernels
> -			Format: [panic,][nopanic,][num]
> +			Format: [panic,][nopanic,][ref-cycles][num]
>   			Valid num: 0 or 1
>   			0 - turn hardlockup detector in nmi_watchdog off
>   			1 - turn hardlockup detector in nmi_watchdog on
> +			ref-cycles - configure the watchdog with perf event
> +			             "ref-cycles" instead of "cycles"
>   			When panic is specified, panic when an NMI watchdog
>   			timeout occurs (or 'nopanic' to not panic on an NMI
>   			watchdog, if CONFIG_BOOTPARAM_HARDLOCKUP_PANIC is set)
> @@ -7097,4 +7099,3 @@
>   				memory, and other data can't be written using
>   				xmon commands.
>   			off	xmon is disabled.
> -
> diff --git a/include/linux/nmi.h b/include/linux/nmi.h
> index 048c0b9aa623..edfd1bcce0f6 100644
> --- a/include/linux/nmi.h
> +++ b/include/linux/nmi.h
> @@ -102,12 +102,14 @@ extern void hardlockup_detector_perf_disable(void);
>   extern void hardlockup_detector_perf_enable(void);
>   extern void hardlockup_detector_perf_cleanup(void);
>   extern int hardlockup_detector_perf_init(void);
> +extern void hardlockup_config_perf_event(const char *str);
>   #else
>   static inline void hardlockup_detector_perf_stop(void) { }
>   static inline void hardlockup_detector_perf_restart(void) { }
>   static inline void hardlockup_detector_perf_disable(void) { }
>   static inline void hardlockup_detector_perf_enable(void) { }
>   static inline void hardlockup_detector_perf_cleanup(void) { }
> +static inline void hardlockup_config_perf_event(const char *str) { }
>   # if !defined(CONFIG_HAVE_NMI_WATCHDOG)
>   static inline int hardlockup_detector_perf_init(void) { return -ENODEV; }
>   static inline void arch_touch_nmi_watchdog(void) {}
> diff --git a/kernel/watchdog.c b/kernel/watchdog.c
> index 8e61f21e7e33..fed4f0be8e1a 100644
> --- a/kernel/watchdog.c
> +++ b/kernel/watchdog.c
> @@ -81,6 +81,8 @@ static int __init hardlockup_panic_setup(char *str)
>   		nmi_watchdog_user_enabled = 0;
>   	else if (!strncmp(str, "1", 1))
>   		nmi_watchdog_user_enabled = 1;
> +	else if (!strncmp(str, "ref-cycles", 10))

str vs. 'ref-cycles' is tested here.

> +		hardlockup_config_perf_event(str);
>   	return 1;
>   }
>   __setup("nmi_watchdog=", hardlockup_panic_setup);
> diff --git a/kernel/watchdog_hld.c b/kernel/watchdog_hld.c
> index 247bf0b1582c..4deca58ba6ed 100644
> --- a/kernel/watchdog_hld.c
> +++ b/kernel/watchdog_hld.c
> @@ -294,3 +294,12 @@ int __init hardlockup_detector_perf_init(void)
>   	}
>   	return ret;
>   }
> +
> +/**
> + * hardlockup_config_perf_event - Overwrite config of wd_hw_attr
> + */
> +void __init hardlockup_config_perf_event(const char *str)
> +{
> +	if (!strncmp(str, "ref-cycles", 10))

It is unnecessarily tested again here.

> +		wd_hw_attr.config = PERF_COUNT_HW_REF_CPU_CYCLES;
> +}

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v4] watchdog: Allow nmi watchdog to use "ref-cycles" event
  2023-05-18  6:44 ` Yonghong Song
@ 2023-05-18 20:12   ` Song Liu
  0 siblings, 0 replies; 9+ messages in thread
From: Song Liu @ 2023-05-18 20:12 UTC (permalink / raw)
  To: Yonghong Song; +Cc: Song Liu, LKML, Kernel Team, Andrew Morton, Peter Zijlstra



> On May 17, 2023, at 11:44 PM, Yonghong Song <yhs@meta.com> wrote:
> 
> 
> 
> On 5/17/23 5:25 PM, Song Liu wrote:
>> NMI watchdog permanently consumes one hardware counters per CPU on the
>> system. For systems that use many hardware counters, this causes more
>> aggressive time multiplexing of perf events.
>> OTOH, some CPUs (mostly Intel) support "ref-cycles" event, which is rarely
>> used. Add kernel cmdline arg nmi_watchdog=ref-cycles to configure the
>> watchdog to use "ref-cycles" event instead of "cycles".
> 
> Maybe list some example how this new option will used?

In most case, all we need is to add "nmi_watchdog=refcycles" to kernel args.

> 
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Cc: Peter Zijlstra <peterz@infradead.org>
>> Signed-off-by: Song Liu <song@kernel.org>
>> ---
>> Changes in v4:
>> Fix compile error for !CONFIG_HARDLOCKUP_DETECTOR_PERF. (kernel test bot)
>> Changes in v3:
>> Pivot the design to use kernel arg nmi_watchdog=ref-cycles (Peter)
>> ---
>>  Documentation/admin-guide/kernel-parameters.txt | 5 +++--
>>  include/linux/nmi.h                             | 2 ++
>>  kernel/watchdog.c                               | 2 ++
>>  kernel/watchdog_hld.c                           | 9 +++++++++
>>  4 files changed, 16 insertions(+), 2 deletions(-)
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>> index 9e5bab29685f..d378e23dad7c 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -3593,10 +3593,12 @@
>>   Format: [state][,regs][,debounce][,die]
>>     nmi_watchdog= [KNL,BUGS=X86] Debugging features for SMP kernels
>> - Format: [panic,][nopanic,][num]
>> + Format: [panic,][nopanic,][ref-cycles][num]
>>   Valid num: 0 or 1
>>   0 - turn hardlockup detector in nmi_watchdog off
>>   1 - turn hardlockup detector in nmi_watchdog on
>> + ref-cycles - configure the watchdog with perf event
>> +              "ref-cycles" instead of "cycles"
>>   When panic is specified, panic when an NMI watchdog
>>   timeout occurs (or 'nopanic' to not panic on an NMI
>>   watchdog, if CONFIG_BOOTPARAM_HARDLOCKUP_PANIC is set)
>> @@ -7097,4 +7099,3 @@
>>   memory, and other data can't be written using
>>   xmon commands.
>>   off xmon is disabled.
>> -
>> diff --git a/include/linux/nmi.h b/include/linux/nmi.h
>> index 048c0b9aa623..edfd1bcce0f6 100644
>> --- a/include/linux/nmi.h
>> +++ b/include/linux/nmi.h
>> @@ -102,12 +102,14 @@ extern void hardlockup_detector_perf_disable(void);
>>  extern void hardlockup_detector_perf_enable(void);
>>  extern void hardlockup_detector_perf_cleanup(void);
>>  extern int hardlockup_detector_perf_init(void);
>> +extern void hardlockup_config_perf_event(const char *str);
>>  #else
>>  static inline void hardlockup_detector_perf_stop(void) { }
>>  static inline void hardlockup_detector_perf_restart(void) { }
>>  static inline void hardlockup_detector_perf_disable(void) { }
>>  static inline void hardlockup_detector_perf_enable(void) { }
>>  static inline void hardlockup_detector_perf_cleanup(void) { }
>> +static inline void hardlockup_config_perf_event(const char *str) { }
>>  # if !defined(CONFIG_HAVE_NMI_WATCHDOG)
>>  static inline int hardlockup_detector_perf_init(void) { return -ENODEV; }
>>  static inline void arch_touch_nmi_watchdog(void) {}
>> diff --git a/kernel/watchdog.c b/kernel/watchdog.c
>> index 8e61f21e7e33..fed4f0be8e1a 100644
>> --- a/kernel/watchdog.c
>> +++ b/kernel/watchdog.c
>> @@ -81,6 +81,8 @@ static int __init hardlockup_panic_setup(char *str)
>>   nmi_watchdog_user_enabled = 0;
>>   else if (!strncmp(str, "1", 1))
>>   nmi_watchdog_user_enabled = 1;
>> + else if (!strncmp(str, "ref-cycles", 10))
> 
> str vs. 'ref-cycles' is tested here.
> 
>> + hardlockup_config_perf_event(str);
>>   return 1;
>>  }
>>  __setup("nmi_watchdog=", hardlockup_panic_setup);
>> diff --git a/kernel/watchdog_hld.c b/kernel/watchdog_hld.c
>> index 247bf0b1582c..4deca58ba6ed 100644
>> --- a/kernel/watchdog_hld.c
>> +++ b/kernel/watchdog_hld.c
>> @@ -294,3 +294,12 @@ int __init hardlockup_detector_perf_init(void)
>>   }
>>   return ret;
>>  }
>> +
>> +/**
>> + * hardlockup_config_perf_event - Overwrite config of wd_hw_attr
>> + */
>> +void __init hardlockup_config_perf_event(const char *str)
>> +{
>> + if (!strncmp(str, "ref-cycles", 10))
> 
> It is unnecessarily tested again here.

In that case, we can probably rename the function as 
hardlockup_use_ref_cycles(). I am fine with either way. 

Thanks,
Song


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v4] watchdog: Allow nmi watchdog to use "ref-cycles" event
  2023-05-18  0:25 [PATCH v4] watchdog: Allow nmi watchdog to use "ref-cycles" event Song Liu
  2023-05-18  6:44 ` Yonghong Song
@ 2023-05-19 16:59 ` Song Liu
  2023-05-25 22:20   ` Song Liu
  2023-06-02 22:47 ` Peter Zijlstra
  2 siblings, 1 reply; 9+ messages in thread
From: Song Liu @ 2023-05-19 16:59 UTC (permalink / raw)
  To: Song Liu; +Cc: LKML, Kernel Team, Andrew Morton, Peter Zijlstra

Hi Andrew and Peter, 

Does this version look good do you?

Thanks,
Song

> On May 17, 2023, at 5:25 PM, Song Liu <song@kernel.org> wrote:
> 
> NMI watchdog permanently consumes one hardware counters per CPU on the
> system. For systems that use many hardware counters, this causes more
> aggressive time multiplexing of perf events.
> 
> OTOH, some CPUs (mostly Intel) support "ref-cycles" event, which is rarely
> used. Add kernel cmdline arg nmi_watchdog=ref-cycles to configure the
> watchdog to use "ref-cycles" event instead of "cycles".
> 
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Song Liu <song@kernel.org>
> 
> ---
> Changes in v4:
> Fix compile error for !CONFIG_HARDLOCKUP_DETECTOR_PERF. (kernel test bot)
> 
> Changes in v3:
> 
> Pivot the design to use kernel arg nmi_watchdog=ref-cycles (Peter)
> ---
> Documentation/admin-guide/kernel-parameters.txt | 5 +++--
> include/linux/nmi.h                             | 2 ++
> kernel/watchdog.c                               | 2 ++
> kernel/watchdog_hld.c                           | 9 +++++++++
> 4 files changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 9e5bab29685f..d378e23dad7c 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -3593,10 +3593,12 @@
> Format: [state][,regs][,debounce][,die]
> 
> nmi_watchdog= [KNL,BUGS=X86] Debugging features for SMP kernels
> - Format: [panic,][nopanic,][num]
> + Format: [panic,][nopanic,][ref-cycles][num]
> Valid num: 0 or 1
> 0 - turn hardlockup detector in nmi_watchdog off
> 1 - turn hardlockup detector in nmi_watchdog on
> + ref-cycles - configure the watchdog with perf event
> +             "ref-cycles" instead of "cycles"
> When panic is specified, panic when an NMI watchdog
> timeout occurs (or 'nopanic' to not panic on an NMI
> watchdog, if CONFIG_BOOTPARAM_HARDLOCKUP_PANIC is set)
> @@ -7097,4 +7099,3 @@
> memory, and other data can't be written using
> xmon commands.
> off xmon is disabled.
> -
> diff --git a/include/linux/nmi.h b/include/linux/nmi.h
> index 048c0b9aa623..edfd1bcce0f6 100644
> --- a/include/linux/nmi.h
> +++ b/include/linux/nmi.h
> @@ -102,12 +102,14 @@ extern void hardlockup_detector_perf_disable(void);
> extern void hardlockup_detector_perf_enable(void);
> extern void hardlockup_detector_perf_cleanup(void);
> extern int hardlockup_detector_perf_init(void);
> +extern void hardlockup_config_perf_event(const char *str);
> #else
> static inline void hardlockup_detector_perf_stop(void) { }
> static inline void hardlockup_detector_perf_restart(void) { }
> static inline void hardlockup_detector_perf_disable(void) { }
> static inline void hardlockup_detector_perf_enable(void) { }
> static inline void hardlockup_detector_perf_cleanup(void) { }
> +static inline void hardlockup_config_perf_event(const char *str) { }
> # if !defined(CONFIG_HAVE_NMI_WATCHDOG)
> static inline int hardlockup_detector_perf_init(void) { return -ENODEV; }
> static inline void arch_touch_nmi_watchdog(void) {}
> diff --git a/kernel/watchdog.c b/kernel/watchdog.c
> index 8e61f21e7e33..fed4f0be8e1a 100644
> --- a/kernel/watchdog.c
> +++ b/kernel/watchdog.c
> @@ -81,6 +81,8 @@ static int __init hardlockup_panic_setup(char *str)
> nmi_watchdog_user_enabled = 0;
> else if (!strncmp(str, "1", 1))
> nmi_watchdog_user_enabled = 1;
> + else if (!strncmp(str, "ref-cycles", 10))
> + hardlockup_config_perf_event(str);
> return 1;
> }
> __setup("nmi_watchdog=", hardlockup_panic_setup);
> diff --git a/kernel/watchdog_hld.c b/kernel/watchdog_hld.c
> index 247bf0b1582c..4deca58ba6ed 100644
> --- a/kernel/watchdog_hld.c
> +++ b/kernel/watchdog_hld.c
> @@ -294,3 +294,12 @@ int __init hardlockup_detector_perf_init(void)
> }
> return ret;
> }
> +
> +/**
> + * hardlockup_config_perf_event - Overwrite config of wd_hw_attr
> + */
> +void __init hardlockup_config_perf_event(const char *str)
> +{
> + if (!strncmp(str, "ref-cycles", 10))
> + wd_hw_attr.config = PERF_COUNT_HW_REF_CPU_CYCLES;
> +}
> -- 
> 2.34.1
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v4] watchdog: Allow nmi watchdog to use "ref-cycles" event
  2023-05-19 16:59 ` Song Liu
@ 2023-05-25 22:20   ` Song Liu
  2023-06-02 22:11     ` Song Liu
  0 siblings, 1 reply; 9+ messages in thread
From: Song Liu @ 2023-05-25 22:20 UTC (permalink / raw)
  To: Song Liu; +Cc: Song Liu, LKML, Kernel Team, Andrew Morton, Peter Zijlstra

Hi Andrew and Peter,


> On May 19, 2023, at 9:59 AM, Song Liu <songliubraving@meta.com> wrote:
> 
> Hi Andrew and Peter, 
> 
> Does this version look good do you?
> 
> Thanks,
> Song
> 
>> On May 17, 2023, at 5:25 PM, Song Liu <song@kernel.org> wrote:
>> 
>> NMI watchdog permanently consumes one hardware counters per CPU on the
>> system. For systems that use many hardware counters, this causes more
>> aggressive time multiplexing of perf events.
>> 
>> OTOH, some CPUs (mostly Intel) support "ref-cycles" event, which is rarely
>> used. Add kernel cmdline arg nmi_watchdog=ref-cycles to configure the
>> watchdog to use "ref-cycles" event instead of "cycles".
>> 
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Cc: Peter Zijlstra <peterz@infradead.org>
>> Signed-off-by: Song Liu <song@kernel.org>

Could you please share your comments on this patch?

Thanks,
Song



>> 
>> ---
>> Changes in v4:
>> Fix compile error for !CONFIG_HARDLOCKUP_DETECTOR_PERF. (kernel test bot)
>> 
>> Changes in v3:
>> 
>> Pivot the design to use kernel arg nmi_watchdog=ref-cycles (Peter)
>> ---
>> Documentation/admin-guide/kernel-parameters.txt | 5 +++--
>> include/linux/nmi.h                             | 2 ++
>> kernel/watchdog.c                               | 2 ++
>> kernel/watchdog_hld.c                           | 9 +++++++++
>> 4 files changed, 16 insertions(+), 2 deletions(-)
>> 
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>> index 9e5bab29685f..d378e23dad7c 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -3593,10 +3593,12 @@
>> Format: [state][,regs][,debounce][,die]
>> 
>> nmi_watchdog= [KNL,BUGS=X86] Debugging features for SMP kernels
>> - Format: [panic,][nopanic,][num]
>> + Format: [panic,][nopanic,][ref-cycles][num]
>> Valid num: 0 or 1
>> 0 - turn hardlockup detector in nmi_watchdog off
>> 1 - turn hardlockup detector in nmi_watchdog on
>> + ref-cycles - configure the watchdog with perf event
>> +             "ref-cycles" instead of "cycles"
>> When panic is specified, panic when an NMI watchdog
>> timeout occurs (or 'nopanic' to not panic on an NMI
>> watchdog, if CONFIG_BOOTPARAM_HARDLOCKUP_PANIC is set)
>> @@ -7097,4 +7099,3 @@
>> memory, and other data can't be written using
>> xmon commands.
>> off xmon is disabled.
>> -
>> diff --git a/include/linux/nmi.h b/include/linux/nmi.h
>> index 048c0b9aa623..edfd1bcce0f6 100644
>> --- a/include/linux/nmi.h
>> +++ b/include/linux/nmi.h
>> @@ -102,12 +102,14 @@ extern void hardlockup_detector_perf_disable(void);
>> extern void hardlockup_detector_perf_enable(void);
>> extern void hardlockup_detector_perf_cleanup(void);
>> extern int hardlockup_detector_perf_init(void);
>> +extern void hardlockup_config_perf_event(const char *str);
>> #else
>> static inline void hardlockup_detector_perf_stop(void) { }
>> static inline void hardlockup_detector_perf_restart(void) { }
>> static inline void hardlockup_detector_perf_disable(void) { }
>> static inline void hardlockup_detector_perf_enable(void) { }
>> static inline void hardlockup_detector_perf_cleanup(void) { }
>> +static inline void hardlockup_config_perf_event(const char *str) { }
>> # if !defined(CONFIG_HAVE_NMI_WATCHDOG)
>> static inline int hardlockup_detector_perf_init(void) { return -ENODEV; }
>> static inline void arch_touch_nmi_watchdog(void) {}
>> diff --git a/kernel/watchdog.c b/kernel/watchdog.c
>> index 8e61f21e7e33..fed4f0be8e1a 100644
>> --- a/kernel/watchdog.c
>> +++ b/kernel/watchdog.c
>> @@ -81,6 +81,8 @@ static int __init hardlockup_panic_setup(char *str)
>> nmi_watchdog_user_enabled = 0;
>> else if (!strncmp(str, "1", 1))
>> nmi_watchdog_user_enabled = 1;
>> + else if (!strncmp(str, "ref-cycles", 10))
>> + hardlockup_config_perf_event(str);
>> return 1;
>> }
>> __setup("nmi_watchdog=", hardlockup_panic_setup);
>> diff --git a/kernel/watchdog_hld.c b/kernel/watchdog_hld.c
>> index 247bf0b1582c..4deca58ba6ed 100644
>> --- a/kernel/watchdog_hld.c
>> +++ b/kernel/watchdog_hld.c
>> @@ -294,3 +294,12 @@ int __init hardlockup_detector_perf_init(void)
>> }
>> return ret;
>> }
>> +
>> +/**
>> + * hardlockup_config_perf_event - Overwrite config of wd_hw_attr
>> + */
>> +void __init hardlockup_config_perf_event(const char *str)
>> +{
>> + if (!strncmp(str, "ref-cycles", 10))
>> + wd_hw_attr.config = PERF_COUNT_HW_REF_CPU_CYCLES;
>> +}
>> -- 
>> 2.34.1
>> 
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v4] watchdog: Allow nmi watchdog to use "ref-cycles" event
  2023-05-25 22:20   ` Song Liu
@ 2023-06-02 22:11     ` Song Liu
  0 siblings, 0 replies; 9+ messages in thread
From: Song Liu @ 2023-06-02 22:11 UTC (permalink / raw)
  To: Song Liu; +Cc: Song Liu, LKML, Kernel Team, Andrew Morton, Peter Zijlstra

Hi Andrew and Peter, 

Friendly ping... Any comment on this one? 

Thanks,
Song

> On May 25, 2023, at 3:20 PM, Song Liu <songliubraving@meta.com> wrote:
> 
> Hi Andrew and Peter,
> 
> 
>> On May 19, 2023, at 9:59 AM, Song Liu <songliubraving@meta.com> wrote:
>> 
>> Hi Andrew and Peter, 
>> 
>> Does this version look good do you?
>> 
>> Thanks,
>> Song
>> 
>>> On May 17, 2023, at 5:25 PM, Song Liu <song@kernel.org> wrote:
>>> 
>>> NMI watchdog permanently consumes one hardware counters per CPU on the
>>> system. For systems that use many hardware counters, this causes more
>>> aggressive time multiplexing of perf events.
>>> 
>>> OTOH, some CPUs (mostly Intel) support "ref-cycles" event, which is rarely
>>> used. Add kernel cmdline arg nmi_watchdog=ref-cycles to configure the
>>> watchdog to use "ref-cycles" event instead of "cycles".
>>> 
>>> Cc: Andrew Morton <akpm@linux-foundation.org>
>>> Cc: Peter Zijlstra <peterz@infradead.org>
>>> Signed-off-by: Song Liu <song@kernel.org>
> 
> Could you please share your comments on this patch?
> 
> Thanks,
> Song



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v4] watchdog: Allow nmi watchdog to use "ref-cycles" event
  2023-05-18  0:25 [PATCH v4] watchdog: Allow nmi watchdog to use "ref-cycles" event Song Liu
  2023-05-18  6:44 ` Yonghong Song
  2023-05-19 16:59 ` Song Liu
@ 2023-06-02 22:47 ` Peter Zijlstra
  2023-06-02 23:15   ` Song Liu
  2 siblings, 1 reply; 9+ messages in thread
From: Peter Zijlstra @ 2023-06-02 22:47 UTC (permalink / raw)
  To: Song Liu; +Cc: linux-kernel, kernel-team, Andrew Morton

On Wed, May 17, 2023 at 05:25:55PM -0700, Song Liu wrote:
> NMI watchdog permanently consumes one hardware counters per CPU on the
> system. For systems that use many hardware counters, this causes more
> aggressive time multiplexing of perf events.
> 
> OTOH, some CPUs (mostly Intel) support "ref-cycles" event, which is rarely
> used. Add kernel cmdline arg nmi_watchdog=ref-cycles to configure the
> watchdog to use "ref-cycles" event instead of "cycles".
> 
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Song Liu <song@kernel.org>
> 
> ---
> Changes in v4:
> Fix compile error for !CONFIG_HARDLOCKUP_DETECTOR_PERF. (kernel test bot)
> 
> Changes in v3:
> 
> Pivot the design to use kernel arg nmi_watchdog=ref-cycles (Peter)
> ---
>  Documentation/admin-guide/kernel-parameters.txt | 5 +++--
>  include/linux/nmi.h                             | 2 ++
>  kernel/watchdog.c                               | 2 ++
>  kernel/watchdog_hld.c                           | 9 +++++++++
>  4 files changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 9e5bab29685f..d378e23dad7c 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -3593,10 +3593,12 @@
>  			Format: [state][,regs][,debounce][,die]
>  
>  	nmi_watchdog=	[KNL,BUGS=X86] Debugging features for SMP kernels
> -			Format: [panic,][nopanic,][num]
> +			Format: [panic,][nopanic,][ref-cycles][num]
>  			Valid num: 0 or 1
>  			0 - turn hardlockup detector in nmi_watchdog off
>  			1 - turn hardlockup detector in nmi_watchdog on
> +			ref-cycles - configure the watchdog with perf event
> +			             "ref-cycles" instead of "cycles"
>  			When panic is specified, panic when an NMI watchdog
>  			timeout occurs (or 'nopanic' to not panic on an NMI
>  			watchdog, if CONFIG_BOOTPARAM_HARDLOCKUP_PANIC is set)

I still hate the whole ref-cycles thing, at the very least powerpc also
has HAVE_HARDLOCKUP_DETECTOR_PERF and they don't have ref-cycles, but
perhaps them wants to use a different event when the moon is just so...

What again was wrong with the option of specifying a raw event value and
falling back to cpu-cycles if that fails?

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v4] watchdog: Allow nmi watchdog to use "ref-cycles" event
  2023-06-02 22:47 ` Peter Zijlstra
@ 2023-06-02 23:15   ` Song Liu
  2023-06-08 18:18     ` Song Liu
  0 siblings, 1 reply; 9+ messages in thread
From: Song Liu @ 2023-06-02 23:15 UTC (permalink / raw)
  To: Peter Zijlstra; +Cc: Song Liu, LKML, Kernel Team, Andrew Morton



> On Jun 2, 2023, at 3:47 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> 
> On Wed, May 17, 2023 at 05:25:55PM -0700, Song Liu wrote:
>> NMI watchdog permanently consumes one hardware counters per CPU on the
>> system. For systems that use many hardware counters, this causes more
>> aggressive time multiplexing of perf events.
>> 
>> OTOH, some CPUs (mostly Intel) support "ref-cycles" event, which is rarely
>> used. Add kernel cmdline arg nmi_watchdog=ref-cycles to configure the
>> watchdog to use "ref-cycles" event instead of "cycles".
>> 
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Cc: Peter Zijlstra <peterz@infradead.org>
>> Signed-off-by: Song Liu <song@kernel.org>
>> 
>> ---
>> Changes in v4:
>> Fix compile error for !CONFIG_HARDLOCKUP_DETECTOR_PERF. (kernel test bot)
>> 
>> Changes in v3:
>> 
>> Pivot the design to use kernel arg nmi_watchdog=ref-cycles (Peter)
>> ---
>> Documentation/admin-guide/kernel-parameters.txt | 5 +++--
>> include/linux/nmi.h                             | 2 ++
>> kernel/watchdog.c                               | 2 ++
>> kernel/watchdog_hld.c                           | 9 +++++++++
>> 4 files changed, 16 insertions(+), 2 deletions(-)
>> 
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>> index 9e5bab29685f..d378e23dad7c 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -3593,10 +3593,12 @@
>> Format: [state][,regs][,debounce][,die]
>> 
>> nmi_watchdog= [KNL,BUGS=X86] Debugging features for SMP kernels
>> - Format: [panic,][nopanic,][num]
>> + Format: [panic,][nopanic,][ref-cycles][num]
>> Valid num: 0 or 1
>> 0 - turn hardlockup detector in nmi_watchdog off
>> 1 - turn hardlockup detector in nmi_watchdog on
>> + ref-cycles - configure the watchdog with perf event
>> +             "ref-cycles" instead of "cycles"
>> When panic is specified, panic when an NMI watchdog
>> timeout occurs (or 'nopanic' to not panic on an NMI
>> watchdog, if CONFIG_BOOTPARAM_HARDLOCKUP_PANIC is set)
> 
> I still hate the whole ref-cycles thing, at the very least powerpc also
> has HAVE_HARDLOCKUP_DETECTOR_PERF and they don't have ref-cycles, but
> perhaps them wants to use a different event when the moon is just so...
> 
> What again was wrong with the option of specifying a raw event value and
> falling back to cpu-cycles if that fails?

The same raw event number may mean different events on different hardware. 
So it is more likely to make mistakes in configurations. For example, r300 
means ref-cycles on Intel CPUs, but it also means something else on AMD 
CPUs. I need to be very careful which hosts to run with nmi_watchdog=r300, 
as it may cause surprises. OTOH, nmi_watchdog=ref-cycles won't have this 
issue. Of course, this won't work for powerpc. 

Does this make sense?

Thanks,
Song


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v4] watchdog: Allow nmi watchdog to use "ref-cycles" event
  2023-06-02 23:15   ` Song Liu
@ 2023-06-08 18:18     ` Song Liu
  0 siblings, 0 replies; 9+ messages in thread
From: Song Liu @ 2023-06-08 18:18 UTC (permalink / raw)
  To: Peter Zijlstra; +Cc: Song Liu, LKML, Kernel Team, Andrew Morton

Hi Peter, 

> On Jun 2, 2023, at 4:15 PM, Song Liu <songliubraving@meta.com> wrote:

[...]

>>> nmi_watchdog= [KNL,BUGS=X86] Debugging features for SMP kernels
>>> - Format: [panic,][nopanic,][num]
>>> + Format: [panic,][nopanic,][ref-cycles][num]
>>> Valid num: 0 or 1
>>> 0 - turn hardlockup detector in nmi_watchdog off
>>> 1 - turn hardlockup detector in nmi_watchdog on
>>> + ref-cycles - configure the watchdog with perf event
>>> +             "ref-cycles" instead of "cycles"
>>> When panic is specified, panic when an NMI watchdog
>>> timeout occurs (or 'nopanic' to not panic on an NMI
>>> watchdog, if CONFIG_BOOTPARAM_HARDLOCKUP_PANIC is set)
>> 
>> I still hate the whole ref-cycles thing, at the very least powerpc also
>> has HAVE_HARDLOCKUP_DETECTOR_PERF and they don't have ref-cycles, but
>> perhaps them wants to use a different event when the moon is just so...
>> 
>> What again was wrong with the option of specifying a raw event value and
>> falling back to cpu-cycles if that fails?
> 
> The same raw event number may mean different events on different hardware. 
> So it is more likely to make mistakes in configurations. For example, r300 
> means ref-cycles on Intel CPUs, but it also means something else on AMD 
> CPUs. I need to be very careful which hosts to run with nmi_watchdog=r300, 
> as it may cause surprises. OTOH, nmi_watchdog=ref-cycles won't have this 
> issue. Of course, this won't work for powerpc. 

Does this make sense? Do we have other ideas to configure this?

Thanks,
Song


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2023-06-08 18:18 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-18  0:25 [PATCH v4] watchdog: Allow nmi watchdog to use "ref-cycles" event Song Liu
2023-05-18  6:44 ` Yonghong Song
2023-05-18 20:12   ` Song Liu
2023-05-19 16:59 ` Song Liu
2023-05-25 22:20   ` Song Liu
2023-06-02 22:11     ` Song Liu
2023-06-02 22:47 ` Peter Zijlstra
2023-06-02 23:15   ` Song Liu
2023-06-08 18:18     ` Song Liu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®