linux-kernel - Re: [PATCH v4 2/5] watchdog: introduce arch_touch_nmi

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:   Sat, 17 Jun 2017 12:43:25 +1000
From:   Nicholas Piggin <npiggin@...il.com>
To:     Andrew Morton <akpm@...ux-foundation.org>
Cc:     Don Zickus <dzickus@...hat.com>,
        Babu Moger <babu.moger@...cle.com>,
        linux-kernel@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org
Subject: Re: [PATCH v4 2/5] watchdog: introduce arch_touch_nmi_watchdog()

On Fri, 16 Jun 2017 11:21:17 -0700
Andrew Morton <akpm@...ux-foundation.org> wrote:

> On Fri, 16 Jun 2017 16:57:12 +1000 Nicholas Piggin <npiggin@...il.com> 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.
> > 
> > ...
> >
> > --- 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);  
> 
> Do we actually need to add this to the arch header files...

[snip]

> > +#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
> > +  
> 
> given that we have a global declaration here?

Probably not. I think it was a holdover from an earlier version where
I tried to let the arch declare it (one of the little embedded ones
had a comment somewhere saying it would be nice if they could make it
inline).

There was some difficulty with it, so yes let's remove these and do
that next time.

Thanks,
Nick