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

Groups > linux.kernel > #1650324 > unrolled thread

[PATCH 0/4][V2] Improve watchdog config for arch watchdogs

Started byNicholas Piggin <npiggin@gmail.com>
First post2017-05-25 10:40 +0200
Last post2017-05-26 16:10 +0200
Articles 5 — 2 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/4][V2] Improve watchdog config for arch watchdogs Nicholas Piggin <npiggin@gmail.com> - 2017-05-25 10:40 +0200
    [PATCH 2/4] watchdog: introduce arch_touch_nmi_watchdog() Nicholas Piggin <npiggin@gmail.com> - 2017-05-25 10:40 +0200
      Re: [PATCH 2/4] watchdog: introduce arch_touch_nmi_watchdog() Don Zickus <dzickus@redhat.com> - 2017-05-25 16:00 +0200
        Re: [PATCH 2/4] watchdog: introduce arch_touch_nmi_watchdog() Nicholas Piggin <npiggin@gmail.com> - 2017-05-26 02:40 +0200
          Re: [PATCH 2/4] watchdog: introduce arch_touch_nmi_watchdog() Don Zickus <dzickus@redhat.com> - 2017-05-26 16:10 +0200

#1650324 — [PATCH 0/4][V2] Improve watchdog config for arch watchdogs

FromNicholas Piggin <npiggin@gmail.com>
Date2017-05-25 10:40 +0200
Subject[PATCH 0/4][V2] Improve watchdog config for arch watchdogs
Message-ID<tKWjE-5Mt-9@gated-at.bofh.it>
Hi Don,

The kbuild 0day caught a compile bug on some archs, I think I've
got that fixed.

I'm resending without the powerpc watchdog example. However if you
decide to accept these changes for the next merge window, I may send
the powerpc patch through your tree with appropriate acks, because
it's not very intrusive.

So the basic idea of these 4 patches is to allow arch specific
(non-perf) hardlockup watchdogs to properly hook into the existing
watchdog interfaces (cmdline, sysctl, kernel apis).

sparc for example implements its own detector, but it can't use all
the same sysctls, some cmdline options are a bit different, etc.
powerpc wants to move to its own detector like sparc, but without
causing interface functionality to change.

Thanks,
Nick

Nicholas Piggin (4):
  watchdog: remove unused declaration
  watchdog: introduce arch_touch_nmi_watchdog()
  watchdog: split up config options
  watchdog: provide watchdog_reconfigure() for arch watchdogs

 arch/blackfin/include/asm/nmi.h            |   2 +
 arch/blackfin/kernel/nmi.c                 |   2 +-
 arch/mn10300/include/asm/nmi.h             |   2 +
 arch/mn10300/kernel/mn10300-watchdog-low.S |   8 +-
 arch/mn10300/kernel/mn10300-watchdog.c     |   2 +-
 arch/powerpc/kernel/setup_64.c             |   2 +-
 arch/sparc/include/asm/nmi.h               |   1 +
 arch/sparc/kernel/nmi.c                    |   6 +-
 arch/x86/kernel/apic/hw_nmi.c              |   2 +-
 include/linux/nmi.h                        |  58 ++++---
 kernel/Makefile                            |   2 +-
 kernel/sysctl.c                            |  18 +-
 kernel/watchdog.c                          | 262 +++++++++++++++++++----------
 kernel/watchdog_hld.c                      |  37 +---
 lib/Kconfig.debug                          |  27 ++-
 15 files changed, 261 insertions(+), 170 deletions(-)

-- 
2.11.0

[toc] | [next] | [standalone]

#1650325 — [PATCH 2/4] watchdog: introduce arch_touch_nmi_watchdog()

FromNicholas Piggin <npiggin@gmail.com>
Date2017-05-25 10:40 +0200
Subject[PATCH 2/4] watchdog: introduce arch_touch_nmi_watchdog()
Message-ID<tKWtk-5Qx-33@gated-at.bofh.it>
In reply to#1650324
For architectures that define HAVE_NMI_WATCHDOG, instead of having
them provide the complete touch_nmi_watchdog() function, just have
them provide arch_touch_nmi_watchdog().

