Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1496884 > unrolled thread

[PATCH 1/2] watchdog: Introduce update_arch_nmi_watchdog

Started byBabu Moger <babu.moger@oracle.com>
First post2016-10-07 00:20 +0200
Last post2016-10-07 17:00 +0200
Articles 3 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  [PATCH 1/2] watchdog: Introduce update_arch_nmi_watchdog Babu Moger <babu.moger@oracle.com> - 2016-10-07 00:20 +0200
    Re: [PATCH 1/2] watchdog: Introduce update_arch_nmi_watchdog Sam Ravnborg <sam@ravnborg.org> - 2016-10-07 06:40 +0200
      Re: [PATCH 1/2] watchdog: Introduce update_arch_nmi_watchdog Babu Moger <babu.moger@oracle.com> - 2016-10-07 17:00 +0200

#1496884 — [PATCH 1/2] watchdog: Introduce update_arch_nmi_watchdog

FromBabu Moger <babu.moger@oracle.com>
Date2016-10-07 00:20 +0200
Subject[PATCH 1/2] watchdog: Introduce update_arch_nmi_watchdog
Message-ID<sppbc-7ce-13@gated-at.bofh.it>
Currently we do not have a way to enable/disable arch specific
watchdog handlers if it was implemented by any of the architectures.

This patch introduces new function update_arch_nmi_watchdog
which can be used to enable/disable architecture specific NMI
watchdog handlers. Also exposes watchdog_enabled variable outside
so that arch specific nmi watchdogs can use it to implement
enalbe/disable behavour.

Signed-off-by: Babu Moger <babu.moger@oracle.com>
---
 include/linux/nmi.h |    1 +
 kernel/watchdog.c   |   16 +++++++++++++---
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/include/linux/nmi.h b/include/linux/nmi.h
index 4630eea..01b4830 100644
--- a/include/linux/nmi.h
+++ b/include/linux/nmi.h
@@ -66,6 +66,7 @@ static inline bool trigger_allbutself_cpu_backtrace(void)
 
 #ifdef CONFIG_LOCKUP_DETECTOR
 u64 hw_nmi_get_sample_period(int watchdog_thresh);
+extern unsigned long watchdog_enabled;
 extern int nmi_watchdog_enabled;
 extern int soft_watchdog_enabled;
 extern int watchdog_user_enabled;
diff --git a/kernel/watchdog.c b/kernel/watchdog.c
index 9acb29f..1ac2814 100644
--- a/kernel/watchdog.c
+++ b/kernel/watchdog.c
@@ -46,16 +46,21 @@
 
 static DEFINE_MUTEX(watchdog_proc_mutex);
 
-#ifdef CONFIG_HARDLOCKUP_DETECTOR
-static unsigned long __read_mostly watchdog_enabled = SOFT_WATCHDOG_ENABLED|NMI_WATCHDOG_ENABLED;
+#if defined(CONFIG_HARDLOCKUP_DETECTOR) || defined(CONFIG_HAVE_NMI_WATCHDOG)
+unsigned long __read_mostly watchdog_enabled = SOFT_WATCHDOG_ENABLED|NMI_WATCHDOG_ENABLED;
 #else
-static unsigned long __read_mostly watchdog_enabled = SOFT_WATCHDOG_ENABLED;
+unsigned long __read_mostly watchdog_enabled = SOFT_WATCHDOG_ENABLED;
 #endif
 int __read_mostly nmi_watchdog_enabled;
 int __read_mostly soft_watchdog_enabled;
 int __read_mostly watchdog_user_enabled;
 int __read_mostly watchdog_thresh = 10;
 
+/*
+ * Implemented by arch specific handlers if it defines CONFIG_HAVE_NMI_WATCHDOG
+ */
+void __weak update_arch_nmi_watchdog(void) {}
+
 #ifdef CONFIG_SMP
 int __read_mostly sysctl_softlockup_all_cpu_backtrace;
 int __read_mostly sysctl_hardlockup_all_cpu_backtrace;
