lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Thu, 15 Jun 2017 13:04:01 +1000
From:   Nicholas Piggin <npiggin@...il.com>
To:     Babu Moger <babu.moger@...cle.com>
Cc:     Don Zickus <dzickus@...hat.com>, linux-kernel@...r.kernel.org,
        linux-arch@...r.kernel.org
Subject: Re: [PATCH 3/4] watchdog: Split up config options

On Wed, 14 Jun 2017 21:16:04 -0500
Babu Moger <babu.moger@...cle.com> wrote:

> Hi Don,
> 
> On 6/14/2017 9:09 AM, Don Zickus wrote:
> > On Wed, Jun 14, 2017 at 02:11:18AM +1000, Nicholas Piggin wrote:  
> >>> Yeah, if you wouldn't mind.  Sorry for dragging this out, but I feel like we
> >>> are getting close to have this defined properly which would allow us to
> >>> split the code up correctly in the future.  
> >> How's this for a replacement patch 3? I think the Kconfig works out much
> >> better now.  
> > Hi Nick,
> >
> > I think you made this much clearer, thank you!  I am good with this.
> >
> >
> > Hi Babu,
> >
> > Can you give this patchset (and particularly this version of patch 3) a try
> > on sparc to make sure we didn't break anything?  I believe this should
> > resolve the start nmi watchdog on boot issue you noticed.  Thanks!  
> 
> There is still one problem with the patch.
> 
> # cat /proc/sys/kernel/watchdog
> 1
> # cat /proc/sys/kernel/nmi_watchdog
> 0
> 
> Problem is setting the initial value for  "nmi_watchdog"
> 
> We need something(or similar) patch on top to address this.
> ============================================
> diff --git a/kernel/watchdog.c b/kernel/watchdog.c
> index 5397c63..0105856 100644
> --- a/kernel/watchdog.c
> +++ b/kernel/watchdog.c
> @@ -34,9 +34,13 @@
> 
>   int __read_mostly nmi_watchdog_enabled;
> 
> -#ifdef CONFIG_HARDLOCKUP_DETECTOR
> +#if defined(CONFIG_HARDLOCKUP_DETECTOR) || 
> defined(CONFIG_HAVE_NMI_WATCHDOG)
>   unsigned long __read_mostly watchdog_enabled = 
> SOFT_WATCHDOG_ENABLED|NMI_WATCHDOG_ENABLED;
> +#else
> +unsigned long __read_mostly watchdog_enabled = SOFT_WATCHDOG_ENABLED;
> +#endif
> 
> +#ifdef CONFIG_HARDLOCKUP_DETECTOR
>   /* boot commands */
>   /*
>    * Should we panic when a soft-lockup or hard-lockup occurs:
> @@ -69,9 +73,6 @@ static int __init hardlockup_panic_setup(char *str)
>          return 1;
>   }
>   __setup("nmi_watchdog=", hardlockup_panic_setup);
> -
> -#else
> -unsigned long __read_mostly watchdog_enabled = SOFT_WATCHDOG_ENABLED;
>   #endif
> 
>   #ifdef CONFIG_SOFTLOCKUP_DETECTOR

Hmm, I guess I missed this because sparc parses nmi_watchdog=, but it
also relies on the watchdog_enabled value.

I guess I can fold your incremental patch in. I hope we could get
sparc quickly to adopt the complate HAVE_HARDLOCKUP_DETECTOR_ARCH soon
afterwards though, so we only have 2 cases -- complete hardlockup
detector, or the very bare minimum NMI_WATCHDOG.

Thanks,
Nick

Powered by blists - more mailing lists