This gives the generic code more flexibility in implementing this
function, and arch implementations don't miss out on touching the
softlockup watchdog or other generic details.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/blackfin/include/asm/nmi.h            |  2 ++
 arch/blackfin/kernel/nmi.c                 |  2 +-
 arch/mn10300/include/asm/nmi.h             |  2 ++
 arch/mn10300/kernel/mn10300-watchdog-low.S |  8 ++++----
 arch/mn10300/kernel/mn10300-watchdog.c     |  2 +-
 arch/sparc/include/asm/nmi.h               |  1 +
 arch/sparc/kernel/nmi.c                    |  6 ++----
 include/linux/nmi.h                        | 26 +++++++++++++++-----------
 kernel/watchdog_hld.c                      |  5 ++---
 9 files changed, 30 insertions(+), 24 deletions(-)

diff --git a/arch/blackfin/include/asm/nmi.h b/arch/blackfin/include/asm/nmi.h
index b9caac4fcfd8..6e7047ceec79 100644
--- a/arch/blackfin/include/asm/nmi.h
+++ b/arch/blackfin/include/asm/nmi.h
@@ -7,6 +7,8 @@
 #ifndef _BFIN_NMI_H_
 #define _BFIN_NMI_H_
 
+void arch_touch_nmi_watchdog(void);
+
 #include <linux/nmi.h>
 
 #endif
diff --git a/arch/blackfin/kernel/nmi.c b/arch/blackfin/kernel/nmi.c
index 633c37083e87..1e714329fe8a 100644
--- a/arch/blackfin/kernel/nmi.c
+++ b/arch/blackfin/kernel/nmi.c
@@ -190,7 +190,7 @@ static int __init init_nmi_wdt(void)
 }
 device_initcall(init_nmi_wdt);
 
-void touch_nmi_watchdog(void)
+void arch_touch_nmi_watchdog(void)
 {
 	atomic_set(&nmi_touched[smp_processor_id()], 1);
 }
diff --git a/arch/mn10300/include/asm/nmi.h b/arch/mn10300/include/asm/nmi.h
index f3671cbbc117..aee8aa22d9ee 100644
--- a/arch/mn10300/include/asm/nmi.h
+++ b/arch/mn10300/include/asm/nmi.h
@@ -11,4 +11,6 @@
 #ifndef _ASM_NMI_H
 #define _ASM_NMI_H
 
+void arch_touch_nmi_watchdog(void);
+
 #endif /* _ASM_NMI_H */
diff --git a/arch/mn10300/kernel/mn10300-watchdog-low.S b/arch/mn10300/kernel/mn10300-watchdog-low.S
index f2f5c9cfaabd..34f8773de7d0 100644
--- a/arch/mn10300/kernel/mn10300-watchdog-low.S
+++ b/arch/mn10300/kernel/mn10300-watchdog-low.S
@@ -50,9 +50,9 @@ watchdog_handler:
 #   we can't inline it)
 #
 ###############################################################################
-	.globl	touch_nmi_watchdog
-	.type	touch_nmi_watchdog,@function
-touch_nmi_watchdog:
+	.globl	arch_touch_nmi_watchdog
+	.type	arch_touch_nmi_watchdog,@function
+arch_touch_nmi_watchdog:
 	clr	d0
 	clr	d1
 	mov	watchdog_alert_counter, a0
@@ -63,4 +63,4 @@ touch_nmi_watchdog:
 	lne
 	ret	[],0
 
-	.size	touch_nmi_watchdog,.-touch_nmi_watchdog
+	.size	arch_touch_nmi_watchdog,.-arch_touch_nmi_watchdog
diff --git a/arch/mn10300/kernel/mn10300-watchdog.c b/arch/mn10300/kernel/mn10300-watchdog.c
index a2d8e6938d67..0d5641beadf5 100644
--- a/arch/mn10300/kernel/mn10300-watchdog.c
+++ b/arch/mn10300/kernel/mn10300-watchdog.c
@@ -31,7 +31,7 @@ static unsigned int watchdog;
 static unsigned int watchdog_hz = 1;
 unsigned int watchdog_alert_counter[NR_CPUS];
 
