linux-kernel - Re: [PATCH 4/7] watchdog/hardlockup: Enable HAVE_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]

Message-ID: <ZIHbvlw05razk-oJ@alley>
Date:   Thu, 8 Jun 2023 15:46:38 +0200
From:   Petr Mladek <pmladek@...e.com>
To:     Doug Anderson <dianders@...omium.org>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        kgdb-bugreport@...ts.sourceforge.net, linux-kernel@...r.kernel.org,
        Nicholas Piggin <npiggin@...il.com>,
        Michael Ellerman <mpe@...erman.id.au>,
        linuxppc-dev@...ts.ozlabs.org,
        Christophe Leroy <christophe.leroy@...roup.eu>,
        sparclinux@...r.kernel.org,
        "David S . Miller" <davem@...emloft.net>,
        linux-perf-users@...r.kernel.org
Subject: Re: [PATCH 4/7] watchdog/hardlockup: Enable HAVE_NMI_WATCHDOG only
 on sparc64

On Wed 2023-06-07 16:36:35, Doug Anderson wrote:
> Hi,
> 
> On Wed, Jun 7, 2023 at 8:25 AM Petr Mladek <pmladek@...e.com> wrote:
> >
> > diff --git a/arch/Kconfig b/arch/Kconfig
> > index 13c6e596cf9e..57f15babe188 100644
> > --- a/arch/Kconfig
> > +++ b/arch/Kconfig
> > @@ -404,10 +404,9 @@ config HAVE_NMI_WATCHDOG
> >         depends on HAVE_NMI
> >         bool
> >         help
> > -         The arch provides its own hardlockup detector implementation instead
> > +         Sparc64 provides its own hardlockup detector implementation instead
> >           of the generic perf one.
> 
> It's a little weird to document generic things with the specifics of
> the user. The exception, IMO, is when something is deprecated.
> Personally, it would sound less weird to me to say something like:

Or I could replace "The arch" by "Sparc64" in the 5th patch which
renames the variable to HAVE_HARDLOCKUP_DETECTOR_SPARC64. It will
not longer be a generic thing...

Or I could squash the two patches. I did not want to do too many
changes at the same time. But it might actually make sense to
do this in one step.

> 
> > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> > index d201f5d3876b..4b4aa0f941f9 100644
> > --- a/lib/Kconfig.debug
> > +++ b/lib/Kconfig.debug
> > @@ -1050,15 +1050,13 @@ config HAVE_HARDLOCKUP_DETECTOR_BUDDY
> >  #      sparc64: has a custom implementation which is not using the common
> >  #              hardlockup command line options and sysctl interface.
> >  #
> > -# Note that HAVE_NMI_WATCHDOG is used to distinguish the sparc64 specific
> > -# implementaion. It is automatically enabled also for other arch-specific
> > -# variants which set HAVE_HARDLOCKUP_DETECTOR_ARCH. It makes the check
> > -# of avaialable and supported variants quite tricky.
> > +# Note that HAVE_NMI_WATCHDOG is set when the sparc64 specific implementation
> > +# is used.
> >  #
> >  config HARDLOCKUP_DETECTOR
> >         bool "Detect Hard Lockups"
> > -       depends on DEBUG_KERNEL && !S390
> > -       depends on ((HAVE_HARDLOCKUP_DETECTOR_PERF || HAVE_HARDLOCKUP_DETECTOR_BUDDY) && !HAVE_NMI_WATCHDOG) || HAVE_HARDLOCKUP_DETECTOR_ARCH
> > +       depends on DEBUG_KERNEL && !S390 && !HAVE_NMI_WATCHDOG
> > +       depends on HAVE_HARDLOCKUP_DETECTOR_PERF || HAVE_HARDLOCKUP_DETECTOR_BUDDY || HAVE_HARDLOCKUP_DETECTOR_ARCH
> 
> If you add the "!HAVE_NMI_WATCHDOG" as a dependency to
> HAVE_HARDLOCKUP_DETECTOR_BUDDY, as discussed in a previous patch, you
> can skip adding it here.

It it related to the 2nd patch. Let's discuss it there.

> 
> >         imply HARDLOCKUP_DETECTOR_PERF
> >         imply HARDLOCKUP_DETECTOR_BUDDY
> >         select LOCKUP_DETECTOR
> > @@ -1079,7 +1077,7 @@ config HARDLOCKUP_DETECTOR_PREFER_BUDDY
> >         bool "Prefer the buddy CPU hardlockup detector"
> >         depends on HARDLOCKUP_DETECTOR
> >         depends on HAVE_HARDLOCKUP_DETECTOR_PERF && HAVE_HARDLOCKUP_DETECTOR_BUDDY
> > -       depends on !HAVE_NMI_WATCHDOG
> > +       depends on !HAVE_HARLOCKUP_DETECTOR_ARCH
> 
> Don't need this. Architectures never are allowed to define
> HAVE_HARDLOCKUP_DETECTOR_PERF and HAVE_HARLOCKUP_DETECTOR_ARCH

Same here...

Best Regards,
Petr