[patch] Fix large MCA bootmem allocation

[patch] Fix large MCA bootmem allocation

[Russ Anderson]

[patch] Fix large MCA bootmem allocation

The MCA code allocates bootmem memory for NR_CPUS, regardless
of how many cpus the system actually has.  This change allocates
memory only for cpus that actually exist.

On my test system with NR_CPUS = 1024, reserved memory was reduced by 130944k.

Before: Memory: 27886976k/28111168k available (8282k code, 242304k reserved, 5928k data, 1792k init)
After:  Memory: 28017920k/28111168k available (8282k code, 111360k reserved, 5928k data, 1792k init)


Signed-off-by: Russ Anderson <rja@sgi.com>

---
 arch/ia64/kernel/mca.c |   52 +++++++++++++++++++++----------------------------
 1 file changed, 23 insertions(+), 29 deletions(-)

Index: test/arch/ia64/kernel/mca.c
=================================--- test.orig/arch/ia64/kernel/mca.c	2008-02-05 13:01:56.000000000 -0600
+++ test/arch/ia64/kernel/mca.c	2008-02-05 13:12:34.921332805 -0600
@@ -17,7 +17,7 @@
  * Copyright (C) 2000 Intel
  * Copyright (C) Chuck Fleckenstein <cfleck@co.intel.com>
  *
- * Copyright (C) 1999, 2004 Silicon Graphics, Inc.
+ * Copyright (C) 1999, 2004-2008 Silicon Graphics, Inc.
  * Copyright (C) Vijay Chander <vijay@engr.sgi.com>
  *
  * Copyright (C) 2006 FUJITSU LIMITED
