linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] x86: fixmap: use CONFIG_NR_CPUS instead of NR_CPUS
@ 2021-05-21 19:59 Randy Dunlap
  2021-05-26  6:50 ` Ingo Molnar
  0 siblings, 1 reply; 6+ messages in thread
From: Randy Dunlap @ 2021-05-21 19:59 UTC (permalink / raw)
  To: linux-kernel
  Cc: Randy Dunlap, Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
	Mel Gorman

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")
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: x86@kernel.org
Cc: Mel Gorman <mgorman@techsingularity.net>
---
 arch/x86/include/asm/fixmap.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- linux-next-20210521.orig/arch/x86/include/asm/fixmap.h
+++ linux-next-20210521/arch/x86/include/asm/fixmap.h
@@ -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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] x86: fixmap: use CONFIG_NR_CPUS instead of NR_CPUS
  2021-05-21 19:59 [PATCH] x86: fixmap: use CONFIG_NR_CPUS instead of NR_CPUS Randy Dunlap
@ 2021-05-26  6:50 ` Ingo Molnar
  2021-05-26 17:59   ` Randy Dunlap
  0 siblings, 1 reply; 6+ messages in thread
From: Ingo Molnar @ 2021-05-26  6:50 UTC (permalink / raw)
  To: Randy Dunlap, Andrew Morton
  Cc: linux-kernel, Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
	Mel Gorman


* 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] x86: fixmap: use CONFIG_NR_CPUS instead of NR_CPUS
  2021-05-26  6:50 ` Ingo Molnar
@ 2021-05-26 17:59   ` Randy Dunlap
  2021-05-27 13:53     ` Mel Gorman
  0 siblings, 1 reply; 6+ messages in thread
From: Randy Dunlap @ 2021-05-26 17:59 UTC (permalink / raw)
  To: Ingo Molnar, Andrew Morton
  Cc: linux-kernel, Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
	Mel Gorman, Tom Rix

On 5/25/21 11:50 PM, Ingo Molnar wrote:
> 
> * 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.

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@techsingularity.net>
Date:   Fri May 21 10:40:56 2021 +1000

    mm/early_ioremap: add prototype for early_memremap_pgprot_adjust


thanks.
-- 
~Randy


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] x86: fixmap: use CONFIG_NR_CPUS instead of NR_CPUS
  2021-05-26 17:59   ` Randy Dunlap
@ 2021-05-27 13:53     ` Mel Gorman
  2021-05-27 15:48       ` Randy Dunlap
  0 siblings, 1 reply; 6+ messages in thread
From: Mel Gorman @ 2021-05-27 13:53 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Ingo Molnar, Andrew Morton, linux-kernel, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, x86, Tom Rix

On Wed, May 26, 2021 at 10:59:51AM -0700, Randy Dunlap wrote:
> >  - 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:

For whatever reason, I do not see the same build warnings you report.

Dropping "mm/early_ioremap: add prototype for early_memremap_pgprot_adjust"
is one option. Alternatively, this should also work and it's a more
sensible dependency.

diff --git a/include/asm-generic/early_ioremap.h b/include/asm-generic/early_ioremap.h
index 022f8f908b42..d95c693de640 100644
--- a/include/asm-generic/early_ioremap.h
+++ b/include/asm-generic/early_ioremap.h
@@ -3,7 +3,7 @@
 #define _ASM_EARLY_IOREMAP_H_
 
 #include <linux/types.h>
-#include <asm/fixmap.h>
+#include <linux/pgtable.h>
 
 /*
  * early_ioremap() and early_iounmap() are for temporary early boot-time

-- 
Mel Gorman
SUSE Labs

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] x86: fixmap: use CONFIG_NR_CPUS instead of NR_CPUS
  2021-05-27 13:53     ` Mel Gorman
@ 2021-05-27 15:48       ` Randy Dunlap
  2021-05-27 16:55         ` Mel Gorman
  0 siblings, 1 reply; 6+ messages in thread
From: Randy Dunlap @ 2021-05-27 15:48 UTC (permalink / raw)
  To: Mel Gorman
  Cc: Ingo Molnar, Andrew Morton, linux-kernel, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, x86, Tom Rix

