2005-03-20 19:29:40

by Adrian Bunk

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

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

Signed-off-by: Adrian Bunk <[email protected]>

---

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;
}



2005-03-20 22:46:30

by Dave Jones

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

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

2005-03-20 23:19:19

by Adrian Bunk

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

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

2005-03-20 23:33:26

by Dave Jones

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

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

2005-03-21 00:00:19

by Adrian Bunk

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

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

2005-03-21 00:03:10

by Dave Jones

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

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

2005-03-21 00:45:18

by Adrian Bunk

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

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