Re: [PATCH] kernel/cpu.c: Section mismatch warning fix.

Re: [PATCH] kernel/cpu.c: Section mismatch warning fix.

[Andrew Morton]

On Thu, 30 Oct 2008 10:04:54 +0600
"Rakib Mullick" <rakib.mullick@gmail.com> wrote:

>  LD      kernel/built-in.o
> WARNING: kernel/built-in.o(.text+0xb7c8): Section mismatch in
> reference from the function notify_cpu_starting() to the variable
> .cpuinit.data:cpu_chain
> The function notify_cpu_starting() references
> the variable __cpuinitdata cpu_chain.
> This is often because notify_cpu_starting lacks a __cpuinitdata
> annotation or the annotation of cpu_chain is wrong.
> 
> This patch fixes the above section mismatch warning. If anything else
> please notice.
> Thanks.
> 
> Signed-off-by: Md.Rakib H. Mullick <rakib.mullick@gmail.com>
> 
> --- linux-2.6-orig/kernel/cpu.c	2008-10-28 20:52:38.000000000 +0600
> +++ linux-2.6/kernel/cpu.c	2008-10-28 22:46:22.000000000 +0600
> @@ -462,7 +462,7 @@ out:
>   * It must be called by the arch code on the new cpu, before the new cpu
>   * enables interrupts and before the "boot" cpu returns from __cpu_up().
>   */
> -void notify_cpu_starting(unsigned int cpu)
> +void __cpuinit notify_cpu_starting(unsigned int cpu)
>  {
>  	unsigned long val = CPU_STARTING;

arch/alpha/kernel/smp.c calls notify_cpu_starting() from __init code.

arch/cris/arch-v32/kernel/smp.c calls notify_cpu_starting() from __init code.

arch/x86/mach-voyager/voyager_smp.c calls notify_cpu_starting() from
__init code.

arch/m32r/kernel/smpboot.c calls notify_cpu_starting() from __init code.

arch/sparc/kernel/sun4d_smp.c calls notify_cpu_starting() from __init code.

arch/powerpc/kernel/smp.c calls notify_cpu_starting() from __devinit
code.

arch/um/kernel/smp.c calls notify_cpu_starting() from .text code.


The other nine callers call notify_cpu_starting() from __cpuinit code.


What a mess.

Re: [PATCH] kernel/cpu.c: Section mismatch warning fix.

[Ingo Molnar]

* Andrew Morton <akpm@linux-foundation.org> wrote:

> On Thu, 30 Oct 2008 10:04:54 +0600
> "Rakib Mullick" <rakib.mullick@gmail.com> wrote:
> 
> >  LD      kernel/built-in.o
> > WARNING: kernel/built-in.o(.text+0xb7c8): Section mismatch in
> > reference from the function notify_cpu_starting() to the variable
> > .cpuinit.data:cpu_chain
> > The function notify_cpu_starting() references
> > the variable __cpuinitdata cpu_chain.
> > This is often because notify_cpu_starting lacks a __cpuinitdata
> > annotation or the annotation of cpu_chain is wrong.
> > 
> > This patch fixes the above section mismatch warning. If anything else
> > please notice.
> > Thanks.
> > 
> > Signed-off-by: Md.Rakib H. Mullick <rakib.mullick@gmail.com>
> > 
> > --- linux-2.6-orig/kernel/cpu.c	2008-10-28 20:52:38.000000000 +0600
> > +++ linux-2.6/kernel/cpu.c	2008-10-28 22:46:22.000000000 +0600
> > @@ -462,7 +462,7 @@ out:
> >   * It must be called by the arch code on the new cpu, before the new cpu
> >   * enables interrupts and before the "boot" cpu returns from __cpu_up().
> >   */
> > -void notify_cpu_starting(unsigned int cpu)
> > +void __cpuinit notify_cpu_starting(unsigned int cpu)
> >  {
> >  	unsigned long val = CPU_STARTING;
> 
> arch/alpha/kernel/smp.c calls notify_cpu_starting() from __init code.
> 
> arch/cris/arch-v32/kernel/smp.c calls notify_cpu_starting() from __init code.
> 
> arch/x86/mach-voyager/voyager_smp.c calls notify_cpu_starting() from
> __init code.
> 
> arch/m32r/kernel/smpboot.c calls notify_cpu_starting() from __init code.
> 
> arch/sparc/kernel/sun4d_smp.c calls notify_cpu_starting() from __init code.
> 
> arch/powerpc/kernel/smp.c calls notify_cpu_starting() from __devinit
> code.
> 
> arch/um/kernel/smp.c calls notify_cpu_starting() from .text code.
> 
> 
> The other nine callers call notify_cpu_starting() from __cpuinit code.
> 
> 
> What a mess.

__cpuinit seems safe for all but UML.

But even for UML it appears to be de-facto safe: as after bootup we 
never return back into arch/um/kernel/smp.c::idle_proc(). (as UML's 
default_idle() is an infinite loop)

Yesterday i've queued it up in tip/core/urgent:

  685aebb: kernel/cpu.c: section mismatch warning fix

to be pushed to Linus today-ish. Any objections?

	Ingo

Re: [PATCH] kernel/cpu.c: Section mismatch warning fix.

[Sam Ravnborg]

On Tue, Nov 04, 2008 at 10:48:31AM +0100, Ingo Molnar wrote:
> 
> * Andrew Morton <akpm@linux-foundation.org> wrote:
> 
> > On Thu, 30 Oct 2008 10:04:54 +0600
> > "Rakib Mullick" <rakib.mullick@gmail.com> wrote:
> > 
> > >  LD      kernel/built-in.o
> > > WARNING: kernel/built-in.o(.text+0xb7c8): Section mismatch in
> > > reference from the function notify_cpu_starting() to the variable
> > > .cpuinit.data:cpu_chain
> > > The function notify_cpu_starting() references
> > > the variable __cpuinitdata cpu_chain.
> > > This is often because notify_cpu_starting lacks a __cpuinitdata
> > > annotation or the annotation of cpu_chain is wrong.
> > > 
> > > This patch fixes the above section mismatch warning. If anything else
> > > please notice.
> > > Thanks.
> > > 
> > > Signed-off-by: Md.Rakib H. Mullick <rakib.mullick@gmail.com>
> > > 
> > > --- linux-2.6-orig/kernel/cpu.c	2008-10-28 20:52:38.000000000 +0600
> > > +++ linux-2.6/kernel/cpu.c	2008-10-28 22:46:22.000000000 +0600
> > > @@ -462,7 +462,7 @@ out:
> > >   * It must be called by the arch code on the new cpu, before the new cpu
> > >   * enables interrupts and before the "boot" cpu returns from __cpu_up().
> > >   */
> > > -void notify_cpu_starting(unsigned int cpu)
> > > +void __cpuinit notify_cpu_starting(unsigned int cpu)
> > >  {
> > >  	unsigned long val = CPU_STARTING;
> > 
> > arch/alpha/kernel/smp.c calls notify_cpu_starting() from __init code.
> > 
> > arch/cris/arch-v32/kernel/smp.c calls notify_cpu_starting() from __init code.
> > 
> > arch/x86/mach-voyager/voyager_smp.c calls notify_cpu_starting() from
> > __init code.
> > 
> > arch/m32r/kernel/smpboot.c calls notify_cpu_starting() from __init code.
> > 
> > arch/sparc/kernel/sun4d_smp.c calls notify_cpu_starting() from __init code.
> > 
> > arch/powerpc/kernel/smp.c calls notify_cpu_starting() from __devinit
> > code.
> > 
> > arch/um/kernel/smp.c calls notify_cpu_starting() from .text code.
> > 
> > 
> > The other nine callers call notify_cpu_starting() from __cpuinit code.
> > 
> > 
> > What a mess.
> 
> __cpuinit seems safe for all but UML.
> 
> But even for UML it appears to be de-facto safe: as after bootup we 
> never return back into arch/um/kernel/smp.c::idle_proc(). (as UML's 
From the list Andrew provided powerpc needs to be looked after.
We cannot call an __init function from __devinit context.
If you already checked that then no objections.

	Sam

Re: [PATCH] kernel/cpu.c: Section mismatch warning fix.

[Sam Ravnborg]

On Tue, Nov 04, 2008 at 10:48:31AM +0100, Ingo Molnar wrote:
> 
> * Andrew Morton <akpm@linux-foundation.org> wrote:
> 
> > On Thu, 30 Oct 2008 10:04:54 +0600
> > "Rakib Mullick" <rakib.mullick@gmail.com> wrote:
> > 
> > >  LD      kernel/built-in.o
> > > WARNING: kernel/built-in.o(.text+0xb7c8): Section mismatch in
> > > reference from the function notify_cpu_starting() to the variable
> > > .cpuinit.data:cpu_chain
> > > The function notify_cpu_starting() references
> > > the variable __cpuinitdata cpu_chain.
> > > This is often because notify_cpu_starting lacks a __cpuinitdata
> > > annotation or the annotation of cpu_chain is wrong.
> > > 
> > > This patch fixes the above section mismatch warning. If anything else
> > > please notice.
> > > Thanks.
> > > 
> > > Signed-off-by: Md.Rakib H. Mullick <rakib.mullick@gmail.com>
> > > 
> > > --- linux-2.6-orig/kernel/cpu.c	2008-10-28 20:52:38.000000000 +0600
> > > +++ linux-2.6/kernel/cpu.c	2008-10-28 22:46:22.000000000 +0600
> > > @@ -462,7 +462,7 @@ out:
> > >   * It must be called by the arch code on the new cpu, before the new cpu
> > >   * enables interrupts and before the "boot" cpu returns from __cpu_up().
> > >   */
> > > -void notify_cpu_starting(unsigned int cpu)
> > > +void __cpuinit notify_cpu_starting(unsigned int cpu)
> > >  {
> > >  	unsigned long val = CPU_STARTING;
> > 
> > arch/alpha/kernel/smp.c calls notify_cpu_starting() from __init code.
> > 
> > arch/cris/arch-v32/kernel/smp.c calls notify_cpu_starting() from __init code.
> > 
> > arch/x86/mach-voyager/voyager_smp.c calls notify_cpu_starting() from
> > __init code.
> > 
> > arch/m32r/kernel/smpboot.c calls notify_cpu_starting() from __init code.
> > 
> > arch/sparc/kernel/sun4d_smp.c calls notify_cpu_starting() from __init code.
> > 
> > arch/powerpc/kernel/smp.c calls notify_cpu_starting() from __devinit
> > code.
> > 
> > arch/um/kernel/smp.c calls notify_cpu_starting() from .text code.
> > 
> > 
> > The other nine callers call notify_cpu_starting() from __cpuinit code.
> > 
> > 
> > What a mess.
> 
> __cpuinit seems safe for all but UML.
> 
> But even for UML it appears to be de-facto safe: as after bootup we 
> never return back into arch/um/kernel/smp.c::idle_proc(). (as UML's 
From the list Andrew provided powerpc needs to be looked after.
We cannot call an __init function from __devinit context.
If you already checked that then no objections.

	Sam

Re: [PATCH] kernel/cpu.c: Section mismatch warning fix.

[Ingo Molnar]

* Sam Ravnborg <sam@ravnborg.org> wrote:

> On Tue, Nov 04, 2008 at 10:48:31AM +0100, Ingo Molnar wrote:
> > 
> > * Andrew Morton <akpm@linux-foundation.org> wrote:
> > 
> > > On Thu, 30 Oct 2008 10:04:54 +0600
> > > "Rakib Mullick" <rakib.mullick@gmail.com> wrote:
> > > 
> > > >  LD      kernel/built-in.o
> > > > WARNING: kernel/built-in.o(.text+0xb7c8): Section mismatch in
> > > > reference from the function notify_cpu_starting() to the variable
> > > > .cpuinit.data:cpu_chain
> > > > The function notify_cpu_starting() references
> > > > the variable __cpuinitdata cpu_chain.
> > > > This is often because notify_cpu_starting lacks a __cpuinitdata
> > > > annotation or the annotation of cpu_chain is wrong.
> > > > 
> > > > This patch fixes the above section mismatch warning. If anything else
> > > > please notice.
> > > > Thanks.
> > > > 
> > > > Signed-off-by: Md.Rakib H. Mullick <rakib.mullick@gmail.com>
> > > > 
> > > > --- linux-2.6-orig/kernel/cpu.c	2008-10-28 20:52:38.000000000 +0600
> > > > +++ linux-2.6/kernel/cpu.c	2008-10-28 22:46:22.000000000 +0600
> > > > @@ -462,7 +462,7 @@ out:
> > > >   * It must be called by the arch code on the new cpu, before the new cpu
> > > >   * enables interrupts and before the "boot" cpu returns from __cpu_up().
> > > >   */
> > > > -void notify_cpu_starting(unsigned int cpu)
> > > > +void __cpuinit notify_cpu_starting(unsigned int cpu)
> > > >  {
> > > >  	unsigned long val = CPU_STARTING;
> > > 
> > > arch/alpha/kernel/smp.c calls notify_cpu_starting() from __init code.
> > > 
> > > arch/cris/arch-v32/kernel/smp.c calls notify_cpu_starting() from __init code.
> > > 
> > > arch/x86/mach-voyager/voyager_smp.c calls notify_cpu_starting() from
> > > __init code.
> > > 
> > > arch/m32r/kernel/smpboot.c calls notify_cpu_starting() from __init code.
> > > 
> > > arch/sparc/kernel/sun4d_smp.c calls notify_cpu_starting() from __init code.
> > > 
> > > arch/powerpc/kernel/smp.c calls notify_cpu_starting() from __devinit
> > > code.
> > > 
> > > arch/um/kernel/smp.c calls notify_cpu_starting() from .text code.
> > > 
> > > 
> > > The other nine callers call notify_cpu_starting() from __cpuinit code.
> > > 
> > > 
> > > What a mess.
> > 
> > __cpuinit seems safe for all but UML.
> > 
> > But even for UML it appears to be de-facto safe: as after bootup we 
> > never return back into arch/um/kernel/smp.c::idle_proc(). (as UML's 
> >From the list Andrew provided powerpc needs to be looked after.
> We cannot call an __init function from __devinit context.
> If you already checked that then no objections.

the patch/context in question is attached below - in that we weaken 
the persistency of notify_cpu_starting() from .text to __cpuinit. 
Which should be safe, right?

	Ingo

-------->
From 685aebb5fb26126e9be708cfed30e5ee20033c95 Mon Sep 17 00:00:00 2001
From: Rakib Mullick <rakib.mullick@gmail.com>
Date: Thu, 30 Oct 2008 10:04:54 +0600
Subject: [PATCH] kernel/cpu.c: section mismatch warning fix

Impact: small kernel .text reduction in certain configs, and fix warning

This warning:

  LD      kernel/built-in.o
 WARNING: kernel/built-in.o(.text+0xb7c8): Section mismatch in
 reference from the function notify_cpu_starting() to the variable
 .cpuinit.data:cpu_chain
 The function notify_cpu_starting() references
 the variable __cpuinitdata cpu_chain.
 This is often because notify_cpu_starting lacks a __cpuinitdata
 annotation or the annotation of cpu_chain is wrong.

Points out the fact that notify_cpu_starting() is only called
from __cpuinit sequences and itself uses a __cpuinitdata structure.

This patch fixes the above section mismatch warning.

Signed-off-by: Md.Rakib H. Mullick <rakib.mullick@gmail.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 kernel/cpu.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/kernel/cpu.c b/kernel/cpu.c
index 86d4904..2e53420 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -462,7 +462,7 @@ out:
  * It must be called by the arch code on the new cpu, before the new cpu
  * enables interrupts and before the "boot" cpu returns from __cpu_up().
  */
-void notify_cpu_starting(unsigned int cpu)
+void __cpuinit notify_cpu_starting(unsigned int cpu)
 {
 	unsigned long val = CPU_STARTING;
 

Re: [PATCH] kernel/cpu.c: Section mismatch warning fix.

[Sam Ravnborg]

On Tue, Nov 04, 2008 at 11:22:52AM +0100, Ingo Molnar wrote:
> 
> * Sam Ravnborg <sam@ravnborg.org> wrote:
> 
> > On Tue, Nov 04, 2008 at 10:48:31AM +0100, Ingo Molnar wrote:
> > > 
> > > * Andrew Morton <akpm@linux-foundation.org> wrote:
> > > 
> > > > On Thu, 30 Oct 2008 10:04:54 +0600
> > > > "Rakib Mullick" <rakib.mullick@gmail.com> wrote:
> > > > 
> > > > >  LD      kernel/built-in.o
> > > > > WARNING: kernel/built-in.o(.text+0xb7c8): Section mismatch in
> > > > > reference from the function notify_cpu_starting() to the variable
> > > > > .cpuinit.data:cpu_chain
> > > > > The function notify_cpu_starting() references
> > > > > the variable __cpuinitdata cpu_chain.
> > > > > This is often because notify_cpu_starting lacks a __cpuinitdata
> > > > > annotation or the annotation of cpu_chain is wrong.
> > > > > 
> > > > > This patch fixes the above section mismatch warning. If anything else
> > > > > please notice.
> > > > > Thanks.
> > > > > 
> > > > > Signed-off-by: Md.Rakib H. Mullick <rakib.mullick@gmail.com>
> > > > > 
> > > > > --- linux-2.6-orig/kernel/cpu.c	2008-10-28 20:52:38.000000000 +0600
> > > > > +++ linux-2.6/kernel/cpu.c	2008-10-28 22:46:22.000000000 +0600
> > > > > @@ -462,7 +462,7 @@ out:
> > > > >   * It must be called by the arch code on the new cpu, before the new cpu
> > > > >   * enables interrupts and before the "boot" cpu returns from __cpu_up().
> > > > >   */
> > > > > -void notify_cpu_starting(unsigned int cpu)
> > > > > +void __cpuinit notify_cpu_starting(unsigned int cpu)
> > > > >  {
> > > > >  	unsigned long val = CPU_STARTING;
> > > > 
> > > > arch/alpha/kernel/smp.c calls notify_cpu_starting() from __init code.
> > > > 
> > > > arch/cris/arch-v32/kernel/smp.c calls notify_cpu_starting() from __init code.
> > > > 
> > > > arch/x86/mach-voyager/voyager_smp.c calls notify_cpu_starting() from
> > > > __init code.
> > > > 
> > > > arch/m32r/kernel/smpboot.c calls notify_cpu_starting() from __init code.
> > > > 
> > > > arch/sparc/kernel/sun4d_smp.c calls notify_cpu_starting() from __init code.
> > > > 
> > > > arch/powerpc/kernel/smp.c calls notify_cpu_starting() from __devinit
> > > > code.
> > > > 
> > > > arch/um/kernel/smp.c calls notify_cpu_starting() from .text code.
> > > > 
> > > > 
> > > > The other nine callers call notify_cpu_starting() from __cpuinit code.
> > > > 
> > > > 
> > > > What a mess.
> > > 
> > > __cpuinit seems safe for all but UML.
> > > 
> > > But even for UML it appears to be de-facto safe: as after bootup we 
> > > never return back into arch/um/kernel/smp.c::idle_proc(). (as UML's 
> > >From the list Andrew provided powerpc needs to be looked after.
> > We cannot call an __init function from __devinit context.
> > If you already checked that then no objections.
> 
> the patch/context in question is attached below - in that we weaken 
> the persistency of notify_cpu_starting() from .text to __cpuinit. 
> Which should be safe, right?

The problem is powerpc where we have a __devinit function that
now calls a __cpuinit function.
And last I looked it was possible to tweak the configuration
so we had HOTPLUG enabled but HOTPLUG_CPU disabled in which
case we could end up in a situation where we call
notify_cpu_starting() from __devinit context but we have freed
the __cpuinit memory where notify_cpu_starting() was living.

From a quick look the annotation in powerpc is wrong
but as it is used from assembler and I did not look carefully
I cannot say for sure.

So until the powerpc case is properly analysed we should hold
on this patch. If this patch is correct then we should also
get powerpc fixed.

	Sam

Re: [PATCH] kernel/cpu.c: Section mismatch warning fix.

[James Bottomley]

On Tue, 2008-11-04 at 12:02 +0100, Sam Ravnborg wrote:
> On Tue, Nov 04, 2008 at 11:22:52AM +0100, Ingo Molnar wrote:
> > 
> > * Sam Ravnborg <sam@ravnborg.org> wrote:
> > 
> > > On Tue, Nov 04, 2008 at 10:48:31AM +0100, Ingo Molnar wrote:
> > > > 
> > > > * Andrew Morton <akpm@linux-foundation.org> wrote:
> > > > 
> > > > > On Thu, 30 Oct 2008 10:04:54 +0600
> > > > > "Rakib Mullick" <rakib.mullick@gmail.com> wrote:
> > > > > 
> > > > > >  LD      kernel/built-in.o
> > > > > > WARNING: kernel/built-in.o(.text+0xb7c8): Section mismatch in
> > > > > > reference from the function notify_cpu_starting() to the variable
> > > > > > .cpuinit.data:cpu_chain
> > > > > > The function notify_cpu_starting() references
> > > > > > the variable __cpuinitdata cpu_chain.
> > > > > > This is often because notify_cpu_starting lacks a __cpuinitdata
> > > > > > annotation or the annotation of cpu_chain is wrong.
> > > > > > 
> > > > > > This patch fixes the above section mismatch warning. If anything else
> > > > > > please notice.
> > > > > > Thanks.
> > > > > > 
> > > > > > Signed-off-by: Md.Rakib H. Mullick <rakib.mullick@gmail.com>
> > > > > > 
> > > > > > --- linux-2.6-orig/kernel/cpu.c	2008-10-28 20:52:38.000000000 +0600
> > > > > > +++ linux-2.6/kernel/cpu.c	2008-10-28 22:46:22.000000000 +0600
> > > > > > @@ -462,7 +462,7 @@ out:
> > > > > >   * It must be called by the arch code on the new cpu, before the new cpu
> > > > > >   * enables interrupts and before the "boot" cpu returns from __cpu_up().
> > > > > >   */
> > > > > > -void notify_cpu_starting(unsigned int cpu)
> > > > > > +void __cpuinit notify_cpu_starting(unsigned int cpu)
> > > > > >  {
> > > > > >  	unsigned long val = CPU_STARTING;
> > > > > 
> > > > > arch/alpha/kernel/smp.c calls notify_cpu_starting() from __init code.
> > > > > 
> > > > > arch/cris/arch-v32/kernel/smp.c calls notify_cpu_starting() from __init code.
> > > > > 
> > > > > arch/x86/mach-voyager/voyager_smp.c calls notify_cpu_starting() from
> > > > > __init code.
> > > > > 
> > > > > arch/m32r/kernel/smpboot.c calls notify_cpu_starting() from __init code.
> > > > > 
> > > > > arch/sparc/kernel/sun4d_smp.c calls notify_cpu_starting() from __init code.
> > > > > 
> > > > > arch/powerpc/kernel/smp.c calls notify_cpu_starting() from __devinit
> > > > > code.
> > > > > 
> > > > > arch/um/kernel/smp.c calls notify_cpu_starting() from .text code.
> > > > > 
> > > > > 
> > > > > The other nine callers call notify_cpu_starting() from __cpuinit code.
> > > > > 
> > > > > 
> > > > > What a mess.
> > > > 
> > > > __cpuinit seems safe for all but UML.
> > > > 
> > > > But even for UML it appears to be de-facto safe: as after bootup we 
> > > > never return back into arch/um/kernel/smp.c::idle_proc(). (as UML's 
> > > >From the list Andrew provided powerpc needs to be looked after.
> > > We cannot call an __init function from __devinit context.
> > > If you already checked that then no objections.
> > 
> > the patch/context in question is attached below - in that we weaken 
> > the persistency of notify_cpu_starting() from .text to __cpuinit. 
> > Which should be safe, right?
> 
> The problem is powerpc where we have a __devinit function that
> now calls a __cpuinit function.
> And last I looked it was possible to tweak the configuration
> so we had HOTPLUG enabled but HOTPLUG_CPU disabled in which
> case we could end up in a situation where we call
> notify_cpu_starting() from __devinit context but we have freed
> the __cpuinit memory where notify_cpu_starting() was living.
> 
> >From a quick look the annotation in powerpc is wrong
> but as it is used from assembler and I did not look carefully
> I cannot say for sure.
> 
> So until the powerpc case is properly analysed we should hold
> on this patch. If this patch is correct then we should also
> get powerpc fixed.

Sigh, I know this is a broken record, but honestly, can't you see here
we've created a monster of incredible and subtle complexity which is
very difficult even for kernel experts to get right.  Your last
objection to this was that the tools do it for us, but clearly that's
not the case; it requires thought and planning to work as well.

The memory savings of __cpuinit are miniscule and only work on the 1% of
hardware that doesn't have hotplug CPU set.  Can we not just dump the
__cpuinit designation and have this entire headache go away ... I
believe Arjan even has patches.

James

Re: [PATCH] kernel/cpu.c: Section mismatch warning fix.

[Sam Ravnborg]

On Tue, Nov 04, 2008 at 08:06:33AM -0500, James Bottomley wrote:
> On Tue, 2008-11-04 at 12:02 +0100, Sam Ravnborg wrote:
> > On Tue, Nov 04, 2008 at 11:22:52AM +0100, Ingo Molnar wrote:
> > > 
> > > * Sam Ravnborg <sam@ravnborg.org> wrote:
> > > 
> > > > On Tue, Nov 04, 2008 at 10:48:31AM +0100, Ingo Molnar wrote:
> > > > > 
> > > > > * Andrew Morton <akpm@linux-foundation.org> wrote:
> > > > > 
> > > > > > On Thu, 30 Oct 2008 10:04:54 +0600
> > > > > > "Rakib Mullick" <rakib.mullick@gmail.com> wrote:
> > > > > > 
> > > > > > >  LD      kernel/built-in.o
> > > > > > > WARNING: kernel/built-in.o(.text+0xb7c8): Section mismatch in
> > > > > > > reference from the function notify_cpu_starting() to the variable
> > > > > > > .cpuinit.data:cpu_chain
> > > > > > > The function notify_cpu_starting() references
> > > > > > > the variable __cpuinitdata cpu_chain.
> > > > > > > This is often because notify_cpu_starting lacks a __cpuinitdata
> > > > > > > annotation or the annotation of cpu_chain is wrong.
> > > > > > > 
> > > > > > > This patch fixes the above section mismatch warning. If anything else
> > > > > > > please notice.
> > > > > > > Thanks.
> > > > > > > 
> > > > > > > Signed-off-by: Md.Rakib H. Mullick <rakib.mullick@gmail.com>
> > > > > > > 
> > > > > > > --- linux-2.6-orig/kernel/cpu.c	2008-10-28 20:52:38.000000000 +0600
> > > > > > > +++ linux-2.6/kernel/cpu.c	2008-10-28 22:46:22.000000000 +0600
> > > > > > > @@ -462,7 +462,7 @@ out:
> > > > > > >   * It must be called by the arch code on the new cpu, before the new cpu
> > > > > > >   * enables interrupts and before the "boot" cpu returns from __cpu_up().
> > > > > > >   */
> > > > > > > -void notify_cpu_starting(unsigned int cpu)
> > > > > > > +void __cpuinit notify_cpu_starting(unsigned int cpu)
> > > > > > >  {
> > > > > > >  	unsigned long val = CPU_STARTING;
> > > > > > 
> > > > > > arch/alpha/kernel/smp.c calls notify_cpu_starting() from __init code.
> > > > > > 
> > > > > > arch/cris/arch-v32/kernel/smp.c calls notify_cpu_starting() from __init code.
> > > > > > 
> > > > > > arch/x86/mach-voyager/voyager_smp.c calls notify_cpu_starting() from
> > > > > > __init code.
> > > > > > 
> > > > > > arch/m32r/kernel/smpboot.c calls notify_cpu_starting() from __init code.
> > > > > > 
> > > > > > arch/sparc/kernel/sun4d_smp.c calls notify_cpu_starting() from __init code.
> > > > > > 
> > > > > > arch/powerpc/kernel/smp.c calls notify_cpu_starting() from __devinit
> > > > > > code.
> > > > > > 
> > > > > > arch/um/kernel/smp.c calls notify_cpu_starting() from .text code.
> > > > > > 
> > > > > > 
> > > > > > The other nine callers call notify_cpu_starting() from __cpuinit code.
> > > > > > 
> > > > > > 
> > > > > > What a mess.
> > > > > 
> > > > > __cpuinit seems safe for all but UML.
> > > > > 
> > > > > But even for UML it appears to be de-facto safe: as after bootup we 
> > > > > never return back into arch/um/kernel/smp.c::idle_proc(). (as UML's 
> > > > >From the list Andrew provided powerpc needs to be looked after.
> > > > We cannot call an __init function from __devinit context.
> > > > If you already checked that then no objections.
> > > 
> > > the patch/context in question is attached below - in that we weaken 
> > > the persistency of notify_cpu_starting() from .text to __cpuinit. 
> > > Which should be safe, right?
> > 
> > The problem is powerpc where we have a __devinit function that
> > now calls a __cpuinit function.
> > And last I looked it was possible to tweak the configuration
> > so we had HOTPLUG enabled but HOTPLUG_CPU disabled in which
> > case we could end up in a situation where we call
> > notify_cpu_starting() from __devinit context but we have freed
> > the __cpuinit memory where notify_cpu_starting() was living.
> > 
> > >From a quick look the annotation in powerpc is wrong
> > but as it is used from assembler and I did not look carefully
> > I cannot say for sure.
> > 
> > So until the powerpc case is properly analysed we should hold
> > on this patch. If this patch is correct then we should also
> > get powerpc fixed.
> 
> Sigh, I know this is a broken record, but honestly, can't you see here
> we've created a monster of incredible and subtle complexity which is
> very difficult even for kernel experts to get right.  Your last
> objection to this was that the tools do it for us, but clearly that's
> not the case; it requires thought and planning to work as well.
> 
> The memory savings of __cpuinit are miniscule and only work on the 1% of
> hardware that doesn't have hotplug CPU set.  Can we not just dump the
> __cpuinit designation and have this entire headache go away ... I
> believe Arjan even has patches.

I support killing of __cpuinit too if you check the thread where Arjan
posted patches. But until then we better do things correct.
cpuinit is by far the most difficult of them all, especially due
to all the wrong use of cpuinit.

	Sam