(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
>
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
>>
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