linux-kernel - RE: [PATCH v2] watchdog: Prefer use "ref-cycles" for NMI watchdog

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <SA1PR11MB6734A17FF1E12FC3AC256C91A87E9@SA1PR11MB6734.namprd11.prod.outlook.com>
Date:   Wed, 17 May 2023 01:23:27 +0000
From:   "Li, Xin3" <xin3.li@...el.com>
To:     Song Liu <song@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC:     "kernel-team@...a.com" <kernel-team@...a.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Peter Zijlstra <peterz@...radead.org>
Subject: RE: [PATCH v2] watchdog: Prefer use "ref-cycles" for NMI watchdog

> 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. Try use "ref-cycles" for the watchdog, so that one more hardware
> counter is available to the user. If the CPU doesn't support "ref-cycles",
> fall back to "cycles".
> 
> The downside of this change is that users of "ref-cycles" need to disable
> nmi_watchdog.

>From the discussion in v1, the users don't have to disable the NMI watchdog
*permanently*, right?

> 
> Cc: Andrew Morton <akpm@...ux-foundation.org>
> Cc: Peter Zijlstra <peterz@...radead.org>
> Signed-off-by: Song Liu <song@...nel.org>
> 
> ---
> 
> Changes in v2:
> 1. Do not send warning when failed to create ref-cycles event.
> ---
>  kernel/watchdog_hld.c | 20 ++++++++++++++------
>  1 file changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/kernel/watchdog_hld.c b/kernel/watchdog_hld.c
> index 247bf0b1582c..a1d2a43ea31f 100644
> --- a/kernel/watchdog_hld.c
> +++ b/kernel/watchdog_hld.c
> @@ -100,7 +100,7 @@ static inline bool watchdog_check_timestamp(void)
> 
>  static struct perf_event_attr wd_hw_attr = {
>  	.type		= PERF_TYPE_HARDWARE,
> -	.config		= PERF_COUNT_HW_CPU_CYCLES,
> +	.config		= PERF_COUNT_HW_REF_CPU_CYCLES,
>  	.size		= sizeof(struct perf_event_attr),
>  	.pinned		= 1,
>  	.disabled	= 1,
> @@ -163,7 +163,7 @@ static void watchdog_overflow_callback(struct perf_event
> *event,
>  	return;
>  }
> 
> -static int hardlockup_detector_event_create(void)
> +static int hardlockup_detector_event_create(bool send_warning)
>  {
>  	unsigned int cpu = smp_processor_id();
>  	struct perf_event_attr *wd_attr;
> @@ -176,8 +176,10 @@ static int hardlockup_detector_event_create(void)
>  	evt = perf_event_create_kernel_counter(wd_attr, cpu, NULL,
>  					       watchdog_overflow_callback, NULL);
>  	if (IS_ERR(evt)) {
> -		pr_debug("Perf event create on CPU %d failed with %ld\n", cpu,
> -			 PTR_ERR(evt));
> +		if (send_warning) {
> +			pr_debug("Perf event create on CPU %d failed with
> %ld\n", cpu,
> +				 PTR_ERR(evt));
> +		}
>  		return PTR_ERR(evt);
>  	}
>  	this_cpu_write(watchdog_ev, evt);
> @@ -189,7 +191,7 @@ static int hardlockup_detector_event_create(void)
>   */
>  void hardlockup_detector_perf_enable(void)
>  {
> -	if (hardlockup_detector_event_create())
> +	if (hardlockup_detector_event_create(true))
>  		return;
> 
>  	/* use original value for check */
> @@ -284,7 +286,13 @@ void __init hardlockup_detector_perf_restart(void)
>   */
>  int __init hardlockup_detector_perf_init(void)
>  {
> -	int ret = hardlockup_detector_event_create();
> +	int ret = hardlockup_detector_event_create(false);
> +
> +	if (ret) {
> +		/* Failed to create "ref-cycles", try "cycles" instead */
> +		wd_hw_attr.config = PERF_COUNT_HW_CPU_CYCLES;
> +		ret = hardlockup_detector_event_create(true);
> +	}
> 
>  	if (ret) {
>  		pr_info("Perf NMI watchdog permanently disabled\n");
> --
> 2.34.1