@@ -842,6 +847,11 @@ static int proc_watchdog_update(void)
 	int err = 0;
 
 	/*
+	 * Enable/Disable arch specific nmi watchdogs if there is one
+	 */
+	update_arch_nmi_watchdog();
+
+	/*
 	 * Watchdog threads won't be started if they are already active.
 	 * The 'watchdog_running' variable in watchdog_*_all_cpus() takes
 	 * care of this. If those threads are already active, the sample
-- 
1.7.1

[toc] | [next] | [standalone]


#1496994

FromSam Ravnborg <sam@ravnborg.org>
Date2016-10-07 06:40 +0200
Message-ID<spv6V-38J-3@gated-at.bofh.it>
In reply to#1496884
On Thu, Oct 06, 2016 at 03:16:42PM -0700, Babu Moger wrote:
> Currently we do not have a way to enable/disable arch specific
> watchdog handlers if it was implemented by any of the architectures.
> 
> This patch introduces new function update_arch_nmi_watchdog
> which can be used to enable/disable architecture specific NMI
> watchdog handlers. Also exposes watchdog_enabled variable outside
> so that arch specific nmi watchdogs can use it to implement
> enalbe/disable behavour.
> 
> Signed-off-by: Babu Moger <babu.moger@oracle.com>
> ---
>  include/linux/nmi.h |    1 +
>  kernel/watchdog.c   |   16 +++++++++++++---
>  2 files changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/nmi.h b/include/linux/nmi.h
> index 4630eea..01b4830 100644
> --- a/include/linux/nmi.h
> +++ b/include/linux/nmi.h
> @@ -66,6 +66,7 @@ static inline bool trigger_allbutself_cpu_backtrace(void)
>  
>  #ifdef CONFIG_LOCKUP_DETECTOR
>  u64 hw_nmi_get_sample_period(int watchdog_thresh);
> +extern unsigned long watchdog_enabled;

The extern is within an #ifdef, but the definition later is
valid alway.
So extern definition should be outside the #ifdef to match the
actual implementation.

To manipulate / read watchdog_enabled two constants are used: 
NMI_WATCHDOG_ENABLED, SOFT_WATCHDOG_ENABLED

They should be visible too, so uses do not fall into the trap
and uses constants (like in patch 2).

	Sam

[toc] | [prev] | [next] | [standalone]


#1497203

FromBabu Moger <babu.moger@oracle.com>
Date2016-10-07 17:00 +0200
Message-ID<spEMW-1UC-13@gated-at.bofh.it>
In reply to#1496994
On 10/6/2016 11:34 PM, Sam Ravnborg wrote:
> On Thu, Oct 06, 2016 at 03:16:42PM -0700, Babu Moger wrote:
>> Currently we do not have a way to enable/disable arch specific
>> watchdog handlers if it was implemented by any of the architectures.
>>
>> This patch introduces new function update_arch_nmi_watchdog
>> which can be used to enable/disable architecture specific NMI
>> watchdog handlers. Also exposes watchdog_enabled variable outside
>> so that arch specific nmi watchdogs can use it to implement
>> enalbe/disable behavour.
>>
>> Signed-off-by: Babu Moger <babu.moger@oracle.com>
>> ---
>>   include/linux/nmi.h |    1 +
>>   kernel/watchdog.c   |   16 +++++++++++++---
>>   2 files changed, 14 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/linux/nmi.h b/include/linux/nmi.h
>> index 4630eea..01b4830 100644
>> --- a/include/linux/nmi.h
>> +++ b/include/linux/nmi.h
>> @@ -66,6 +66,7 @@ static inline bool trigger_allbutself_cpu_backtrace(void)
>>   
>>   #ifdef CONFIG_LOCKUP_DETECTOR
>>   u64 hw_nmi_get_sample_period(int watchdog_thresh);
>> +extern unsigned long watchdog_enabled;
> The extern is within an #ifdef, but the definition later is
> valid alway.
> So extern definition should be outside the #ifdef to match the
> actual implementation.

      Ok. Sure.

>
> To manipulate / read watchdog_enabled two constants are used:
> NMI_WATCHDOG_ENABLED, SOFT_WATCHDOG_ENABLED

     Sure. I will bring these definitions to nmi.h from watchdog.c

> They should be visible too, so uses do not fall into the trap
> and uses constants (like in patch 2).

     Will re-post v2 version with these changes. Thanks for the comments.

>
> 	Sam

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web