From: Russell King - ARM Linux on
Sam,

Any idea how to solve this:

WARNING: arch/arm/kernel/built-in.o(.text+0x1ebc): Section mismatch in reference from the function cpu_idle() to the function .cpuexit.text:cpu_die()
The function cpu_idle() references a function in an exit section.
Often the function cpu_die() has valid usage outside the exit section
and the fix is to remove the __cpuexit annotation of cpu_die.

WARNING: arch/arm/kernel/built-in.o(.cpuexit.text+0x3c): Section mismatch in reference from the function cpu_die() to the function .cpuinit.text:secondary_start_kernel()
The function __cpuexit cpu_die() references
a function __cpuinit secondary_start_kernel().
This is often seen when error handling in the exit function
uses functionality in the init path.
The fix is often to remove the __cpuinit annotation of
secondary_start_kernel() so it may be used outside an init section.

Logically, the annotations are correct - in the first case, cpu_die()
will only ever be called if hotplug CPU is enabled, since you can't
offline a CPU without hotplug CPU enabled. In that case, the __cpuexit.*
sections are not discarded.

In the second case, this is platform code to restart the CPU, and
again the anotations are entirely correct - cpuexit code only exists
if hotplug CPU is enabled, in which case the cpuinit code must also
be kept around. Therefore, it is entirely legal for cpuexit code to
call cpuinit code - unlike __init vs __exit or __devinit vs __devexit.

Therefore, I believe both of these warnings to be incorrect.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Sam Ravnborg on
On Sun, Sep 27, 2009 at 05:41:16PM +0100, Russell King - ARM Linux wrote:
> Sam,
>
> Any idea how to solve this:
>
> WARNING: arch/arm/kernel/built-in.o(.text+0x1ebc): Section mismatch in reference from the function cpu_idle() to the function .cpuexit.text:cpu_die()
> The function cpu_idle() references a function in an exit section.
> Often the function cpu_die() has valid usage outside the exit section
> and the fix is to remove the __cpuexit annotation of cpu_die.
>
> WARNING: arch/arm/kernel/built-in.o(.cpuexit.text+0x3c): Section mismatch in reference from the function cpu_die() to the function .cpuinit.text:secondary_start_kernel()
> The function __cpuexit cpu_die() references
> a function __cpuinit secondary_start_kernel().
> This is often seen when error handling in the exit function
> uses functionality in the init path.
> The fix is often to remove the __cpuinit annotation of
> secondary_start_kernel() so it may be used outside an init section.
>
> Logically, the annotations are correct - in the first case, cpu_die()
> will only ever be called if hotplug CPU is enabled, since you can't
> offline a CPU without hotplug CPU enabled. In that case, the __cpuexit.*
> sections are not discarded.

The annotation of cpu_die() is wrong.
To be annotated __cpuexit the function shall:
- be used in exit context and only in exit context with HOTPLUG_CPU=n
- be used outside exit context with HOTPLUG_CPU=y

cpu_die() fails on the first condition because it is only used
if HOTPLUG_CPU=y.
The annotation is wrongly used as a replacement for an ifdef.
As cpu_die() is already inside ifdef CONFIG_HOTPLUG_CPU is should
be enough to just remove the annotation.

Like this (copy'n'paste so it does not apply)

diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
index e0d3277..de4ef1c 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -214,7 +214,7 @@ void __cpuexit __cpu_die(unsigned int cpu)
* of the other hotplug-cpu capable cores, so presumably coming
* out of idle fixes this.
*/
-void __cpuexit cpu_die(void)
+void cpu_die(void)
{
unsigned int cpu = smp_processor_id();



I did not see any section mismatch with defconfig so I failed to
test if it made a difference.

>
> In the second case, this is platform code to restart the CPU, and
> again the anotations are entirely correct - cpuexit code only exists
> if hotplug CPU is enabled, in which case the cpuinit code must also
> be kept around. Therefore, it is entirely legal for cpuexit code to
> call cpuinit code - unlike __init vs __exit or __devinit vs __devexit.
>
> Therefore, I believe both of these warnings to be incorrect.

Same reasoning as above.
And the patch ougth to solve this case too.

Please note that throughout the kernel there is a lot of places where
the __cpu* annotations has been misused.
I once suggested to drop the chek from modpost as I could not see
this being fixed - but alas no replies it never happened.

Sam
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Russell King - ARM Linux on
On Sun, Sep 27, 2009 at 08:27:07PM +0200, Sam Ravnborg wrote:
> On Sun, Sep 27, 2009 at 05:41:16PM +0100, Russell King - ARM Linux wrote:
> > Sam,
> >
> > Any idea how to solve this:
> >
> > WARNING: arch/arm/kernel/built-in.o(.text+0x1ebc): Section mismatch in reference from the function cpu_idle() to the function .cpuexit.text:cpu_die()
> > The function cpu_idle() references a function in an exit section.
> > Often the function cpu_die() has valid usage outside the exit section
> > and the fix is to remove the __cpuexit annotation of cpu_die.
> >
> > WARNING: arch/arm/kernel/built-in.o(.cpuexit.text+0x3c): Section mismatch in reference from the function cpu_die() to the function .cpuinit.text:secondary_start_kernel()
> > The function __cpuexit cpu_die() references
> > a function __cpuinit secondary_start_kernel().
> > This is often seen when error handling in the exit function
> > uses functionality in the init path.
> > The fix is often to remove the __cpuinit annotation of
> > secondary_start_kernel() so it may be used outside an init section.
> >
> > Logically, the annotations are correct - in the first case, cpu_die()
> > will only ever be called if hotplug CPU is enabled, since you can't
> > offline a CPU without hotplug CPU enabled. In that case, the __cpuexit.*
> > sections are not discarded.
>
> The annotation of cpu_die() is wrong.
> To be annotated __cpuexit the function shall:
> - be used in exit context and only in exit context with HOTPLUG_CPU=n
> - be used outside exit context with HOTPLUG_CPU=y
>
> cpu_die() fails on the first condition because it is only used
> if HOTPLUG_CPU=y.
> The annotation is wrongly used as a replacement for an ifdef.
> As cpu_die() is already inside ifdef CONFIG_HOTPLUG_CPU is should
> be enough to just remove the annotation.
>
> Like this (copy'n'paste so it does not apply)
>
> diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
> index e0d3277..de4ef1c 100644
> --- a/arch/arm/kernel/smp.c
> +++ b/arch/arm/kernel/smp.c
> @@ -214,7 +214,7 @@ void __cpuexit __cpu_die(unsigned int cpu)
> * of the other hotplug-cpu capable cores, so presumably coming
> * out of idle fixes this.
> */
> -void __cpuexit cpu_die(void)
> +void cpu_die(void)
> {
> unsigned int cpu = smp_processor_id();

This is wrong. cpu_die() does not need to exist if hotplug CPU is
disabled. In that case, it should be discarded and this is precisely
what __cpuexit does. The annotation is, therefore, correct.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Sam Ravnborg on
On Sun, Sep 27, 2009 at 07:39:04PM +0100, Russell King - ARM Linux wrote:
> On Sun, Sep 27, 2009 at 08:27:07PM +0200, Sam Ravnborg wrote:
> > On Sun, Sep 27, 2009 at 05:41:16PM +0100, Russell King - ARM Linux wrote:
> > > Sam,
> > >
> > > Any idea how to solve this:
> > >
> > > WARNING: arch/arm/kernel/built-in.o(.text+0x1ebc): Section mismatch in reference from the function cpu_idle() to the function .cpuexit.text:cpu_die()
> > > The function cpu_idle() references a function in an exit section.
> > > Often the function cpu_die() has valid usage outside the exit section
> > > and the fix is to remove the __cpuexit annotation of cpu_die.
> > >
> > > WARNING: arch/arm/kernel/built-in.o(.cpuexit.text+0x3c): Section mismatch in reference from the function cpu_die() to the function .cpuinit.text:secondary_start_kernel()
> > > The function __cpuexit cpu_die() references
> > > a function __cpuinit secondary_start_kernel().
> > > This is often seen when error handling in the exit function
> > > uses functionality in the init path.
> > > The fix is often to remove the __cpuinit annotation of
> > > secondary_start_kernel() so it may be used outside an init section.
> > >
> > > Logically, the annotations are correct - in the first case, cpu_die()
> > > will only ever be called if hotplug CPU is enabled, since you can't
> > > offline a CPU without hotplug CPU enabled. In that case, the __cpuexit.*
> > > sections are not discarded.
> >
> > The annotation of cpu_die() is wrong.
> > To be annotated __cpuexit the function shall:
> > - be used in exit context and only in exit context with HOTPLUG_CPU=n
> > - be used outside exit context with HOTPLUG_CPU=y
> >
> > cpu_die() fails on the first condition because it is only used
> > if HOTPLUG_CPU=y.
> > The annotation is wrongly used as a replacement for an ifdef.
> > As cpu_die() is already inside ifdef CONFIG_HOTPLUG_CPU is should
> > be enough to just remove the annotation.
> >
> > Like this (copy'n'paste so it does not apply)
> >
> > diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
> > index e0d3277..de4ef1c 100644
> > --- a/arch/arm/kernel/smp.c
> > +++ b/arch/arm/kernel/smp.c
> > @@ -214,7 +214,7 @@ void __cpuexit __cpu_die(unsigned int cpu)
> > * of the other hotplug-cpu capable cores, so presumably coming
> > * out of idle fixes this.
> > */
> > -void __cpuexit cpu_die(void)
> > +void cpu_die(void)
> > {
> > unsigned int cpu = smp_processor_id();
>
> This is wrong. cpu_die() does not need to exist if hotplug CPU is
> disabled. In that case, it should be discarded and this is precisely
> what __cpuexit does. The annotation is, therefore, correct.

From arch/arm/kernel/smp.c

#ifdef CONFIG_HOTPLUG_CPU

....
void __cpuexit cpu_die(void)
{
unsigned int cpu = smp_processor_id();

local_irq_disable();
idle_task_exit();

/*
* actual CPU shutdown procedure is at least platform (if not
* CPU) specific
*/
platform_cpu_die(cpu);

/*
* Do not return to the idle loop - jump back to the secondary
* cpu initialisation. There's some initialisation which needs
* to be repeated to undo the effects of taking the CPU offline.
*/
__asm__("mov sp, %0\n"
" b secondary_start_kernel"
:
: "r" (task_stack_page(current) + THREAD_SIZE - 8));
}
#endif /* CONFIG_HOTPLUG_CPU */


Please look at the above and realise that cpu_die() is only ever defined
in case that HOTPLUG_CPU is defined. So there is nothing to discard if
HOTPLUG_CPU equals to n.

And just to repeat myself....
The only correct use of __cpu* annotation is for function/data that is
used with or without HOTPLUG_CPU equals to y.
Which is NOT the case for cpu_die().

The __cpu* annotation is not a replacement for ifdeffed out code that is
not relevant for the non-HOTPLUG_CPU case.

Sam
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Russell King - ARM Linux on
On Sun, Sep 27, 2009 at 08:46:45PM +0200, Sam Ravnborg wrote:
> Please look at the above and realise that cpu_die() is only ever defined
> in case that HOTPLUG_CPU is defined. So there is nothing to discard if
> HOTPLUG_CPU equals to n.
>
> And just to repeat myself....
> The only correct use of __cpu* annotation is for function/data that is
> used with or without HOTPLUG_CPU equals to y.
> Which is NOT the case for cpu_die().
>
> The __cpu* annotation is not a replacement for ifdeffed out code that is
> not relevant for the non-HOTPLUG_CPU case.

Ok. But that still doesn't solve:

WARNING: arch/arm/kernel/built-in.o(.text+0x6274): Section mismatch in reference from the function cpu_die() to the function .cpuinit.text:secondary_start_kernel()
The function cpu_die() references
the function __cpuinit secondary_start_kernel().
This is often because cpu_die lacks a __cpuinit
annotation or the annotation of secondary_start_kernel is wrong.

So are you saying secondary_start_kernel should not have __cpuinit?

That seems wrong, since we do want it to be discarded as __init in the
non-hotplug case, but we do want it kept around in the hotplug cpu
case. I guess something like the following could be used instead:

#ifdef CONFIG_HOTPLUG_CPU
asmlinkage void secondary_start_kernel(void)
#else
asmlinkage void __init secondary_start_kernel(void)
#endif
{
....
}

but my understanding was that's what __cpuinit was supposed to be
doing for us in the first place.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/