Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Sender: Ingo Molnar <mingo.kernel.org@gmail.com>
Date:   Wed, 26 May 2021 08:50:20 +0200
From:   Ingo Molnar <mingo@kernel.org>
To:     Randy Dunlap <rdunlap@infradead.org>,
        Andrew Morton <akpm@linux-foundation.org>
Cc:     linux-kernel@vger.kernel.org, Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        x86@kernel.org, Mel Gorman <mgorman@techsingularity.net>
Subject: Re: [PATCH] x86: fixmap: use CONFIG_NR_CPUS instead of NR_CPUS
Message-ID: <YK3vrIB7cWop+UIW@gmail.com>
References: <20210521195918.2183-1-rdunlap@infradead.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20210521195918.2183-1-rdunlap@infradead.org>
Precedence: bulk


* Randy Dunlap <rdunlap@infradead.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.

 - 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.

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...

Thanks,

	Ingo