linux-kernel - Re: linux-next: Tree for Nov 19 (drivers/edac/igen6

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] [day] [month] [year] [list]

Message-ID: <d8880e79-40a0-4d92-29dd-82b0b59d653b@infradead.org>
Date:   Thu, 19 Nov 2020 10:12:28 -0800
From:   Randy Dunlap <rdunlap@...radead.org>
To:     "Luck, Tony" <tony.luck@...el.com>,
        Stephen Rothwell <sfr@...b.auug.org.au>,
        Linux Next Mailing List <linux-next@...r.kernel.org>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        "Zhuo, Qiuxu" <qiuxu.zhuo@...el.com>,
        Nicholas Piggin <npiggin@...il.com>
Subject: Re: linux-next: Tree for Nov 19 (drivers/edac/igen6_edac.c)

On 11/19/20 9:53 AM, Luck, Tony wrote:
>> ../drivers/edac/igen6_edac.c: In function 'ecclog_nmi_handler':
>> ../drivers/edac/igen6_edac.c:525:10: error: 'NMI_DONE' undeclared (first use in this function); did you mean 'DMI_NONE'?
>>    return NMI_DONE;
> 
> This driver has a #include <linux/nmi.h>
> 
> But inside that file it says:
> 
> #if defined(CONFIG_HAVE_NMI_WATCHDOG)
> #include <asm/nmi.h>
> #endif
> 
> and the randconfig used doesn't set CONFIG_HAVE_NMI_WATCHDOG
> 
> 
> Some options:
> 
> 1) Drop that #ifdef from <linux/nmi.h>
> It was introduced as part of this commit:
> f2e0cff85ed1 ("kernel/watchdog: introduce arch_touch_nmi_watchdog()")
> presumably for some good reason.
> 
> 2) Make this edac driver select CONFIG_HAVE_NMI_WATCHDOG
> Yuck!
> 
> 3) Make this driver #include <asm/nmi.h> instead of <linux/nmi.h>
> This fixes this build error, but I thought that general policy was to
> use the <linux/*.h> if it exists rather than the <asm/*.h> one.
> Maybe that's ok here because this is an x86 specific driver?
> 
> I'm leaning toward option #3.

I lean toward option #1 but #3 would be OK.

We generally try NOT to surround #include lines
with ifdef/endif as was done here for some reason.


-- 
~Randy