[2.6 patch] i386/x86_64 mpparse.c: kill maxcpus

[2.6 patch] i386/x86_64 mpparse.c: kill maxcpus

[Adrian Bunk]

Do we really need a global variable that does only hold the value of 
NR_CPUS?

Signed-off-by: Adrian Bunk <bunk@stusta.de>

---

 arch/i386/kernel/mpparse.c     |    7 +++----
 arch/i386/mach-visws/mpparse.c |    6 ++----
 arch/x86_64/kernel/mpparse.c   |    7 +++----
 3 files changed, 8 insertions(+), 12 deletions(-)

--- linux-2.6.11-mm4-full/arch/i386/kernel/mpparse.c.old	2005-03-20 19:54:21.000000000 +0100
+++ linux-2.6.11-mm4-full/arch/i386/kernel/mpparse.c	2005-03-20 19:56:17.000000000 +0100
@@ -37,7 +37,6 @@
 
 /* Have we found an MP table */
 int smp_found_config;
-unsigned int __initdata maxcpus = NR_CPUS;
 
 /*
  * Various Linux-internal data structures created from the
@@ -189,9 +188,9 @@
 		return;
 	}
 
-	if (num_processors >= maxcpus) {
-		printk(KERN_WARNING "WARNING: maxcpus limit of %i reached."
-			" Processor ignored.\n", maxcpus); 
+	if (num_processors >= NR_CPUS) {
+		printk(KERN_WARNING "WARNING: NR_CPUS limit of %i reached."
+			" Processor ignored.\n", NR_CPUS); 
 		return;
 	}
 	num_processors++;
--- linux-2.6.11-mm4-full/arch/i386/mach-visws/mpparse.c.old	2005-03-20 19:56:31.000000000 +0100
+++ linux-2.6.11-mm4-full/arch/i386/mach-visws/mpparse.c	2005-03-20 19:57:03.000000000 +0100
@@ -28,8 +28,6 @@
 /* Bitmask of physically existing CPUs */
 physid_mask_t phys_cpu_present_map;
 
-unsigned int __initdata maxcpus = NR_CPUS;
-
 /*
  * The Visual Workstation is Intel MP compliant in the hardware
  * sense, but it doesn't have a BIOS(-configuration table).
@@ -90,8 +88,8 @@
 		ncpus = CO_CPU_MAX;
 	}
 
-	if (ncpus > maxcpus)
-		ncpus = maxcpus;
+	if (ncpus > NR_CPUS)
+		ncpus = NR_CPUS;
 
 	smp_found_config = 1;
 	while (ncpus--)
--- linux-2.6.11-mm4-full/arch/x86_64/kernel/mpparse.c.old	2005-03-20 19:58:45.000000000 +0100
+++ linux-2.6.11-mm4-full/arch/x86_64/kernel/mpparse.c	2005-03-20 19:59:04.000000000 +0100
@@ -33,7 +33,6 @@
 
 /* Have we found an MP table */
 int smp_found_config;
-unsigned int __initdata maxcpus = NR_CPUS;
 
 int acpi_found_madt;
 
@@ -126,9 +125,9 @@
 			" Processor ignored.\n", NR_CPUS);
 		return;
 	}
