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

Groups > linux.kernel > #1652757 > unrolled thread

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

Started byNicholas Piggin <npiggin@gmail.com>
First post2017-05-30 03:30 +0200
Last post2017-06-07 16:40 +0200
Articles 7 — 3 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/4][V3] Improve watchdog config for arch watchdogs Nicholas Piggin <npiggin@gmail.com> - 2017-05-30 03:30 +0200
    [PATCH 1/4] watchdog: remove unused declaration Nicholas Piggin <npiggin@gmail.com> - 2017-05-30 03:30 +0200
    [PATCH 2/4] watchdog: Introduce arch_touch_nmi_watchdog() Nicholas Piggin <npiggin@gmail.com> - 2017-05-30 03:30 +0200
    [PATCH 4/4] watchdog: Provide watchdog_reconfigure() for arch watchdogs Nicholas Piggin <npiggin@gmail.com> - 2017-05-30 03:30 +0200
    Re: [PATCH 0/4][V3] Improve watchdog config for arch watchdogs Don Zickus <dzickus@redhat.com> - 2017-06-06 18:10 +0200
      Re: [PATCH 0/4][V3] Improve watchdog config for arch watchdogs Babu Moger <babu.moger@oracle.com> - 2017-06-06 21:50 +0200
        Re: [PATCH 0/4][V3] Improve watchdog config for arch watchdogs Don Zickus <dzickus@redhat.com> - 2017-06-07 16:40 +0200

#1652757 — [PATCH 0/4][V3] Improve watchdog config for arch watchdogs

FromNicholas Piggin <npiggin@gmail.com>
Date2017-05-30 03:30 +0200
Subject[PATCH 0/4][V3] Improve watchdog config for arch watchdogs
Message-ID<tME8V-8vo-3@gated-at.bofh.it>
Since last time:

- Have the perf based hardlockup detector use arch_touch_nmi_watchdog()
  rather than hld_touch_nmi_watchdog(). This changes direction slightly
  to make the perf-based hard lockup detector an alternative that an
  arch may select, rather than standalone. This better reflects how the
  code works in practice).

- Hopefully fixed the Kconfig options. There's still a bit of ugliness
  that will require another pass or two over interfaces and config
  scheme, but the idea is to make a minimal change to get the powerpc
  HLD in, which gives a reasonable starting point to improve things
  further.

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                        |  57 ++++---
 kernel/Makefile                            |   2 +-
 kernel/sysctl.c                            |  18 +-
 kernel/watchdog.c                          | 263 +++++++++++++++++++----------
 kernel/watchdog_hld.c                      |  37 +---
 lib/Kconfig.debug                          |  29 +++-
 15 files changed, 263 insertions(+), 170 deletions(-)

-- 
2.11.0

[toc] | [next] | [standalone]

#1652758 — [PATCH 1/4] watchdog: remove unused declaration

FromNicholas Piggin <npiggin@gmail.com>
Date2017-05-30 03:30 +0200
Subject[PATCH 1/4] watchdog: remove unused declaration
Message-ID<tME8V-8vo-5@gated-at.bofh.it>
In reply to#1652757
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 include/linux/nmi.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/include/linux/nmi.h b/include/linux/nmi.h
index aa3cd0878270..5e2e57536d98 100644
--- a/include/linux/nmi.h
+++ b/include/linux/nmi.h
@@ -12,9 +12,6 @@ extern void touch_softlockup_watchdog_sched(void);
 extern void touch_softlockup_watchdog(void);
 extern void touch_softlockup_watchdog_sync(void);
 extern void touch_all_softlockup_watchdogs(void);
-extern int proc_dowatchdog_thresh(struct ctl_table *table, int write,
-				  void __user *buffer,
-				  size_t *lenp, loff_t *ppos);
 extern unsigned int  softlockup_panic;
 extern unsigned int  hardlockup_panic;
 void lockup_detector_init(void);
-- 
2.11.0

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

#1652759 — [PATCH 2/4] watchdog: Introduce arch_touch_nmi_watchdog()

FromNicholas Piggin <npiggin@gmail.com>
Date2017-05-30 03:30 +0200
Subject[PATCH 2/4] watchdog: Introduce arch_touch_nmi_watchdog()
Message-ID<tME8V-8vo-7@gated-at.bofh.it>
In reply to#1652757
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                        | 27 ++++++++++++++++-----------
 kernel/watchdog_hld.c                      |  5 ++---
 9 files changed, 31 insertions(+), 24 deletions(-)

diff --git a/arch/blackfin/include/asm/nmi.h b/arch/blackfin/include/asm/nmi.h
index b9caac4fcfd8..107d23705f46 100644
--- a/arch/blackfin/include/asm/nmi.h
+++ b/arch/blackfin/include/asm/nmi.h
@@ -9,4 +9,6 @@
 
 #include <linux/nmi.h>
 
+extern void arch_touch_nmi_watchdog(void);
+
 #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..b05627597b1b 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
 
+extern 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..bd387ef8bccd 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,18 @@ 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 hardlockup_detector_disable(void);
+#else
+static inline void hardlockup_detector_disable(void) {}
+#endif
+
+#if defined(CONFIG_HARDLOCKUP_DETECTOR) || defined(CONFIG_HAVE_NMI_WATCHDOG)
+extern void arch_touch_nmi_watchdog(void);
+#else
+static inline void arch_touch_nmi_watchdog(void) {}
+#endif
+
 /**
  * touch_nmi_watchdog - restart NMI watchdog timeout.
  * 
@@ -65,21 +80,11 @@ 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)
 {
+	arch_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..90d688df6ce1 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 arch_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(arch_touch_nmi_watchdog);
 
 static struct perf_event_attr wd_hw_attr = {
 	.type		= PERF_TYPE_HARDWARE,
-- 
2.11.0

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

#1652760 — [PATCH 4/4] watchdog: Provide watchdog_reconfigure() for arch watchdogs

FromNicholas Piggin <npiggin@gmail.com>
Date2017-05-30 03:30 +0200
Subject[PATCH 4/4] watchdog: Provide watchdog_reconfigure() for arch watchdogs
Message-ID<tME8V-8vo-9@gated-at.bofh.it>
In reply to#1652757
After reconfiguring watchdog sysctls etc., architecture specific
watchdogs may not get all their parameters updated.

watchdog_reconfigure() can be implemented to pull the new values
in and set the arch NMI watchdog.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 kernel/watchdog.c | 29 +++++++++++++++++++++++++----
 1 file changed, 25 insertions(+), 4 deletions(-)

diff --git a/kernel/watchdog.c b/kernel/watchdog.c
index deb010505646..5397c637db2d 100644
--- a/kernel/watchdog.c
+++ b/kernel/watchdog.c
@@ -120,6 +120,11 @@ void __weak watchdog_nmi_disable(unsigned int cpu)
 {
 }
 
+void __weak watchdog_nmi_reconfigure(void)
+{
+}
+
+
 #ifdef CONFIG_SOFTLOCKUP_DETECTOR
 
 /* Helper for online, unparked cpus. */
@@ -597,6 +602,12 @@ static void watchdog_disable_all_cpus(void)
 	}
 }
 
+static int watchdog_update_cpus(void)
+{
+	return smpboot_update_cpumask_percpu_thread(
+		    &watchdog_threads, &watchdog_cpumask);
+}
+
 #else /* SOFTLOCKUP */
 static int watchdog_park_threads(void)
 {
@@ -616,6 +627,11 @@ static void watchdog_disable_all_cpus(void)
 {
 }
 
+static int watchdog_update_cpus(void)
+{
+	return 0;
+}
+
 static void set_sample_period(void)
 {
 }
@@ -648,6 +664,8 @@ int lockup_detector_suspend(void)
 		watchdog_enabled = 0;
 	}
 
+	watchdog_nmi_reconfigure();
+
 	mutex_unlock(&watchdog_proc_mutex);
 
 	return ret;
@@ -668,6 +686,8 @@ void lockup_detector_resume(void)
 	if (watchdog_running && !watchdog_suspended)
 		watchdog_unpark_threads();
 
+	watchdog_nmi_reconfigure();
+
 	mutex_unlock(&watchdog_proc_mutex);
 	put_online_cpus();
 }
@@ -693,6 +713,8 @@ static int proc_watchdog_update(void)
 	else
 		watchdog_disable_all_cpus();
 
+	watchdog_nmi_reconfigure();
+
 	return err;
 
 }
@@ -878,12 +900,11 @@ int proc_watchdog_cpumask(struct ctl_table *table, int write,
 			 * a temporary cpumask, so we are likely not in a
 			 * position to do much else to make things better.
 			 */
-#ifdef CONFIG_SOFTLOCKUP_DETECTOR
-			if (smpboot_update_cpumask_percpu_thread(
-				    &watchdog_threads, &watchdog_cpumask) != 0)
+			if (watchdog_update_cpus() != 0)
 				pr_err("cpumask update failed\n");
-#endif
 		}
+
+		watchdog_nmi_reconfigure();
 	}
 out:
 	mutex_unlock(&watchdog_proc_mutex);
-- 
2.11.0

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

#1658890

FromDon Zickus <dzickus@redhat.com>
Date2017-06-06 18:10 +0200
Message-ID<tPpdn-2Df-7@gated-at.bofh.it>
In reply to#1652757
(adding Babu)

On Tue, May 30, 2017 at 11:26:55AM +1000, Nicholas Piggin wrote:
> Since last time:
> 
> - Have the perf based hardlockup detector use arch_touch_nmi_watchdog()
>   rather than hld_touch_nmi_watchdog(). This changes direction slightly
>   to make the perf-based hard lockup detector an alternative that an
>   arch may select, rather than standalone. This better reflects how the
>   code works in practice).
> 
> - Hopefully fixed the Kconfig options. There's still a bit of ugliness
>   that will require another pass or two over interfaces and config
>   scheme, but the idea is to make a minimal change to get the powerpc
>   HLD in, which gives a reasonable starting point to improve things
>   further.

Hi Babu,

Does this patchset break sparc?  Specifically patch3 with all the config
option changes?

Cheers,
Don

> 
> 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                        |  57 ++++---
>  kernel/Makefile                            |   2 +-
>  kernel/sysctl.c                            |  18 +-
>  kernel/watchdog.c                          | 263 +++++++++++++++++++----------
>  kernel/watchdog_hld.c                      |  37 +---
>  lib/Kconfig.debug                          |  29 +++-
>  15 files changed, 263 insertions(+), 170 deletions(-)
> 
> -- 
> 2.11.0
>

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

#1659155

FromBabu Moger <babu.moger@oracle.com>
Date2017-06-06 21:50 +0200
Message-ID<tPsEi-4Hz-3@gated-at.bofh.it>
In reply to#1658890
Hi Don, Nicholas,


On 6/6/2017 11:08 AM, Don Zickus wrote:
> (adding Babu)
>
> On Tue, May 30, 2017 at 11:26:55AM +1000, Nicholas Piggin wrote:
>> Since last time:
>>
>> - Have the perf based hardlockup detector use arch_touch_nmi_watchdog()
>>    rather than hld_touch_nmi_watchdog(). This changes direction slightly
>>    to make the perf-based hard lockup detector an alternative that an
>>    arch may select, rather than standalone. This better reflects how the
>>    code works in practice).
>>
>> - Hopefully fixed the Kconfig options. There's still a bit of ugliness
>>    that will require another pass or two over interfaces and config
>>    scheme, but the idea is to make a minimal change to get the powerpc
>>    HLD in, which gives a reasonable starting point to improve things
>>    further.
> Hi Babu,
>
> Does this patchset break sparc?  Specifically patch3 with all the config
Patches applies, compiles fine and also works fine for most part. 
However, there are few issues.

We need to enter 'N' or 'Y'  for SOFTLOCKUP_DETECTOR.

*
* Restart config...
*
*
* Debug Lockups and Hangs
*
Detect Hard and Soft Lockups (LOCKUP_DETECTOR) [Y/n/?] y
   Detect Soft Lockups (SOFTLOCKUP_DETECTOR) [N/y] (NEW)

For SPARC, softlockup is enabled by default earlier.   May be we need to 
submit another patch to enable this in

arch/sparc/configs/sparc64_defconfig.   Not  a big issue.


Another issue.
before that patch

# cat /proc/sys/kernel/watchdog
1
# cat /proc/sys/kernel/nmi_watchdog
1

After the patch

# cat /proc/sys/kernel/watchdog
1
# cat /proc/sys/kernel/nmi_watchdog

0

I think this is mostly due to change in this code below.

#ifdef CONFIG_HARDLOCKUP_DETECTOR
unsigned long __read_mostly watchdog_enabled = 
SOFT_WATCHDOG_ENABLED|NMI_WATCHDOG_ENABLED;

Old code was like this

#if defined(CONFIG_HAVE_NMI_WATCHDOG) || defined(CONFIG_HARDLOCKUP_DETECTOR)
unsigned long __read_mostly watchdog_enabled = 
SOFT_WATCHDOG_ENABLED|NMI_WATCHDOG_ENABLED;
#else
unsigned long __read_mostly watchdog_enabled = SOFT_WATCHDOG_ENABLED;
#endif

