linux-kernel - Re: [PATCH] x86: fixmap: use CONFIG_NR_CPUS instead of NR

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: <59676378-1b52-cae7-7944-adeffd27190e@infradead.org>
Date:   Wed, 26 May 2021 10:59:51 -0700
From:   Randy Dunlap <rdunlap@...radead.org>
To:     Ingo Molnar <mingo@...nel.org>,
        Andrew Morton <akpm@...ux-foundation.org>
Cc:     linux-kernel@...r.kernel.org, Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        x86@...nel.org, Mel Gorman <mgorman@...hsingularity.net>,
        Tom Rix <trix@...hat.com>
Subject: Re: [PATCH] x86: fixmap: use CONFIG_NR_CPUS instead of NR_CPUS

On 5/25/21 11:50 PM, Ingo Molnar wrote:
> 
> * Randy Dunlap <rdunlap@...radead.org> wrote:
> 
>> Use CONFIG_NR_CPUS instead of NR_CPUS for an enum entry item.
>> (Alternatively, #include <linux/threads.h> unconditionally instead of
>> conditionally.)
>>
>> This fixes 100+ build errors like so:
>>
>> In file included from ../include/asm-generic/early_ioremap.h:6:0,
>>                  from ./arch/x86/include/generated/asm/early_ioremap.h:1,
>>                  from ../arch/x86/include/asm/io.h:44,
>>                  from ../include/linux/io.h:13,
>>                  from ../mm/early_ioremap.c:13:
>> ../arch/x86/include/asm/fixmap.h:103:48: error: ‘NR_CPUS’ undeclared here (not in a function); did you mean ‘NR_OPEN’?
>>   FIX_KMAP_END = FIX_KMAP_BEGIN + (KM_MAX_IDX * NR_CPUS) - 1,
>>
>> Fixes: e972c2511967 ("mm/early_ioremap: add prototype for early_memremap_pgprot_adjust")
> 
> I believe this patch is in the -mm tree, not the x86 tree.
> 
>> @@ -100,7 +100,7 @@ enum fixed_addresses {
>>  #endif
>>  #ifdef CONFIG_KMAP_LOCAL
>>  	FIX_KMAP_BEGIN,	/* reserved pte's for temporary kernel mappings */
>> -	FIX_KMAP_END = FIX_KMAP_BEGIN + (KM_MAX_IDX * NR_CPUS) - 1,
>> +	FIX_KMAP_END = FIX_KMAP_BEGIN + (KM_MAX_IDX * CONFIG_NR_CPUS) - 1,
>>  #ifdef CONFIG_PCI_MMCONFIG
>>  	FIX_PCIE_MCFG,
>>  #endif
> 
> Please resolve this bug properly:
> 
>  - Don't sprinkle low level headers with random CONFIG_NR_CPUS conversions.
> 
>  - <asm/io.h> currently includes <asm/early_ioremap.h>, but this seems 
>    unjustified.

Deleting it causes build errors.

>  - Once early_ioremap.h is gone from io.h, it's potentially possible to 
>    include <linux/threads.h>. More work to resolve dependencies might be 
>    needed though.

Yes, my first patch for this (unsent) just included <linux/threads.h>
in fixmap.h unconditionally instead of conditionally.

> Frankly, I'd prefer if such a low level header dependencies change came in 
> via the x86 tree so we can properly review it, test it, and keep it 
> working. Right now I can only guess what is needed here...

Sure, makes sense.

Mel, do you have any patch suggestions here?  re:

commit e972c2511967181d955f74181d74438e26b2e797
Author: Mel Gorman <mgorman@...hsingularity.net>
Date:   Fri May 21 10:40:56 2021 +1000

    mm/early_ioremap: add prototype for early_memremap_pgprot_adjust


thanks.
-- 
~Randy