-EXPORT_SYMBOL(touch_nmi_watchdog);
+EXPORT_SYMBOL(arch_touch_nmi_watchdog);
 
 /*
  * the best way to detect whether a CPU has a 'hard lockup' problem
diff --git a/arch/sparc/include/asm/nmi.h b/arch/sparc/include/asm/nmi.h
index 26ad2b2607c6..284eac3ffaf2 100644
--- a/arch/sparc/include/asm/nmi.h
+++ b/arch/sparc/include/asm/nmi.h
@@ -7,6 +7,7 @@ void nmi_adjust_hz(unsigned int new_hz);
 
 extern atomic_t nmi_active;
 
+void arch_touch_nmi_watchdog(void);
 void start_nmi_watchdog(void *unused);
 void stop_nmi_watchdog(void *unused);
 
diff --git a/arch/sparc/kernel/nmi.c b/arch/sparc/kernel/nmi.c
index 95e73c63c99d..048ad783ea3f 100644
--- a/arch/sparc/kernel/nmi.c
+++ b/arch/sparc/kernel/nmi.c
@@ -51,7 +51,7 @@ static DEFINE_PER_CPU(unsigned int, last_irq_sum);
 static DEFINE_PER_CPU(long, alert_counter);
 static DEFINE_PER_CPU(int, nmi_touch);
 
-void touch_nmi_watchdog(void)
+void arch_touch_nmi_watchdog(void)
 {
 	if (atomic_read(&nmi_active)) {
 		int cpu;
@@ -61,10 +61,8 @@ void touch_nmi_watchdog(void)
 				per_cpu(nmi_touch, cpu) = 1;
 		}
 	}
-
-	touch_softlockup_watchdog();
 }
-EXPORT_SYMBOL(touch_nmi_watchdog);
+EXPORT_SYMBOL(arch_touch_nmi_watchdog);
 
 static void die_nmi(const char *str, struct pt_regs *regs, int do_panic)
 {
diff --git a/include/linux/nmi.h b/include/linux/nmi.h
index 5e2e57536d98..6ea465a842a1 100644
--- a/include/linux/nmi.h
+++ b/include/linux/nmi.h
@@ -6,6 +6,9 @@
 
 #include <linux/sched.h>
 #include <asm/irq.h>
+#if defined(CONFIG_HAVE_NMI_WATCHDOG)
+#include <asm/nmi.h>
+#endif
 
 #ifdef CONFIG_LOCKUP_DETECTOR
 extern void touch_softlockup_watchdog_sched(void);
@@ -58,6 +61,14 @@ static inline void reset_hung_task_detector(void)
 #define NMI_WATCHDOG_ENABLED      (1 << NMI_WATCHDOG_ENABLED_BIT)
 #define SOFT_WATCHDOG_ENABLED     (1 << SOFT_WATCHDOG_ENABLED_BIT)
 
+#if defined(CONFIG_HARDLOCKUP_DETECTOR)
+extern void hld_touch_nmi_watchdog(void);
+extern void hardlockup_detector_disable(void);
+#else
+static inline void hld_touch_nmi_watchdog(void) {}
+static inline void hardlockup_detector_disable(void) {}
+#endif
+
 /**
  * touch_nmi_watchdog - restart NMI watchdog timeout.
  * 
@@ -65,21 +76,14 @@ static inline void reset_hung_task_detector(void)
  * may be used to reset the timeout - for code which intentionally
  * disables interrupts for a long time. This call is stateless.
  */
-#if defined(CONFIG_HAVE_NMI_WATCHDOG) || defined(CONFIG_HARDLOCKUP_DETECTOR)
-#include <asm/nmi.h>
-extern void touch_nmi_watchdog(void);
-#else
 static inline void touch_nmi_watchdog(void)
 {
+#if defined(CONFIG_HAVE_NMI_WATCHDOG)
+	arch_touch_nmi_watchdog();
+#endif
+	hld_touch_nmi_watchdog();
 	touch_softlockup_watchdog();
 }
