Prev: Quickstart Button ACPI driver to serve PNP0C32 ACPI devices
Next: Email Admin Warning Alert !!!
From: Andrew Morton on 28 May 2010 17:20
On Tue, 25 May 2010 17:28:58 +0200
Borislav Petkov <borislav.petkov(a)amd.com> wrote:

> Fix the following warning:
>
> "WARNING: arch/x86/kernel/built-in.o(.exit.text+0x72):
> Section mismatch in reference from the function powernowk8_exit() to the variable .cpuinit.data:cpb_nb
>
> The function __exit powernowk8_exit() references a variable
> __cpuinitdata cpb_nb. This is often seen when error handling in the exit
> function uses functionality in the init path. The fix is often to remove
> the __cpuinitdata annotation of cpb_nb so it may be used outside an init
> section."
>
> Cc: <stable(a)kernel.org>
> Reported-by: H. Peter Anvin <hpa(a)zytor.com>
> Signed-off-by: Borislav Petkov <borislav.petkov(a)amd.com>
> ---
> arch/x86/kernel/cpu/cpufreq/powernow-k8.c | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/cpufreq/powernow-k8.c b/arch/x86/kernel/cpu/cpufreq/powernow-k8.c
> index 6f3dc8f..7ec2123 100644
> --- a/arch/x86/kernel/cpu/cpufreq/powernow-k8.c
> +++ b/arch/x86/kernel/cpu/cpufreq/powernow-k8.c
> @@ -1497,8 +1497,8 @@ static struct cpufreq_driver cpufreq_amd64_driver = {
> * simply keep the boost-disable flag in sync with the current global
> * state.
> */
> -static int __cpuinit cpb_notify(struct notifier_block *nb, unsigned long action,
> - void *hcpu)
> +static int cpb_notify(struct notifier_block *nb, unsigned long action,
> + void *hcpu)
> {
> unsigned cpu = (long)hcpu;
> u32 lo, hi;
> @@ -1528,7 +1528,7 @@ static int __cpuinit cpb_notify(struct notifier_block *nb, unsigned long action,
> return NOTIFY_OK;
> }
>
> -static struct notifier_block __cpuinitdata cpb_nb = {
> +static struct notifier_block cpb_nb = {
> .notifier_call = cpb_notify,
> };

I _think_ this is notabug. If CONFIG_HOTPLUG_CPU=y, this code/data
doesn't get discarded. If CONFIG_HOTPLUG_CPU=n,
[un]register_cpu_notifier() is a no-op.

But apparently the build system _did_ detect such a reference, so
what's going on? Maybe I'm confused again.


This code needs work. It should use hotcpu_notifier() rather than
diving down into the lower-level register_cpu_notifier(). If this is
done then linker magic will cause all the hotplug-specific code to be
omitted from vmlinux if CONFIG_HOTPLUG_CPU=n, and I suppose it will fix
the above warning.


--
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: Borislav Petkov on 29 May 2010 05:40
From: Andrew Morton <akpm(a)linux-foundation.org>
Date: Fri, May 28, 2010 at 05:09:08PM -0400

Hi Andrew,

> On Tue, 25 May 2010 17:28:58 +0200
> Borislav Petkov <borislav.petkov(a)amd.com> wrote:
>
> > Fix the following warning:
> >
> > "WARNING: arch/x86/kernel/built-in.o(.exit.text+0x72):
> > Section mismatch in reference from the function powernowk8_exit() to the variable .cpuinit.data:cpb_nb
> >
> > The function __exit powernowk8_exit() references a variable
> > __cpuinitdata cpb_nb. This is often seen when error handling in the exit
> > function uses functionality in the init path. The fix is often to remove
> > the __cpuinitdata annotation of cpb_nb so it may be used outside an init
> > section."
> >
> > Cc: <stable(a)kernel.org>
> > Reported-by: H. Peter Anvin <hpa(a)zytor.com>
> > Signed-off-by: Borislav Petkov <borislav.petkov(a)amd.com>
> > ---
> > arch/x86/kernel/cpu/cpufreq/powernow-k8.c | 6 +++---
> > 1 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/x86/kernel/cpu/cpufreq/powernow-k8.c b/arch/x86/kernel/cpu/cpufreq/powernow-k8.c
> > index 6f3dc8f..7ec2123 100644
> > --- a/arch/x86/kernel/cpu/cpufreq/powernow-k8.c
> > +++ b/arch/x86/kernel/cpu/cpufreq/powernow-k8.c
> > @@ -1497,8 +1497,8 @@ static struct cpufreq_driver cpufreq_amd64_driver = {
> > * simply keep the boost-disable flag in sync with the current global
> > * state.
> > */
> > -static int __cpuinit cpb_notify(struct notifier_block *nb, unsigned long action,
> > - void *hcpu)
> > +static int cpb_notify(struct notifier_block *nb, unsigned long action,
> > + void *hcpu)
> > {
> > unsigned cpu = (long)hcpu;
> > u32 lo, hi;
> > @@ -1528,7 +1528,7 @@ static int __cpuinit cpb_notify(struct notifier_block *nb, unsigned long action,
> > return NOTIFY_OK;
> > }
> >
> > -static struct notifier_block __cpuinitdata cpb_nb = {
> > +static struct notifier_block cpb_nb = {
> > .notifier_call = cpb_notify,
> > };
>
> I _think_ this is notabug. If CONFIG_HOTPLUG_CPU=y, this code/data
> doesn't get discarded. If CONFIG_HOTPLUG_CPU=n,
> [un]register_cpu_notifier() is a no-op.
>
> But apparently the build system _did_ detect such a reference, so
> what's going on? Maybe I'm confused again.

No, you're right. We need all that notifier code strictly for when we
take a cpu offline. The registration should too be cpu hotplug-aware for
it is the only logical thing to do. However...

> This code needs work. It should use hotcpu_notifier() rather than
> diving down into the lower-level register_cpu_notifier(). If this is
> done then linker magic will cause all the hotplug-specific code to be
> omitted from vmlinux if CONFIG_HOTPLUG_CPU=n, and I suppose it will fix
> the above warning.

.... the original warning is __exit versus __cpuinitdata section
marking which CONFIG_DEBUG_SECTION_MISMATCH complains about. Using the
[un]register_hotcpu_notifier() causes another one:

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

LD vmlinux.o
MODPOST vmlinux.o
WARNING: vmlinux.o(.text+0x3ba8d): Section mismatch in reference from the function __cpu_notify() to the variable .cpuinit.data:cpu_chain
The function __cpu_notify() references
the variable __cpuinitdata cpu_chain.
This is often because __cpu_notify lacks a __cpuinitdata
annotation or the annotation of cpu_chain is wrong.

so we're kinda like between a rock and a hard place here :).

Bottomline is: I wanna unregister the notifier at module exit time, and,
at the same time, get rid of all that code when CONFIG_HOTPLUG_CPU is
off.

But, if .cpuinit.data is being discarded at link time and the module
exit function references a .cpuinit.data symbol, we're simply dropping
all those [un]register_cpu_notifier() calls. Which means an unused
function and a notifier_block.

It could be that the above is becoming moot, though, for HOTPLUG_CPU
is almost always enabled (and being selected by a bunch of other core
stuff especially like PM_SLEEP_SMP etc) so we can save us the trouble of
section marking all those symbols. In this case of EMBEDDED you simply
disable powernow-k8.

But I don't know, I might just as well be missing something else.

Thanks.

--
Regards/Gruss,
Boris.

Operating Systems Research Center
Advanced Micro Devices, Inc.
--
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: Borislav Petkov on 29 May 2010 05:50
From: Borislav Petkov <bp(a)amd64.org>
Date: Sat, May 29, 2010 at 05:31:19AM -0400

> But I don't know, I might just as well be missing something else.

And yes, the hotcpu_notifier() thingy is for core code only. Using that
won't allow using powernow-k8 as a module and might cause some ugliness
at module exit time.

Besides, it also screams:

WARNING: vmlinux.o(.text+0x3ba8d): Section mismatch in reference from the function __cpu_notify() to the variable .cpuinit.data:cpu_chain
The function __cpu_notify() references
the variable __cpuinitdata cpu_chain.
This is often because __cpu_notify lacks a __cpuinitdata
annotation or the annotation of cpu_chain is wrong.

Hmm...

--
Regards/Gruss,
Boris.

Operating Systems Research Center
Advanced Micro Devices, Inc.
--
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/
 | 
Pages: 1
Prev: Quickstart Button ACPI driver to serve PNP0C32 ACPI devices
Next: Email Admin Warning Alert !!!