SPARC defines CONFIG_HAVE_NMI_WATCHDOG.

Thanks
Babu

> option changes?
>
> Cheers,
> Don
>
>> 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                        |  57 ++++---
>>   kernel/Makefile                            |   2 +-
>>   kernel/sysctl.c                            |  18 +-
>>   kernel/watchdog.c                          | 263 +++++++++++++++++++----------
>>   kernel/watchdog_hld.c                      |  37 +---
>>   lib/Kconfig.debug                          |  29 +++-
>>   15 files changed, 263 insertions(+), 170 deletions(-)
>>
>> -- 
>> 2.11.0
>>

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

#1659860

FromDon Zickus <dzickus@redhat.com>
Date2017-06-07 16:40 +0200
Message-ID<tPKhQ-7Rr-35@gated-at.bofh.it>
In reply to#1659155
On Tue, Jun 06, 2017 at 02:46:48PM -0500, Babu Moger wrote:
> Hi Don, Nicholas,
> 
> 
> On 6/6/2017 11:08 AM, Don Zickus wrote:
> > (adding Babu)
> > 
> > On Tue, May 30, 2017 at 11:26:55AM +1000, Nicholas Piggin wrote:
> > > Since last time:
> > > 
> > > - Have the perf based hardlockup detector use arch_touch_nmi_watchdog()
> > >    rather than hld_touch_nmi_watchdog(). This changes direction slightly
> > >    to make the perf-based hard lockup detector an alternative that an
> > >    arch may select, rather than standalone. This better reflects how the
> > >    code works in practice).
> > > 
> > > - Hopefully fixed the Kconfig options. There's still a bit of ugliness
> > >    that will require another pass or two over interfaces and config
> > >    scheme, but the idea is to make a minimal change to get the powerpc
> > >    HLD in, which gives a reasonable starting point to improve things
> > >    further.
> > Hi Babu,
> > 
> > Does this patchset break sparc?  Specifically patch3 with all the config
> Patches applies, compiles fine and also works fine for most part. However,
> there are few issues.

Thanks for the quick turnaround!

> 
> We need to enter 'N' or 'Y'  for SOFTLOCKUP_DETECTOR.
> 
> *
> * Restart config...
> *
> *
> * Debug Lockups and Hangs
> *
> Detect Hard and Soft Lockups (LOCKUP_DETECTOR) [Y/n/?] y
>   Detect Soft Lockups (SOFTLOCKUP_DETECTOR) [N/y] (NEW)
> 
> For SPARC, softlockup is enabled by default earlier.   May be we need to
> submit another patch to enable this in
> 
> arch/sparc/configs/sparc64_defconfig.   Not  a big issue.

Hmm, I think the nmi_enable/disable stuff is wrapped into the
SOFTLOCKUP_DETECTOR code, so you might need it.  Though Nick did create a
separate interface outside of SOFTLOCKUP to something similar.  I believe
patch4 introduces nmi_reconfigure().

Not sure if the spirit of the sparc nmi_watchdog code wants SOFTLOCKUP or
not.

> 
> 
> Another issue.
> before that patch
> 
> # cat /proc/sys/kernel/watchdog
> 1
> # cat /proc/sys/kernel/nmi_watchdog
> 1
> 
> After the patch
> 
> # cat /proc/sys/kernel/watchdog
> 1
> # cat /proc/sys/kernel/nmi_watchdog
> 0

Yes, that is what I thought.  Thanks for confirming!

> 
> I think this is mostly due to change in this code below.
> 
> #ifdef CONFIG_HARDLOCKUP_DETECTOR
> unsigned long __read_mostly watchdog_enabled =
> SOFT_WATCHDOG_ENABLED|NMI_WATCHDOG_ENABLED;
> 
> Old code was like this
> 
> #if defined(CONFIG_HAVE_NMI_WATCHDOG) || defined(CONFIG_HARDLOCKUP_DETECTOR)
> unsigned long __read_mostly watchdog_enabled =
> SOFT_WATCHDOG_ENABLED|NMI_WATCHDOG_ENABLED;
> #else
> unsigned long __read_mostly watchdog_enabled = SOFT_WATCHDOG_ENABLED;
> #endif
> 
> SPARC defines CONFIG_HAVE_NMI_WATCHDOG.


I am still working with Nick to deal with these config issues.  But I am
going to keep this one in mind while we work through it.

Cheers,
Don

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web