Solving section mismatches
in [Kernel]
|
From: Sam Ravnborg on 27 Sep 2009 15:50 On Sun, Sep 27, 2009 at 08:09:53PM +0100, Russell King - ARM Linux wrote: > 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? Lets try it. - is secondary_start_kernel used only in init context with HOTPLUG_CPU=n - yes - is secondary_start_kernel used outside init context with HOTPLUG_CPU=y - yes So the __cpuinit annotation is correct. cpu_idle is calling secondary_start_kernel() but cpu_idle() does not fulfill the criteria for any of the __cpu* annotations. So there is only two ways to tell modpost that this is correct. Either rename it so it fullfill one of the whitelisted names or explicitly tell modpost that this function may reference __*init/__*exit annotated functions/code. The latter is much preferred. So the right fix is to annotate it with __ref. (see include/linux/init.h) Had I looked closely last time I would have know this already - but alas I was to quick to reply. Please try this: diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c index e0d3277..835188b 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 __ref cpu_die(void) { unsigned int cpu = smp_processor_id(); You can add my: Signed-off-by: Sam Ravnborg <sam(a)ravnborg.org> or Acked-by: Sam Ravnborg <sam(a)ravnborg.org> as you prefer. Assuming this works of course. 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 27 Sep 2009 16:20 On Sun, Sep 27, 2009 at 09:47:47PM +0200, Sam Ravnborg wrote: > Please try this: > diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c > index e0d3277..835188b 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 __ref cpu_die(void) > { > unsigned int cpu = smp_processor_id(); That works - so can I get your ack for this entire patch please: From: Russell King <rmk+kernel(a)arm.linux.org.uk> Subject: [PATCH] ARM: Fix __cpuexit section mismatch warnings Fix: WARNING: vmlinux.o(.text+0x247c): 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: vmlinux.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. Sam says: > 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 So, this also means __cpu_disable(), __cpu_die() and twd_timer_stop() are also wrong. However, removing __cpuexit from cpu_die() creates: WARNING: vmlinux.o(.text+0x6834): 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 fix this using __ref. Signed-off-by: Russell King <rmk+kernel(a)arm.linux.org.uk> --- arch/arm/kernel/smp.c | 6 +++--- arch/arm/kernel/smp_twd.c | 4 +++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c index 9d015ee..57162af 100644 --- a/arch/arm/kernel/smp.c +++ b/arch/arm/kernel/smp.c @@ -154,7 +154,7 @@ int __cpuinit __cpu_up(unsigned int cpu) /* * __cpu_disable runs on the processor to be shutdown. */ -int __cpuexit __cpu_disable(void) +int __cpu_disable(void) { unsigned int cpu = smp_processor_id(); struct task_struct *p; @@ -201,7 +201,7 @@ int __cpuexit __cpu_disable(void) * called on the thread which is asking for a CPU to be shutdown - * waits until shutdown has completed, or it is timed out. */ -void __cpuexit __cpu_die(unsigned int cpu) +void __cpu_die(unsigned int cpu) { if (!platform_cpu_kill(cpu)) printk("CPU%u: unable to kill\n", cpu); @@ -215,7 +215,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 __ref cpu_die(void) { unsigned int cpu = smp_processor_id(); diff --git a/arch/arm/kernel/smp_twd.c b/arch/arm/kernel/smp_twd.c index d8c88c6..a73a34d 100644 --- a/arch/arm/kernel/smp_twd.c +++ b/arch/arm/kernel/smp_twd.c @@ -166,10 +166,12 @@ void __cpuinit twd_timer_setup(struct clock_event_device *clk) clockevents_register_device(clk); } +#ifdef CONFIG_HOTPLUG_CPU /* * take a local timer down */ -void __cpuexit twd_timer_stop(void) +void twd_timer_stop(void) { __raw_writel(0, twd_base + TWD_TIMER_CONTROL); } +#endif -- 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 27 Sep 2009 16:30 On Sun, Sep 27, 2009 at 10:22:51PM +0200, Sam Ravnborg wrote: > Patch looks good! > Acked-by: Sam Ravnborg <sam(a)ravnborg.org> Thanks 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: Sam Ravnborg on 27 Sep 2009 16:30 On Sun, Sep 27, 2009 at 09:15:07PM +0100, Russell King - ARM Linux wrote: > On Sun, Sep 27, 2009 at 09:47:47PM +0200, Sam Ravnborg wrote: > > Please try this: > > diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c > > index e0d3277..835188b 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 __ref cpu_die(void) > > { > > unsigned int cpu = smp_processor_id(); > > That works - so can I get your ack for this entire patch please: > > From: Russell King <rmk+kernel(a)arm.linux.org.uk> > Subject: [PATCH] ARM: Fix __cpuexit section mismatch warnings > > Fix: > > WARNING: vmlinux.o(.text+0x247c): 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: vmlinux.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. > > Sam says: > > 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 > > So, this also means __cpu_disable(), __cpu_die() and twd_timer_stop() are > also wrong. However, removing __cpuexit from cpu_die() creates: > > WARNING: vmlinux.o(.text+0x6834): 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 fix this using __ref. > > Signed-off-by: Russell King <rmk+kernel(a)arm.linux.org.uk> Patch looks good! Acked-by: Sam Ravnborg <sam(a)ravnborg.org> 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/
First
|
Prev
|
Pages: 1 2 Prev: [GIT pull] timer updates for 2.6.32 Next: [PATCH] const: constify remaining file_operations |