@@ -1762,11 +1762,8 @@ format_mca_init_stack(void *mca_data, un
 /* Caller prevents this from being called after init */
 static void * __init_refok mca_bootmem(void)
 {
-	void *p;
-
-	p = alloc_bootmem(sizeof(struct ia64_mca_cpu) * NR_CPUS +
-	                  KERNEL_STACK_SIZE);
-	return (void *)ALIGN((unsigned long)p, KERNEL_STACK_SIZE);
+	return __alloc_bootmem(sizeof(struct ia64_mca_cpu),
+	                    KERNEL_STACK_SIZE, 0);
 }
 
 /* Do per-CPU MCA-related initialization.  */
@@ -1774,33 +1771,30 @@ void __cpuinit
 ia64_mca_cpu_init(void *cpu_data)
 {
 	void *pal_vaddr;
+	void *data;
+	long sz = sizeof(struct ia64_mca_cpu);
+	int cpu = smp_processor_id();
 	static int first_time = 1;
 
-	if (first_time) {
-		void *mca_data;
-		int cpu;
-
-		first_time = 0;
-		mca_data = mca_bootmem();
-		for (cpu = 0; cpu < NR_CPUS; cpu++) {
-			format_mca_init_stack(mca_data,
-					offsetof(struct ia64_mca_cpu, mca_stack),
-					"MCA", cpu);
-			format_mca_init_stack(mca_data,
-					offsetof(struct ia64_mca_cpu, init_stack),
-					"INIT", cpu);
-			__per_cpu_mca[cpu] = __pa(mca_data);
-			mca_data += sizeof(struct ia64_mca_cpu);
-		}
-	}
-
 	/*
-	 * The MCA info structure was allocated earlier and its
-	 * physical address saved in __per_cpu_mca[cpu].  Copy that
-	 * address * to ia64_mca_data so we can access it as a per-CPU
-	 * variable.
+	 * Structure will already be allocated if cpu has been online,
+	 * then offlined.
 	 */
-	__get_cpu_var(ia64_mca_data) = __per_cpu_mca[smp_processor_id()];
+	if (__per_cpu_mca[cpu]) {
+		data = __va(__per_cpu_mca[cpu]);
+	} else {
+		if (first_time) {
+			data = mca_bootmem();
+			first_time = 0;
+		} else
+			data = page_address(alloc_pages_node(numa_node_id(),
+					GFP_KERNEL, get_order(sz)));
+	}
+	format_mca_init_stack(data, offsetof(struct ia64_mca_cpu, mca_stack),
+		"MCA", cpu);
+	format_mca_init_stack(data, offsetof(struct ia64_mca_cpu, init_stack),
+		"INIT", cpu);
+	__get_cpu_var(ia64_mca_data) = __per_cpu_mca[cpu] = __pa(data);
 
 	/*
 	 * Stash away a copy of the PTE needed to map the per-CPU page.
-- 
Russ Anderson, OS RAS/Partitioning Project Lead  
SGI - Silicon Graphics Inc          rja@sgi.com

Re: [patch] Fix large MCA bootmem allocation

[Bjorn Helgaas]

On Tuesday 05 February 2008 12:32:33 pm Russ Anderson wrote:
> +		if (first_time) {
> +			data = mca_bootmem();
> +			first_time = 0;
> +		} else
> +			data = page_address(alloc_pages_node(numa_node_id(),
> +					GFP_KERNEL, get_order(sz)));
> +	}

I assume this alloc_pages_node() path happens when a CPU is hot-added.
What happens if this alloc fails?

Bjorn

Re: [patch] Fix large MCA bootmem allocation

[Russ Anderson]

On Tue, Feb 05, 2008 at 01:21:00PM -0700, Bjorn Helgaas wrote:
> On Tuesday 05 February 2008 12:32:33 pm Russ Anderson wrote:
> > +		if (first_time) {
> > +			data = mca_bootmem();
> > +			first_time = 0;
> > +		} else
> > +			data = page_address(alloc_pages_node(numa_node_id(),
> > +					GFP_KERNEL, get_order(sz)));
> > +	}
> 
> I assume this alloc_pages_node() path happens when a CPU is hot-added.

I had assumed that, too, but it does not appear to be the case.
I have not followed the hot-plug enabled code path, but it does 
not call ia64_mca_cpu_init(). 

FWIW, the MCA code behaves correctly when cpus are logically offlined
(ie only the enabled cpus are rendezvoused) and onlined.  The 
disable path does not free the memory, so it is still there when the
cpu is re-enabled.

> What happens if this alloc fails?

The updated patch will panic is alloc fails.

----------------------------------------------------------------
[patch] Fix large MCA bootmem allocation

The MCA code allocates bootmem memory for NR_CPUS, regardless
of how many cpus the system actually has.  This change allocates
memory only for cpus that actually exist.

On my test system with NR_CPUS = 1024, reserved memory was reduced by 130944k.

Before: Memory: 27886976k/28111168k available (8282k code, 242304k reserved, 5928k data, 1792k init)
After:  Memory: 28017920k/28111168k available (8282k code, 111360k reserved, 5928k data, 1792k init)


Signed-off-by: Russ Anderson <rja@sgi.com>

---
 arch/ia64/kernel/mca.c |   55 +++++++++++++++++++++++--------------------------
 1 file changed, 26 insertions(+), 29 deletions(-)

Index: test/arch/ia64/kernel/mca.c
=================================--- test.orig/arch/ia64/kernel/mca.c	2008-02-05 13:01:56.000000000 -0600
+++ test/arch/ia64/kernel/mca.c	2008-02-05 16:39:18.116180868 -0600
@@ -17,7 +17,7 @@
  * Copyright (C) 2000 Intel
  * Copyright (C) Chuck Fleckenstein <cfleck@co.intel.com>
  *
- * Copyright (C) 1999, 2004 Silicon Graphics, Inc.
+ * Copyright (C) 1999, 2004-2008 Silicon Graphics, Inc.
  * Copyright (C) Vijay Chander <vijay@engr.sgi.com>
  *
  * Copyright (C) 2006 FUJITSU LIMITED
@@ -1762,11 +1762,8 @@ format_mca_init_stack(void *mca_data, un
 /* Caller prevents this from being called after init */
 static void * __init_refok mca_bootmem(void)
 {
-	void *p;
-
-	p = alloc_bootmem(sizeof(struct ia64_mca_cpu) * NR_CPUS +
-	                  KERNEL_STACK_SIZE);
-	return (void *)ALIGN((unsigned long)p, KERNEL_STACK_SIZE);
+	return __alloc_bootmem(sizeof(struct ia64_mca_cpu),
+	                    KERNEL_STACK_SIZE, 0);
 }
 
 /* Do per-CPU MCA-related initialization.  */
@@ -1774,33 +1771,33 @@ void __cpuinit
 ia64_mca_cpu_init(void *cpu_data)
 {
 	void *pal_vaddr;
+	void *data;
+	long sz = sizeof(struct ia64_mca_cpu);
+	int cpu = smp_processor_id();
 	static int first_time = 1;
 
-	if (first_time) {
-		void *mca_data;
-		int cpu;
-
-		first_time = 0;
-		mca_data = mca_bootmem();
-		for (cpu = 0; cpu < NR_CPUS; cpu++) {
-			format_mca_init_stack(mca_data,
-					offsetof(struct ia64_mca_cpu, mca_stack),
-					"MCA", cpu);
-			format_mca_init_stack(mca_data,
-					offsetof(struct ia64_mca_cpu, init_stack),
-					"INIT", cpu);
-			__per_cpu_mca[cpu] = __pa(mca_data);
-			mca_data += sizeof(struct ia64_mca_cpu);
-		}
-	}
-
 	/*
-	 * The MCA info structure was allocated earlier and its
-	 * physical address saved in __per_cpu_mca[cpu].  Copy that
-	 * address * to ia64_mca_data so we can access it as a per-CPU
-	 * variable.
+	 * Structure will already be allocated if cpu has been online,
+	 * then offlined.
 	 */
-	__get_cpu_var(ia64_mca_data) = __per_cpu_mca[smp_processor_id()];
+	if (__per_cpu_mca[cpu]) {
+		data = __va(__per_cpu_mca[cpu]);
+	} else {
+		if (first_time) {
+			data = mca_bootmem();
+			first_time = 0;
+		} else
+			data = page_address(alloc_pages_node(numa_node_id(),
+					GFP_KERNEL, get_order(sz)));
+		if (!data)
+			panic("Could not allocate MCA memory for cpu %d\n",
+					cpu);
+	}
+	format_mca_init_stack(data, offsetof(struct ia64_mca_cpu, mca_stack),
+		"MCA", cpu);
+	format_mca_init_stack(data, offsetof(struct ia64_mca_cpu, init_stack),
+		"INIT", cpu);
+	__get_cpu_var(ia64_mca_data) = __per_cpu_mca[cpu] = __pa(data);
 
 	/*
 	 * Stash away a copy of the PTE needed to map the per-CPU page.

-- 
Russ Anderson, OS RAS/Partitioning Project Lead  
SGI - Silicon Graphics Inc          rja@sgi.com

Re: [patch] Fix large MCA bootmem allocation

[Bjorn Helgaas]

On Tuesday 05 February 2008 04:12:32 pm Russ Anderson wrote:
> On Tue, Feb 05, 2008 at 01:21:00PM -0700, Bjorn Helgaas wrote:
> > On Tuesday 05 February 2008 12:32:33 pm Russ Anderson wrote:
> > > +		if (first_time) {
> > > +			data = mca_bootmem();
> > > +			first_time = 0;
> > > +		} else
> > > +			data = page_address(alloc_pages_node(numa_node_id(),
> > > +					GFP_KERNEL, get_order(sz)));
> > > +	}
> > 
> > I assume this alloc_pages_node() path happens when a CPU is hot-added.
> 
> I had assumed that, too, but it does not appear to be the case.
> I have not followed the hot-plug enabled code path, but it does 
> not call ia64_mca_cpu_init(). 
> 
> FWIW, the MCA code behaves correctly when cpus are logically offlined
> (ie only the enabled cpus are rendezvoused) and onlined.  The 
> disable path does not free the memory, so it is still there when the
> cpu is re-enabled.
> 
> > What happens if this alloc fails?
> 
> The updated patch will panic is alloc fails.

I don't know this code well, so I apologize for asking basic
questions.

Is panic the only choice here?  It seems unfriendly to panic
just because we can't successfully add a new CPU.  It'd be
nicer to somehow make the addition fail so the new CPU is
just not usable.

> ----------------------------------------------------------------
> [patch] Fix large MCA bootmem allocation
> 
> The MCA code allocates bootmem memory for NR_CPUS, regardless
> of how many cpus the system actually has.  This change allocates
> memory only for cpus that actually exist.
> 
> On my test system with NR_CPUS = 1024, reserved memory was reduced by 130944k.
> 
> Before: Memory: 27886976k/28111168k available (8282k code, 242304k reserved, 5928k data, 1792k init)
> After:  Memory: 28017920k/28111168k available (8282k code, 111360k reserved, 5928k data, 1792k init)
> 
> 
> Signed-off-by: Russ Anderson <rja@sgi.com>
> 
> ---
>  arch/ia64/kernel/mca.c |   55 +++++++++++++++++++++++--------------------------
>  1 file changed, 26 insertions(+), 29 deletions(-)
> 
> Index: test/arch/ia64/kernel/mca.c
> =================================> --- test.orig/arch/ia64/kernel/mca.c	2008-02-05 13:01:56.000000000 -0600
> +++ test/arch/ia64/kernel/mca.c	2008-02-05 16:39:18.116180868 -0600
> @@ -17,7 +17,7 @@
>   * Copyright (C) 2000 Intel
>   * Copyright (C) Chuck Fleckenstein <cfleck@co.intel.com>
>   *
> - * Copyright (C) 1999, 2004 Silicon Graphics, Inc.
> + * Copyright (C) 1999, 2004-2008 Silicon Graphics, Inc.
>   * Copyright (C) Vijay Chander <vijay@engr.sgi.com>
>   *
>   * Copyright (C) 2006 FUJITSU LIMITED
> @@ -1762,11 +1762,8 @@ format_mca_init_stack(void *mca_data, un
>  /* Caller prevents this from being called after init */
>  static void * __init_refok mca_bootmem(void)
>  {
> -	void *p;
> -
> -	p = alloc_bootmem(sizeof(struct ia64_mca_cpu) * NR_CPUS +
> -	                  KERNEL_STACK_SIZE);
> -	return (void *)ALIGN((unsigned long)p, KERNEL_STACK_SIZE);
> +	return __alloc_bootmem(sizeof(struct ia64_mca_cpu),
> +	                    KERNEL_STACK_SIZE, 0);
>  }
>  
>  /* Do per-CPU MCA-related initialization.  */
> @@ -1774,33 +1771,33 @@ void __cpuinit
>  ia64_mca_cpu_init(void *cpu_data)
>  {
>  	void *pal_vaddr;
> +	void *data;
> +	long sz = sizeof(struct ia64_mca_cpu);
> +	int cpu = smp_processor_id();
>  	static int first_time = 1;
>  
> -	if (first_time) {
> -		void *mca_data;
> -		int cpu;
> -
> -		first_time = 0;
> -		mca_data = mca_bootmem();
> -		for (cpu = 0; cpu < NR_CPUS; cpu++) {
> -			format_mca_init_stack(mca_data,
> -					offsetof(struct ia64_mca_cpu, mca_stack),
> -					"MCA", cpu);
> -			format_mca_init_stack(mca_data,
> -					offsetof(struct ia64_mca_cpu, init_stack),
> -					"INIT", cpu);
> -			__per_cpu_mca[cpu] = __pa(mca_data);
> -			mca_data += sizeof(struct ia64_mca_cpu);
> -		}
> -	}
> -
>  	/*
> -	 * The MCA info structure was allocated earlier and its
> -	 * physical address saved in __per_cpu_mca[cpu].  Copy that
> -	 * address * to ia64_mca_data so we can access it as a per-CPU
> -	 * variable.
> +	 * Structure will already be allocated if cpu has been online,
> +	 * then offlined.
>  	 */
> -	__get_cpu_var(ia64_mca_data) = __per_cpu_mca[smp_processor_id()];
> +	if (__per_cpu_mca[cpu]) {
> +		data = __va(__per_cpu_mca[cpu]);
> +	} else {
> +		if (first_time) {
> +			data = mca_bootmem();
> +			first_time = 0;
> +		} else
> +			data = page_address(alloc_pages_node(numa_node_id(),
> +					GFP_KERNEL, get_order(sz)));
> +		if (!data)
> +			panic("Could not allocate MCA memory for cpu %d\n",
> +					cpu);
> +	}
> +	format_mca_init_stack(data, offsetof(struct ia64_mca_cpu, mca_stack),
> +		"MCA", cpu);
> +	format_mca_init_stack(data, offsetof(struct ia64_mca_cpu, init_stack),
> +		"INIT", cpu);
> +	__get_cpu_var(ia64_mca_data) = __per_cpu_mca[cpu] = __pa(data);
>  
>  	/*
>  	 * Stash away a copy of the PTE needed to map the per-CPU page.
> 

RE: [patch] Fix large MCA bootmem allocation

[Luck, Tony]

> Is panic the only choice here?  It seems unfriendly to panic
> just because we can't successfully add a new CPU.  It'd be
> nicer to somehow make the addition fail so the new CPU is
> just not usable.

The current code sequence is:
	start_secondary -> cpu_init -> ia64_mca_cpu_init

and the memory allocation here happens very late in the
sequence ... so it it fails we'd have to do a lot of work
to back out.  Moving the ia64_mca_cpu_init() much earlier
would help some ... but we don't currently have a graceful
failure path if anything goes wrong.  The new cpu has
already been passed from SAL control to OS control ... so
we'd have to pass it back again.

There are probably a whole host of potential failures in
this code path that ought to be checked but currently
aren't because all these routines were written to be
called once at boot time when memory allocations hardly
ever fail (and it they do, panic is perhaps the best
answer).

The liklihood of this particular allocation failing goes
up as the configured pagesize is reduced.  With the
default 64k, the allocation is order=1 ... but with a
4k page we need an order=4 allocation to succeed.

-Tony

Re: [patch] Fix large MCA bootmem allocation

[Russ Anderson]

On Tue, Feb 05, 2008 at 04:24:33PM -0700, Bjorn Helgaas wrote:
> On Tuesday 05 February 2008 04:12:32 pm Russ Anderson wrote:
> > On Tue, Feb 05, 2008 at 01:21:00PM -0700, Bjorn Helgaas wrote:
> > 
> > > What happens if this alloc fails?
> > 
> > The updated patch will panic is alloc fails.
> 
> I don't know this code well, so I apologize for asking basic
> questions.
> 
> Is panic the only choice here?  It seems unfriendly to panic
> just because we can't successfully add a new CPU.  It'd be
> nicer to somehow make the addition fail so the new CPU is
> just not usable.

It wasn't my first choice, but ia64_mca_cpu_init() is void
so it's not as simple as returning a failing status.  That
code was written as part of early boot and if you can't 
get through boot without running out of memory you have
bigger problems.  For example, __alloc_bootmem() calls panic
if it cannot allocate memory.  There is no easy unwind path
out of that code.

Given that ia64_mca_cpu_init() does not get called in
the hot-plug path, the scenario of hot adding a cpu hitting
this panic is not possible.  If/When the hot-plug code starts
calling ia64_mca_cpu_init(), it will be an issue.  

Thanks,
-- 
Russ Anderson, OS RAS/Partitioning Project Lead  
SGI - Silicon Graphics Inc          rja@sgi.com

RE: [patch] Fix large MCA bootmem allocation

[Luck, Tony]

> Given that ia64_mca_cpu_init() does not get called in
> the hot-plug path

Surely it must be ... you even have code in it to cope
with this case.  The:

	if (__per_cpu_mca[cpu]) {

test can only be true if we are back in this code for a
second run through if the cpu was taken offline and added
again. Just like it says in the comment above this test.

-Tony

Re: [patch] Fix large MCA bootmem allocation

[Hidetoshi Seto]

Russ Anderson wrote:
> The MCA code allocates bootmem memory for NR_CPUS, regardless
> of how many cpus the system actually has.

So... how about using num_possible_cpus() instead?

> @@ -1762,11 +1762,8 @@ format_mca_init_stack(void *mca_data, un
>  /* Caller prevents this from being called after init */
>  static void * __init_refok mca_bootmem(void)
>  {
> -	void *p;
> -
> -	p = alloc_bootmem(sizeof(struct ia64_mca_cpu) * NR_CPUS +
> -	                  KERNEL_STACK_SIZE);
> -	return (void *)ALIGN((unsigned long)p, KERNEL_STACK_SIZE);
> +	return __alloc_bootmem(sizeof(struct ia64_mca_cpu),
> +	                    KERNEL_STACK_SIZE, 0);
>  }
>  
>  /* Do per-CPU MCA-related initialization.  */

i.e.
p = alloc_bootmem(sizeof(struct ia64_mca_cpu) * num_possible_cpus() +
	                  KERNEL_STACK_SIZE);

Thanks,
H.Seto

Re: [patch] Fix large MCA bootmem allocation

[Robin Holt]

On Wed, Feb 06, 2008 at 09:30:17AM +0900, Hidetoshi Seto wrote:
> Russ Anderson wrote:
>> The MCA code allocates bootmem memory for NR_CPUS, regardless
>> of how many cpus the system actually has.
>
> So... how about using num_possible_cpus() instead?

num_possible_cpus() is not initialized until after this code is run for
the first time.

Thanks,
Robin

Re: [patch] Fix large MCA bootmem allocation

[Hidetoshi Seto]

Robin Holt wrote:
> On Wed, Feb 06, 2008 at 09:30:17AM +0900, Hidetoshi Seto wrote:
>> Russ Anderson wrote:
>>> The MCA code allocates bootmem memory for NR_CPUS, regardless
>>> of how many cpus the system actually has.
>> So... how about using num_possible_cpus() instead?
> 
> num_possible_cpus() is not initialized until after this code is run for
> the first time.

I grant you.
Then, I suppose we need a CPU_UP_PREPARE callback to allocate memory
before hot-added CPU enters cpu_init().

Thanks,
H.Seto

Re: [patch] Fix large MCA bootmem allocation

[Hidetoshi Seto]

Hidetoshi Seto wrote:
> Then, I suppose we need a CPU_UP_PREPARE callback to allocate memory
> before hot-added CPU enters cpu_init().

Ah, we cannot allocate bootmem later, so it would be better to make
the callback to return NOTIFY_BAD if (!__per_cpu_mca[hcpu]).

Thanks,
H.Seto

Re: [patch] Fix large MCA bootmem allocation

[Robin Holt]

On Wed, Feb 06, 2008 at 12:50:11PM +0900, Hidetoshi Seto wrote:
> Hidetoshi Seto wrote:
>> Then, I suppose we need a CPU_UP_PREPARE callback to allocate memory
>> before hot-added CPU enters cpu_init().
>
> Ah, we cannot allocate bootmem later, so it would be better to make
> the callback to return NOTIFY_BAD if (!__per_cpu_mca[hcpu]).
>

bootmem is only used for the first cpu.  The other cpus use regular
allocations.  So we should be able to do a CPU_UP_PREPARE.

Comparing the severity of allocating a ton of extra memory with the
chance that we fail on an allocation and given that there are likely
other things in the cpu_init path which are not checked, do you see the
lack of check as a show-stopper for this patch or would you accept this
patch with the plan being fixup the cpu_init path later?

Thanks,
Robin

Re: [patch] Fix large MCA bootmem allocation

[Hidetoshi Seto]

Robin Holt wrote:
> On Wed, Feb 06, 2008 at 12:50:11PM +0900, Hidetoshi Seto wrote:
>> Hidetoshi Seto wrote:
>>> Then, I suppose we need a CPU_UP_PREPARE callback to allocate memory
>>> before hot-added CPU enters cpu_init().
>> Ah, we cannot allocate bootmem later, so it would be better to make
>> the callback to return NOTIFY_BAD if (!__per_cpu_mca[hcpu]).
> 
> bootmem is only used for the first cpu.  The other cpus use regular
> allocations.  So we should be able to do a CPU_UP_PREPARE.

OK, it's my suggestion.

> Comparing the severity of allocating a ton of extra memory with the
> chance that we fail on an allocation and given that there are likely
> other things in the cpu_init path which are not checked, do you see the
> lack of check as a show-stopper for this patch or would you accept this
> patch with the plan being fixup the cpu_init path later?

Which other stuff in the cpu_init path would cause a panic on hot-added CPU?
The per-cpu pages are ready for NR_CPUS. ia64_setreg(), ia64_set_kr() and
kin would be work properly...

I'm afraid that for hot-added CPU there might be nothing dangerous than
this allocation in the path.
That's why I want to hire a CPU_UP_PREPARE at least for this allocation.

Thanks,
H.Seto

Re: [patch] Fix large MCA bootmem allocation

[Robin Holt]

On Wed, Feb 06, 2008 at 09:39:42PM +0900, Hidetoshi Seto wrote:
> Robin Holt wrote:
> Which other stuff in the cpu_init path would cause a panic on hot-added 
> CPU?

I had not looked.  I was going off what Tony Luck had said earlier.

Thanks,
Robin

Re: [patch] Fix large MCA bootmem allocation

[Russ Anderson]

On Tue, Feb 05, 2008 at 04:00:09PM -0800, Luck, Tony wrote:
> > Given that ia64_mca_cpu_init() does not get called in
> > the hot-plug path
> 
> Surely it must be ...

That's what I thought, until testing showed otherwise.

The two code paths are setup_arch->cpu_init->ia64_mca_cpu_init
for the boot processor and 
start_secondary->cpu_init->ia64_mca_cpu_init for the other cpus
at boot time.

>                        you even have code in it to cope
> with this case.  The:
> 
> 	if (__per_cpu_mca[cpu]) {
> 
> test can only be true if we are back in this code for a
> second run through if the cpu was taken offline and added
> again. Just like it says in the comment above this test.

Written on the assumption that the hot-plug code would
eventually call it. 

-- 
Russ Anderson, OS RAS/Partitioning Project Lead  
SGI - Silicon Graphics Inc          rja@sgi.com

Re: [patch] Fix large MCA bootmem allocation

[Russ Anderson]

On Wed, Feb 06, 2008 at 09:13:33AM -0600, Russ Anderson wrote:
> On Tue, Feb 05, 2008 at 04:00:09PM -0800, Luck, Tony wrote:
> > > Given that ia64_mca_cpu_init() does not get called in
> > > the hot-plug path
> > 
> > Surely it must be ...
> 
> That's what I thought, until testing showed otherwise.

My bad.  Additional testing shows ia64_mca_cpu_init() does get called.

I suspect my problem was when CONFIG_HOTPLUG_CPU was turned on,
not all the files were rebuilt.  
 
> The two code paths are setup_arch->cpu_init->ia64_mca_cpu_init
> for the boot processor and 
> start_secondary->cpu_init->ia64_mca_cpu_init for the other cpus
> at boot time.

In the hot-plug path, __cpu_up -> start_seconday .
 
> >                        you even have code in it to cope
> > with this case.  The:
> > 
> > 	if (__per_cpu_mca[cpu]) {
> > 
> > test can only be true if we are back in this code for a
> > second run through if the cpu was taken offline and added
> > again. Just like it says in the comment above this test.
> 
> Written on the assumption that the hot-plug code would
> eventually call it. 

It does.  The second time through the old memory is re-used.

Sorry about the noise...

-- 
Russ Anderson, OS RAS/Partitioning Project Lead  
SGI - Silicon Graphics Inc          rja@sgi.com