-	if (num_processors >= maxcpus) {
-		printk(KERN_WARNING "WARNING: maxcpus limit of %i reached."
-			" Processor ignored.\n", maxcpus);
+	if (num_processors >= NR_CPUS) {
+		printk(KERN_WARNING "WARNING: NR_CPUS limit of %i reached."
+			" Processor ignored.\n", NR_CPUS);
 		return;
 	}
 

Re: [2.6 patch] i386/x86_64 mpparse.c: kill maxcpus

[Dave Jones]

On Sun, Mar 20, 2005 at 08:25:49PM +0100, Adrian Bunk wrote:
 > Do we really need a global variable that does only hold the value of 
 > NR_CPUS?

Yes.
 
NR_CPUS = compile time
maxcpus = boot command line at runtime.

		Dave

Re: [2.6 patch] i386/x86_64 mpparse.c: kill maxcpus

[Adrian Bunk]

On Sun, Mar 20, 2005 at 05:42:34PM -0500, Dave Jones wrote:
> On Sun, Mar 20, 2005 at 08:25:49PM +0100, Adrian Bunk wrote:
>  > Do we really need a global variable that does only hold the value of 
>  > NR_CPUS?
> 
> Yes.
>  
> NR_CPUS = compile time
> maxcpus = boot command line at runtime.

If this is how is was expected to work - it isn't exactly what is 
currently implemented.

The function maxcpus in init/main.c sets a static variable max_cpus -
not the global variable maxcpus in the mpparse.c files.

How should it be?

Should max_cpus in init/main.c become global and replace the maxcpus 
from the mpparse.c files?

> 		Dave

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [2.6 patch] i386/x86_64 mpparse.c: kill maxcpus

[Dave Jones]

On Mon, Mar 21, 2005 at 12:12:32AM +0100, Adrian Bunk wrote:
 > On Sun, Mar 20, 2005 at 05:42:34PM -0500, Dave Jones wrote:
 > > On Sun, Mar 20, 2005 at 08:25:49PM +0100, Adrian Bunk wrote:
 > >  > Do we really need a global variable that does only hold the value of 
 > >  > NR_CPUS?
 > > 
 > > Yes.
 > >  
 > > NR_CPUS = compile time
 > > maxcpus = boot command line at runtime.
 > 
 > If this is how is was expected to work - it isn't exactly what is 
 > currently implemented.

It's ugly, as its setting the same thing in two different places, but
I don't see any obvious reason why it won't work.

 > The function maxcpus in init/main.c sets a static variable max_cpus -
 > not the global variable maxcpus in the mpparse.c files.

Both variables are used differently. The arch specific var is only
used for HT descrimination. The init/main.c one is used for smp bringup.

 > How should it be?
 > 
 > Should max_cpus in init/main.c become global and replace the maxcpus 
 > from the mpparse.c files?

I'd just leave it as it is, as it seems to be working right now.

		Dave

Re: [2.6 patch] i386/x86_64 mpparse.c: kill maxcpus

[Adrian Bunk]

On Sun, Mar 20, 2005 at 06:32:03PM -0500, Dave Jones wrote:
> On Mon, Mar 21, 2005 at 12:12:32AM +0100, Adrian Bunk wrote:
>  > On Sun, Mar 20, 2005 at 05:42:34PM -0500, Dave Jones wrote:
>  > > On Sun, Mar 20, 2005 at 08:25:49PM +0100, Adrian Bunk wrote:
>  > >  > Do we really need a global variable that does only hold the value of 
>  > >  > NR_CPUS?
>  > > 
>  > > Yes.
>  > >  
>  > > NR_CPUS = compile time
>  > > maxcpus = boot command line at runtime.
>  > 
>  > If this is how is was expected to work - it isn't exactly what is 
>  > currently implemented.
> 
> It's ugly, as its setting the same thing in two different places, but
> I don't see any obvious reason why it won't work.
>...

I might be too dumb, but where are the mpparse.c maxcpus variables ever 
set to any value different from NR_CPUS?

> 		Dave

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [2.6 patch] i386/x86_64 mpparse.c: kill maxcpus

[Dave Jones]

On Mon, Mar 21, 2005 at 12:59:46AM +0100, Adrian Bunk wrote:
 > On Sun, Mar 20, 2005 at 06:32:03PM -0500, Dave Jones wrote:
 > > On Mon, Mar 21, 2005 at 12:12:32AM +0100, Adrian Bunk wrote:
 > >  > On Sun, Mar 20, 2005 at 05:42:34PM -0500, Dave Jones wrote:
 > >  > > On Sun, Mar 20, 2005 at 08:25:49PM +0100, Adrian Bunk wrote:
 > >  > >  > Do we really need a global variable that does only hold the value of 
 > >  > >  > NR_CPUS?
 > >  > > 
 > >  > > Yes.
 > >  > >  
 > >  > > NR_CPUS = compile time
 > >  > > maxcpus = boot command line at runtime.
 > >  > 
 > >  > If this is how is was expected to work - it isn't exactly what is 
 > >  > currently implemented.
 > > 
 > > It's ugly, as its setting the same thing in two different places, but
 > > I don't see any obvious reason why it won't work.
 > >...
 > 
 > I might be too dumb, but where are the mpparse.c maxcpus variables ever 
 > set to any value different from NR_CPUS?

arch/x86_64/kernel/setup.c:parse_cmdline_early()

	...
        else if (!memcmp(from, "maxcpus=", 8)) {
            extern unsigned int maxcpus;

            maxcpus = simple_strtoul(from + 8, NULL, 0);
        }
	...


		Dave

Re: [2.6 patch] i386/x86_64 mpparse.c: kill maxcpus

[Adrian Bunk]

On Sun, Mar 20, 2005 at 07:01:49PM -0500, Dave Jones wrote:
> On Mon, Mar 21, 2005 at 12:59:46AM +0100, Adrian Bunk wrote:
>  > On Sun, Mar 20, 2005 at 06:32:03PM -0500, Dave Jones wrote:
>  > > On Mon, Mar 21, 2005 at 12:12:32AM +0100, Adrian Bunk wrote:
>  > >  > On Sun, Mar 20, 2005 at 05:42:34PM -0500, Dave Jones wrote:
>  > >  > > On Sun, Mar 20, 2005 at 08:25:49PM +0100, Adrian Bunk wrote:
>  > >  > >  > Do we really need a global variable that does only hold the value of 
>  > >  > >  > NR_CPUS?
>  > >  > > 
>  > >  > > Yes.
>  > >  > >  
>  > >  > > NR_CPUS = compile time
>  > >  > > maxcpus = boot command line at runtime.
>  > >  > 
>  > >  > If this is how is was expected to work - it isn't exactly what is 
>  > >  > currently implemented.
>  > > 
>  > > It's ugly, as its setting the same thing in two different places, but
>  > > I don't see any obvious reason why it won't work.
>  > >...
>  > 
>  > I might be too dumb, but where are the mpparse.c maxcpus variables ever 
>  > set to any value different from NR_CPUS?
> 
> arch/x86_64/kernel/setup.c:parse_cmdline_early()
> 
> 	...
>         else if (!memcmp(from, "maxcpus=", 8)) {
>             extern unsigned int maxcpus;
> 
>             maxcpus = simple_strtoul(from + 8, NULL, 0);
>         }
> 	...

Ah, thanks.

I didn't see this extern declaration.
(and my test compiles were with SMP=n...)

> 		Dave

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed