Re: [RFC PATCH 22/23] watchdog/hardlockup/hpet: Only enable the HPET watchdog via a boot parameter
On 06/13/2018 05:58 PM, Ricardo Neri wrote: > On Tue, Jun 12, 2018 at 10:26:57PM -0700, Randy Dunlap wrote: >> On 06/12/2018 05:57 PM, Ricardo Neri wrote: >>> diff --git a/Documentation/admin-guide/kernel-parameters.txt >>> b/Documentation/admin-guide/kernel-parameters.txt >>> index f2040d4..a8833c7 100644 >>> --- a/Documentation/admin-guide/kernel-parameters.txt >>> +++ b/Documentation/admin-guide/kernel-parameters.txt >>> @@ -2577,7 +2577,7 @@ >>> Format: [state][,regs][,debounce][,die] >>> >>> nmi_watchdog= [KNL,BUGS=X86] Debugging features for SMP kernels >>> - Format: [panic,][nopanic,][num] >>> + Format: [panic,][nopanic,][num,][hpet] >>> Valid num: 0 or 1 >>> 0 - turn hardlockup detector in nmi_watchdog off >>> 1 - turn hardlockup detector in nmi_watchdog on >> >> This says that I can use "nmi_watchdog=hpet" without using 0 or 1. >> Is that correct? > > Yes, this what I meant. In my view, if you set nmi_watchdog=hpet it > implies that you want to activate the NMI watchdog. In this case, perf. > > I can see how this will be ambiguous for the case of perf and arch NMI > watchdogs. > > Alternative, a new parameter could be added; such as nmi_watchdog_type. I > didn't want to add it in this patchset as I think that a single parameter > can handle the enablement and type of the NMI watchdog. > > What do you think? I think it's OK like it is. thanks, -- ~Randy
Re: [RFC PATCH 22/23] watchdog/hardlockup/hpet: Only enable the HPET watchdog via a boot parameter
On Tue, Jun 12, 2018 at 10:26:57PM -0700, Randy Dunlap wrote: > On 06/12/2018 05:57 PM, Ricardo Neri wrote: > > diff --git a/Documentation/admin-guide/kernel-parameters.txt > > b/Documentation/admin-guide/kernel-parameters.txt > > index f2040d4..a8833c7 100644 > > --- a/Documentation/admin-guide/kernel-parameters.txt > > +++ b/Documentation/admin-guide/kernel-parameters.txt > > @@ -2577,7 +2577,7 @@ > > Format: [state][,regs][,debounce][,die] > > > > nmi_watchdog= [KNL,BUGS=X86] Debugging features for SMP kernels > > - Format: [panic,][nopanic,][num] > > + Format: [panic,][nopanic,][num,][hpet] > > Valid num: 0 or 1 > > 0 - turn hardlockup detector in nmi_watchdog off > > 1 - turn hardlockup detector in nmi_watchdog on > > This says that I can use "nmi_watchdog=hpet" without using 0 or 1. > Is that correct? Yes, this what I meant. In my view, if you set nmi_watchdog=hpet it implies that you want to activate the NMI watchdog. In this case, perf. I can see how this will be ambiguous for the case of perf and arch NMI watchdogs. Alternative, a new parameter could be added; such as nmi_watchdog_type. I didn't want to add it in this patchset as I think that a single parameter can handle the enablement and type of the NMI watchdog. What do you think? Thanks and BR, Ricardo
Re: [RFC PATCH 22/23] watchdog/hardlockup/hpet: Only enable the HPET watchdog via a boot parameter
On 06/12/2018 05:57 PM, Ricardo Neri wrote: > diff --git a/Documentation/admin-guide/kernel-parameters.txt > b/Documentation/admin-guide/kernel-parameters.txt > index f2040d4..a8833c7 100644 > --- a/Documentation/admin-guide/kernel-parameters.txt > +++ b/Documentation/admin-guide/kernel-parameters.txt > @@ -2577,7 +2577,7 @@ > Format: [state][,regs][,debounce][,die] > > nmi_watchdog= [KNL,BUGS=X86] Debugging features for SMP kernels > - Format: [panic,][nopanic,][num] > + Format: [panic,][nopanic,][num,][hpet] > Valid num: 0 or 1 > 0 - turn hardlockup detector in nmi_watchdog off > 1 - turn hardlockup detector in nmi_watchdog on This says that I can use "nmi_watchdog=hpet" without using 0 or 1. Is that correct? > @@ -2587,6 +2587,9 @@ > please see 'nowatchdog'. > This is useful when you use a panic=... timeout and > need the box quickly up again. > + When hpet is specified, the NMI watchdog will be driven > + by an HPET timer, if available in the system. Otherwise, > + the perf-based implementation will be used. > > These settings can be accessed at runtime via > the nmi_watchdog and hardlockup_panic sysctls. thanks, -- ~Randy
[RFC PATCH 22/23] watchdog/hardlockup/hpet: Only enable the HPET watchdog via a boot parameter
Keep the HPET-based hardlockup detector disabled unless explicitly enabled
via a command line argument. If such parameter is not given, the hardlockup
detector will fallback to use the perf-based implementation.
The function hardlockup_panic_setup() is updated to return 0 in order to
to allow __setup functions of specific hardlockup detectors (in this case
hardlockup_detector_hpet_setup()) to inspect the nmi_watchdog boot
parameter.
Cc: Ashok Raj
Cc: Andi Kleen
Cc: Tony Luck
Cc: Borislav Petkov
Cc: Jacob Pan
Cc: "Rafael J. Wysocki"
Cc: Don Zickus
Cc: Nicholas Piggin
Cc: Michael Ellerman
Cc: Frederic Weisbecker
Cc: Alexei Starovoitov
Cc: Babu Moger
Cc: Mathieu Desnoyers
Cc: Masami Hiramatsu
Cc: Peter Zijlstra
Cc: Andrew Morton
Cc: Philippe Ombredanne
Cc: Colin Ian King
Cc: Byungchul Park
Cc: "Paul E. McKenney"
Cc: "Luis R. Rodriguez"
Cc: Waiman Long
Cc: Josh Poimboeuf
Cc: Randy Dunlap
Cc: Davidlohr Bueso
Cc: Christoffer Dall
Cc: Marc Zyngier
Cc: Kai-Heng Feng
Cc: Konrad Rzeszutek Wilk
Cc: David Rientjes
Cc: "Ravi V. Shankar"
Cc: x...@kernel.org
Cc: io...@lists.linux-foundation.org
Signed-off-by: Ricardo Neri
--
checkpatch gives the following warning:
CHECK: __setup appears un-documented -- check
Documentation/admin-guide/kernel-parameters.rst
+__setup("nmi_watchdog=", hardlockup_detector_hpet_setup);
This is a false-positive as the option nmi_watchdog is already
documented. The option is re-evaluated in this file as well.
---
Documentation/admin-guide/kernel-parameters.txt | 5 -
kernel/watchdog.c | 2 +-
kernel/watchdog_hld_hpet.c | 13 +
3 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt
b/Documentation/admin-guide/kernel-parameters.txt
index f2040d4..a8833c7 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -2577,7 +2577,7 @@
Format: [state][,regs][,debounce][,die]
nmi_watchdog= [KNL,BUGS=X86] Debugging features for SMP kernels
- Format: [panic,][nopanic,][num]
+ Format: [panic,][nopanic,][num,][hpet]
Valid num: 0 or 1
0 - turn hardlockup detector in nmi_watchdog off
1 - turn hardlockup detector in nmi_watchdog on
@@ -2587,6 +2587,9 @@
please see 'nowatchdog'.
This is useful when you use a panic=... timeout and
need the box quickly up again.
+ When hpet is specified, the NMI watchdog will be driven
+ by an HPET timer, if available in the system. Otherwise,
+ the perf-based implementation will be used.
These settings can be accessed at runtime via
the nmi_watchdog and hardlockup_panic sysctls.
diff --git a/kernel/watchdog.c b/kernel/watchdog.c
index b94bbe3..b5ce6e4 100644
--- a/kernel/watchdog.c
+++ b/kernel/watchdog.c
@@ -84,7 +84,7 @@ static int __init hardlockup_panic_setup(char *str)
nmi_watchdog_user_enabled = 0;
else if (!strncmp(str, "1", 1))
nmi_watchdog_user_enabled = 1;
- return 1;
+ return 0;
}
__setup("nmi_watchdog=", hardlockup_panic_setup);
diff --git a/kernel/watchdog_hld_hpet.c b/kernel/watchdog_hld_hpet.c
index ebb820d..12e5937 100644
--- a/kernel/watchdog_hld_hpet.c
+++ b/kernel/watchdog_hld_hpet.c
@@ -17,6 +17,7 @@
#define pr_fmt(fmt) "NMI hpet watchdog: " fmt
static struct hpet_hld_data *hld_data;
+static bool hardlockup_use_hpet;
/**
* get_count() - Get the current count of the HPET timer
@@ -488,6 +489,15 @@ static void hardlockup_detector_hpet_stop(void)
spin_unlock(_data->lock);
}
+static int __init hardlockup_detector_hpet_setup(char *str)
+{
+ if (strstr(str, "hpet"))
+ hardlockup_use_hpet = true;
+
+ return 0;
+}
+__setup("nmi_watchdog=", hardlockup_detector_hpet_setup);
+
/**
* hardlockup_detector_hpet_init() - Initialize the hardlockup detector
*
@@ -502,6 +512,9 @@ static int __init hardlockup_detector_hpet_init(void)
{
int ret;
+ if (!hardlockup_use_hpet)
+ return -EINVAL;
+
if (!is_hpet_enabled())
return -ENODEV;
--
2.7.4