-#endif
-
-#if defined(CONFIG_HARDLOCKUP_DETECTOR)
-extern void hardlockup_detector_disable(void);
-#else
-static inline void hardlockup_detector_disable(void) {}
-#endif
 
 /*
  * Create trigger_all_cpu_backtrace() out of the arch-provided
diff --git a/kernel/watchdog_hld.c b/kernel/watchdog_hld.c
index 54a427d1f344..e0d7a7c43fb5 100644
--- a/kernel/watchdog_hld.c
+++ b/kernel/watchdog_hld.c
@@ -56,7 +56,7 @@ static int __init hardlockup_panic_setup(char *str)
 }
 __setup("nmi_watchdog=", hardlockup_panic_setup);
 
-void touch_nmi_watchdog(void)
+void hld_touch_nmi_watchdog(void)
 {
 	/*
 	 * Using __raw here because some code paths have
@@ -66,9 +66,8 @@ void touch_nmi_watchdog(void)
 	 * going off.
 	 */
 	raw_cpu_write(watchdog_nmi_touch, true);
-	touch_softlockup_watchdog();
 }
-EXPORT_SYMBOL(touch_nmi_watchdog);
+EXPORT_SYMBOL(hld_touch_nmi_watchdog);
 
 static struct perf_event_attr wd_hw_attr = {
 	.type		= PERF_TYPE_HARDWARE,
-- 
2.11.0

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

#1650525 — Re: [PATCH 2/4] watchdog: introduce arch_touch_nmi_watchdog()

FromDon Zickus <dzickus@redhat.com>
Date2017-05-25 16:00 +0200
SubjectRe: [PATCH 2/4] watchdog: introduce arch_touch_nmi_watchdog()
Message-ID<tL1sZ-sX-1@gated-at.bofh.it>
In reply to#1650325
On Thu, May 25, 2017 at 06:28:54PM +1000, Nicholas Piggin wrote:
> For architectures that define HAVE_NMI_WATCHDOG, instead of having
> them provide the complete touch_nmi_watchdog() function, just have
> them provide arch_touch_nmi_watchdog().
> 
> This gives the generic code more flexibility in implementing this
> function, and arch implementations don't miss out on touching the
> softlockup watchdog or other generic details.

The idea makes sense.  I don't think you can have hld_touch_nmi_watchdog
defined with arch_touch_nmi_watchdog, so I am wondering if it makes sense to
combine them somehow.  Though renaming hld_touch_nmi_watchdog to
arch_touch_nmi_watchdog sounds odd, I think it mimics the idea.

Cheers,
Don

> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>  arch/blackfin/include/asm/nmi.h            |  2 ++
>  arch/blackfin/kernel/nmi.c                 |  2 +-
>  arch/mn10300/include/asm/nmi.h             |  2 ++
>  arch/mn10300/kernel/mn10300-watchdog-low.S |  8 ++++----
>  arch/mn10300/kernel/mn10300-watchdog.c     |  2 +-
>  arch/sparc/include/asm/nmi.h               |  1 +
>  arch/sparc/kernel/nmi.c                    |  6 ++----
>  include/linux/nmi.h                        | 26 +++++++++++++++-----------
>  kernel/watchdog_hld.c                      |  5 ++---
>  9 files changed, 30 insertions(+), 24 deletions(-)
> 
> diff --git a/arch/blackfin/include/asm/nmi.h b/arch/blackfin/include/asm/nmi.h
> index b9caac4fcfd8..6e7047ceec79 100644
> --- a/arch/blackfin/include/asm/nmi.h
> +++ b/arch/blackfin/include/asm/nmi.h
> @@ -7,6 +7,8 @@
>  #ifndef _BFIN_NMI_H_
>  #define _BFIN_NMI_H_
>  
> +void arch_touch_nmi_watchdog(void);
> +
>  #include <linux/nmi.h>
>  
>  #endif
> diff --git a/arch/blackfin/kernel/nmi.c b/arch/blackfin/kernel/nmi.c
> index 633c37083e87..1e714329fe8a 100644
> --- a/arch/blackfin/kernel/nmi.c
> +++ b/arch/blackfin/kernel/nmi.c
> @@ -190,7 +190,7 @@ static int __init init_nmi_wdt(void)
>  }
>  device_initcall(init_nmi_wdt);
>  
> -void touch_nmi_watchdog(void)
> +void arch_touch_nmi_watchdog(void)
>  {
>  	atomic_set(&nmi_touched[smp_processor_id()], 1);
>  }
> diff --git a/arch/mn10300/include/asm/nmi.h b/arch/mn10300/include/asm/nmi.h
> index f3671cbbc117..aee8aa22d9ee 100644
> --- a/arch/mn10300/include/asm/nmi.h
> +++ b/arch/mn10300/include/asm/nmi.h
> @@ -11,4 +11,6 @@
>  #ifndef _ASM_NMI_H
>  #define _ASM_NMI_H
>  
> +void arch_touch_nmi_watchdog(void);
> +
>  #endif /* _ASM_NMI_H */
> diff --git a/arch/mn10300/kernel/mn10300-watchdog-low.S b/arch/mn10300/kernel/mn10300-watchdog-low.S
> index f2f5c9cfaabd..34f8773de7d0 100644
> --- a/arch/mn10300/kernel/mn10300-watchdog-low.S
> +++ b/arch/mn10300/kernel/mn10300-watchdog-low.S
> @@ -50,9 +50,9 @@ watchdog_handler:
>  #   we can't inline it)
>  #
>  ###############################################################################
> -	.globl	touch_nmi_watchdog
> -	.type	touch_nmi_watchdog,@function
> -touch_nmi_watchdog:
> +	.globl	arch_touch_nmi_watchdog
> +	.type	arch_touch_nmi_watchdog,@function
> +arch_touch_nmi_watchdog:
>  	clr	d0
>  	clr	d1
>  	mov	watchdog_alert_counter, a0
> @@ -63,4 +63,4 @@ touch_nmi_watchdog:
>  	lne
>  	ret	[],0
>  
> -	.size	touch_nmi_watchdog,.-touch_nmi_watchdog
> +	.size	arch_touch_nmi_watchdog,.-arch_touch_nmi_watchdog
> diff --git a/arch/mn10300/kernel/mn10300-watchdog.c b/arch/mn10300/kernel/mn10300-watchdog.c
> index a2d8e6938d67..0d5641beadf5 100644
> --- a/arch/mn10300/kernel/mn10300-watchdog.c
> +++ b/arch/mn10300/kernel/mn10300-watchdog.c
> @@ -31,7 +31,7 @@ static unsigned int watchdog;
>  static unsigned int watchdog_hz = 1;
>  unsigned int watchdog_alert_counter[NR_CPUS];
>  
> -EXPORT_SYMBOL(touch_nmi_watchdog);
> +EXPORT_SYMBOL(arch_touch_nmi_watchdog);
>  
>  /*
>   * the best way to detect whether a CPU has a 'hard lockup' problem
> diff --git a/arch/sparc/include/asm/nmi.h b/arch/sparc/include/asm/nmi.h
> index 26ad2b2607c6..284eac3ffaf2 100644
> --- a/arch/sparc/include/asm/nmi.h
> +++ b/arch/sparc/include/asm/nmi.h
> @@ -7,6 +7,7 @@ void nmi_adjust_hz(unsigned int new_hz);
>  
>  extern atomic_t nmi_active;
>  
> +void arch_touch_nmi_watchdog(void);
>  void start_nmi_watchdog(void *unused);
>  void stop_nmi_watchdog(void *unused);
>  
> diff --git a/arch/sparc/kernel/nmi.c b/arch/sparc/kernel/nmi.c
> index 95e73c63c99d..048ad783ea3f 100644
> --- a/arch/sparc/kernel/nmi.c
> +++ b/arch/sparc/kernel/nmi.c
> @@ -51,7 +51,7 @@ static DEFINE_PER_CPU(unsigned int, last_irq_sum);
>  static DEFINE_PER_CPU(long, alert_counter);
>  static DEFINE_PER_CPU(int, nmi_touch);
>  
> -void touch_nmi_watchdog(void)
> +void arch_touch_nmi_watchdog(void)
>  {
>  	if (atomic_read(&nmi_active)) {
>  		int cpu;
> @@ -61,10 +61,8 @@ void touch_nmi_watchdog(void)
>  				per_cpu(nmi_touch, cpu) = 1;
>  		}
>  	}
> -
> -	touch_softlockup_watchdog();
>  }
> -EXPORT_SYMBOL(touch_nmi_watchdog);
> +EXPORT_SYMBOL(arch_touch_nmi_watchdog);
>  
>  static void die_nmi(const char *str, struct pt_regs *regs, int do_panic)
>  {
> diff --git a/include/linux/nmi.h b/include/linux/nmi.h
> index 5e2e57536d98..6ea465a842a1 100644
> --- a/include/linux/nmi.h
> +++ b/include/linux/nmi.h
> @@ -6,6 +6,9 @@
>  
>  #include <linux/sched.h>
>  #include <asm/irq.h>
> +#if defined(CONFIG_HAVE_NMI_WATCHDOG)
> +#include <asm/nmi.h>
> +#endif
>  
>  #ifdef CONFIG_LOCKUP_DETECTOR
>  extern void touch_softlockup_watchdog_sched(void);
> @@ -58,6 +61,14 @@ static inline void reset_hung_task_detector(void)
>  #define NMI_WATCHDOG_ENABLED      (1 << NMI_WATCHDOG_ENABLED_BIT)
>  #define SOFT_WATCHDOG_ENABLED     (1 << SOFT_WATCHDOG_ENABLED_BIT)
>  
> +#if defined(CONFIG_HARDLOCKUP_DETECTOR)
> +extern void hld_touch_nmi_watchdog(void);
> +extern void hardlockup_detector_disable(void);
> +#else
> +static inline void hld_touch_nmi_watchdog(void) {}
> +static inline void hardlockup_detector_disable(void) {}
> +#endif
> +
>  /**
>   * touch_nmi_watchdog - restart NMI watchdog timeout.
>   * 
> @@ -65,21 +76,14 @@ static inline void reset_hung_task_detector(void)
>   * may be used to reset the timeout - for code which intentionally
>   * disables interrupts for a long time. This call is stateless.
>   */
> -#if defined(CONFIG_HAVE_NMI_WATCHDOG) || defined(CONFIG_HARDLOCKUP_DETECTOR)
> -#include <asm/nmi.h>
> -extern void touch_nmi_watchdog(void);
> -#else
>  static inline void touch_nmi_watchdog(void)
>  {
> +#if defined(CONFIG_HAVE_NMI_WATCHDOG)
> +	arch_touch_nmi_watchdog();
> +#endif
> +	hld_touch_nmi_watchdog();
>  	touch_softlockup_watchdog();
>  }
> -#endif
> -
> -#if defined(CONFIG_HARDLOCKUP_DETECTOR)
> -extern void hardlockup_detector_disable(void);
> -#else
> -static inline void hardlockup_detector_disable(void) {}
> -#endif
>  
>  /*
>   * Create trigger_all_cpu_backtrace() out of the arch-provided
> diff --git a/kernel/watchdog_hld.c b/kernel/watchdog_hld.c
> index 54a427d1f344..e0d7a7c43fb5 100644
> --- a/kernel/watchdog_hld.c
> +++ b/kernel/watchdog_hld.c
> @@ -56,7 +56,7 @@ static int __init hardlockup_panic_setup(char *str)
>  }
>  __setup("nmi_watchdog=", hardlockup_panic_setup);
>  
> -void touch_nmi_watchdog(void)
> +void hld_touch_nmi_watchdog(void)
>  {
>  	/*
>  	 * Using __raw here because some code paths have
> @@ -66,9 +66,8 @@ void touch_nmi_watchdog(void)
>  	 * going off.
>  	 */
>  	raw_cpu_write(watchdog_nmi_touch, true);
> -	touch_softlockup_watchdog();
>  }
> -EXPORT_SYMBOL(touch_nmi_watchdog);
> +EXPORT_SYMBOL(hld_touch_nmi_watchdog);
>  
>  static struct perf_event_attr wd_hw_attr = {
>  	.type		= PERF_TYPE_HARDWARE,
> -- 
> 2.11.0
>

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

#1650997 — Re: [PATCH 2/4] watchdog: introduce arch_touch_nmi_watchdog()

FromNicholas Piggin <npiggin@gmail.com>
Date2017-05-26 02:40 +0200
SubjectRe: [PATCH 2/4] watchdog: introduce arch_touch_nmi_watchdog()
Message-ID<tLbsl-73T-7@gated-at.bofh.it>
In reply to#1650525
On Thu, 25 May 2017 09:55:59 -0400
Don Zickus <dzickus@redhat.com> wrote:

> On Thu, May 25, 2017 at 06:28:54PM +1000, Nicholas Piggin wrote:
> > For architectures that define HAVE_NMI_WATCHDOG, instead of having
> > them provide the complete touch_nmi_watchdog() function, just have
> > them provide arch_touch_nmi_watchdog().
> > 
> > This gives the generic code more flexibility in implementing this
> > function, and arch implementations don't miss out on touching the
> > softlockup watchdog or other generic details.  
> 
> The idea makes sense.  I don't think you can have hld_touch_nmi_watchdog
> defined with arch_touch_nmi_watchdog, so I am wondering if it makes sense to
> combine them somehow.  Though renaming hld_touch_nmi_watchdog to
> arch_touch_nmi_watchdog sounds odd, I think it mimics the idea.

Yeah I agree it's not quite right, and I think using
arch_touch_nmi_watchdog would be fine for the hld, which makes sense
if you think of it as a utility or library function for architectures
that want a hardlockup watchdog and can use perf for it.

I can change that if you prefer. BTW the 0day picked up another
Kconfig compile bug, so I'll respin the series and include any changes
you like.

Thanks,
Nick

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

#1651404 — Re: [PATCH 2/4] watchdog: introduce arch_touch_nmi_watchdog()

FromDon Zickus <dzickus@redhat.com>
Date2017-05-26 16:10 +0200
SubjectRe: [PATCH 2/4] watchdog: introduce arch_touch_nmi_watchdog()
Message-ID<tLo6e-6TK-13@gated-at.bofh.it>
In reply to#1650997
On Fri, May 26, 2017 at 10:31:03AM +1000, Nicholas Piggin wrote:
> On Thu, 25 May 2017 09:55:59 -0400
> Don Zickus <dzickus@redhat.com> wrote:
> 
> > On Thu, May 25, 2017 at 06:28:54PM +1000, Nicholas Piggin wrote:
> > > For architectures that define HAVE_NMI_WATCHDOG, instead of having
> > > them provide the complete touch_nmi_watchdog() function, just have
> > > them provide arch_touch_nmi_watchdog().
> > > 
> > > This gives the generic code more flexibility in implementing this
> > > function, and arch implementations don't miss out on touching the
> > > softlockup watchdog or other generic details.  
> > 
> > The idea makes sense.  I don't think you can have hld_touch_nmi_watchdog
> > defined with arch_touch_nmi_watchdog, so I am wondering if it makes sense to
> > combine them somehow.  Though renaming hld_touch_nmi_watchdog to
> > arch_touch_nmi_watchdog sounds odd, I think it mimics the idea.
> 
> Yeah I agree it's not quite right, and I think using
> arch_touch_nmi_watchdog would be fine for the hld, which makes sense
> if you think of it as a utility or library function for architectures
> that want a hardlockup watchdog and can use perf for it.

Yeah, if you wouldn't mind trying that.  Over the last year it seems there
is a push to make the hld more of a separate thing if folks want to use
perf.  I have been trying to tweak it so it can be used in-place of the arch
solution or just use the arch solution.  And still retain the same function
calls.

Cheers,
Don

> 
> I can change that if you prefer. BTW the 0day picked up another
> Kconfig compile bug, so I'll respin the series and include any changes
> you like.
> 
> Thanks,
> Nick

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web