Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Sat, 3 Jun 2023 00:47:44 +0200
From:   Peter Zijlstra <peterz@infradead.org>
To:     Song Liu <song@kernel.org>
Cc:     linux-kernel@vger.kernel.org, kernel-team@meta.com,
        Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH v4] watchdog: Allow nmi watchdog to use "ref-cycles" event
Message-ID: <20230602224744.GC695361@hirez.programming.kicks-ass.net>
References: <20230518002555.1114189-1-song@kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230518002555.1114189-1-song@kernel.org>
Precedence: bulk

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?