[-- Attachment #1: Type: text/plain, Size: 3384 bytes --]

On 5/27/21 6:53 AM, Mel Gorman wrote:
> On Wed, May 26, 2021 at 10:59:51AM -0700, Randy Dunlap wrote:
>>>  - 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:
> 
> For whatever reason, I do not see the same build warnings you report.

Not warnings, build errors. The attached config file should do it for you.
I am testing on today's linux-next.

> Dropping "mm/early_ioremap: add prototype for early_memremap_pgprot_adjust"
> is one option. Alternatively, this should also work and it's a more
> sensible dependency.
> 
> diff --git a/include/asm-generic/early_ioremap.h b/include/asm-generic/early_ioremap.h
> index 022f8f908b42..d95c693de640 100644
> --- a/include/asm-generic/early_ioremap.h
> +++ b/include/asm-generic/early_ioremap.h
> @@ -3,7 +3,7 @@
>  #define _ASM_EARLY_IOREMAP_H_
>  
>  #include <linux/types.h>
> -#include <asm/fixmap.h>
> +#include <linux/pgtable.h>
>  
>  /*
>   * early_ioremap() and early_iounmap() are for temporary early boot-time
> 

That patch also lots of (different) problems:

../arch/x86/include/asm/pgtable_types.h:58:43: warning: left shift count >= width of type [-Wshift-count-overflow]
../arch/x86/include/asm/pgtable_types.h:69:26: note: in expansion of macro ‘_PAGE_PKEY_BIT0’
../arch/x86/include/asm/pgtable.h:1393:22: note: in expansion of macro ‘_PAGE_PKEY_MASK’
../arch/x86/include/asm/pgtable_types.h:59:43: warning: left shift count >= width of type [-Wshift-count-overflow]
../arch/x86/include/asm/pgtable_types.h:70:5: note: in expansion of macro ‘_PAGE_PKEY_BIT1’
../arch/x86/include/asm/pgtable.h:1393:22: note: in expansion of macro ‘_PAGE_PKEY_MASK’
../arch/x86/include/asm/pgtable_types.h:60:43: warning: left shift count >= width of type [-Wshift-count-overflow]
../arch/x86/include/asm/pgtable_types.h:71:5: note: in expansion of macro ‘_PAGE_PKEY_BIT2’
../arch/x86/include/asm/pgtable.h:1393:22: note: in expansion of macro ‘_PAGE_PKEY_MASK’
../arch/x86/include/asm/pgtable_types.h:61:43: warning: left shift count >= width of type [-Wshift-count-overflow]
../arch/x86/include/asm/pgtable_types.h:72:5: note: in expansion of macro ‘_PAGE_PKEY_BIT3’
../arch/x86/include/asm/pgtable.h:1393:22: note: in expansion of macro ‘_PAGE_PKEY_MASK’
../arch/x86/include/asm/pgtable.h:1393:39: warning: right shift count >= width of type [-Wshift-count-overflow]
../include/linux/pgtable.h:1544:2: error: #error Missing MAX_POSSIBLE_PHYSMEM_BITS definition
make[4]: *** [../scripts/Makefile.build:271: arch/x86/entry/vdso/vdso32/vclock_gettime.o] Error 1
make[3]: *** [../scripts/Makefile.build:532: arch/x86/entry/vdso] Error 2
make[2]: *** [../scripts/Makefile.build:532: arch/x86/entry] Error 2
make[1]: *** [/home/rdunlap/lnx/next/linux-next-20210527/Makefile:1948: arch/x86] Error 2
make: *** [Makefile:222: __sub-make] Error 2



-- 
~Randy


[-- Attachment #2: config-r9622.gz --]
[-- Type: application/gzip, Size: 39741 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] x86: fixmap: use CONFIG_NR_CPUS instead of NR_CPUS
  2021-05-27 15:48       ` Randy Dunlap
@ 2021-05-27 16:55         ` Mel Gorman
  0 siblings, 0 replies; 6+ messages in thread
From: Mel Gorman @ 2021-05-27 16:55 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Ingo Molnar, Andrew Morton, linux-kernel, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, x86, Tom Rix

On Thu, May 27, 2021 at 08:48:02AM -0700, Randy Dunlap wrote:
> > Dropping "mm/early_ioremap: add prototype for early_memremap_pgprot_adjust"
> > is one option. Alternatively, this should also work and it's a more
> > sensible dependency.
> > 
> > diff --git a/include/asm-generic/early_ioremap.h b/include/asm-generic/early_ioremap.h
> > index 022f8f908b42..d95c693de640 100644
> > --- a/include/asm-generic/early_ioremap.h
> > +++ b/include/asm-generic/early_ioremap.h
> > @@ -3,7 +3,7 @@
> >  #define _ASM_EARLY_IOREMAP_H_
> >  
> >  #include <linux/types.h>
> > -#include <asm/fixmap.h>
> > +#include <linux/pgtable.h>
> >  
> >  /*
> >   * early_ioremap() and early_iounmap() are for temporary early boot-time
> > 
> 
> That patch also lots of (different) problems:
> 

Ok, thanks. Turns out the header dependencies are many and it ends
either including a bunch of headers or else just mm_types.h. That's
a lot to import just to get a working definition of pgprot_t and some
other configuration could still be missed leaving linux-next broken for
same people.

Andrew, can you drop
mm-early_ioremap-add-prototype-for-early_memremap_pgprot_adjust-fix.patch
from mmotm please? It's caused more hassle than it's worth.

-- 
Mel Gorman
SUSE Labs

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-05-27 16:55 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-21 19:59 [PATCH] x86: fixmap: use CONFIG_NR_CPUS instead of NR_CPUS Randy Dunlap
2021-05-26  6:50 ` Ingo Molnar
2021-05-26 17:59   ` Randy Dunlap
2021-05-27 13:53     ` Mel Gorman
2021-05-27 15:48       ` Randy Dunlap
2021-05-27 16:55         ` Mel Gorman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).