[PATCH] powernow-k8: Fix section mismatch

[PATCH] powernow-k8: Fix section mismatch

[Borislav Petkov]

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@kernel.org>
Reported-by: H. Peter Anvin <hpa@zytor.com>
Signed-off-by: Borislav Petkov <borislav.petkov@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,
 };
 
-- 
1.7.0


-- 
Regards/Gruss,
Boris.

Operating Systems Research Center
Advanced Micro Devices, Inc.

[tip:x86/urgent] x86, k8: Fix section mismatch for powernowk8_exit()

[tip-bot for Borislav Petkov]

Commit-ID:  fe501f1e89cd460793152f500bf25d81d463515b
Gitweb:     http://git.kernel.org/tip/fe501f1e89cd460793152f500bf25d81d463515b
Author:     Borislav Petkov <borislav.petkov@amd.com>
AuthorDate: Tue, 25 May 2010 15:28:58 +0000
Committer:  H. Peter Anvin <hpa@linux.intel.com>
CommitDate: Tue, 25 May 2010 15:42:21 -0700

x86, k8: Fix section mismatch for powernowk8_exit()

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@kernel.org>
Reported-by: H. Peter Anvin <hpa@zytor.com>
Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
LKML-Reference: <20100525152858.GA24836@aftab>
Signed-off-by: H. Peter Anvin <hpa@linux.intel.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,
 };
 

Re: [PATCH] powernow-k8: Fix section mismatch

[Andrew Morton]

On Tue, 25 May 2010 17:28:58 +0200
Borislav Petkov <borislav.petkov@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@kernel.org>
> Reported-by: H. Peter Anvin <hpa@zytor.com>
> Signed-off-by: Borislav Petkov <borislav.petkov@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.

Re: [PATCH] powernow-k8: Fix section mismatch

[Borislav Petkov]

From: Andrew Morton <akpm@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@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@kernel.org>
> > Reported-by: H. Peter Anvin <hpa@zytor.com>
> > Signed-off-by: Borislav Petkov <borislav.petkov@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.

Re: [PATCH] powernow-k8: Fix section mismatch

[Borislav Petkov]

From: Borislav Petkov <bp@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.