Some section mismatch in acpi_processor_power_init on ia64 build

Some section mismatch in acpi_processor_power_init on ia64 build

[Luck, Tony]

I've only just noticed these warnings when building ia64 !SMP or
!HOTPLUG_CPU
kernels:

WARNING: drivers/acpi/processor.o - Section mismatch: reference to
.init.data: from .text between 'acpi_processor_power_init' (at offset
0x5040) and 'acpi_processor_power_exit'
WARNING: drivers/acpi/processor.o - Section mismatch: reference to
.init.data: from .text between 'acpi_processor_power_init' (at offset
0x5050) and 'acpi_processor_power_exit'

According to git bisect, they began with Matt Domsch's "ia64: use i386
dmi_scan.c"
patch (commit 3ed3bce8), but it appears that the real issue may be
further back when
Ashok Raj marked processor_power_dmi_table as __cpuinitdata in 7ded5689
with a
cryptic comment by AK (Andi Kleen?):
  /* Actually this shouldn't be __cpuinitdata, would be better to fix
the
     callers to only run once -AK */

-Tony

Re: Some section mismatch in acpi_processor_power_init on ia64 build

[Andi Kleen]

On Wednesday 29 March 2006 01:09, Luck, Tony wrote:
> I've only just noticed these warnings when building ia64 !SMP or
> !HOTPLUG_CPU
> kernels:
> 
> WARNING: drivers/acpi/processor.o - Section mismatch: reference to
> .init.data: from .text between 'acpi_processor_power_init' (at offset
> 0x5040) and 'acpi_processor_power_exit'
> WARNING: drivers/acpi/processor.o - Section mismatch: reference to
> .init.data: from .text between 'acpi_processor_power_init' (at offset
> 0x5050) and 'acpi_processor_power_exit'

These functions need to be marked __cpuinit I guess. I doubt they
run without new CPUs.

> 
> According to git bisect, they began with Matt Domsch's "ia64: use i386
> dmi_scan.c"
> patch (commit 3ed3bce8), but it appears that the real issue may be
> further back when
> Ashok Raj marked processor_power_dmi_table as __cpuinitdata in 7ded5689
> with a
> cryptic comment by AK (Andi Kleen?):
>   /* Actually this shouldn't be __cpuinitdata, would be better to fix
> the
>      callers to only run once -AK */

Yes that's me. What is cryptic?

-Andi

Re: Some section mismatch in acpi_processor_power_init on ia64 build

[Ashok Raj]

On Tue, Mar 28, 2006 at 03:09:36PM -0800, Luck, Tony wrote:
> I've only just noticed these warnings when building ia64 !SMP or
> !HOTPLUG_CPU
> kernels:
> 
> WARNING: drivers/acpi/processor.o - Section mismatch: reference to
> .init.data: from .text between 'acpi_processor_power_init' (at offset
> 0x5040) and 'acpi_processor_power_exit'
> WARNING: drivers/acpi/processor.o - Section mismatch: reference to
> .init.data: from .text between 'acpi_processor_power_init' (at offset
> 0x5050) and 'acpi_processor_power_exit'
> 
> According to git bisect, they began with Matt Domsch's "ia64: use i386
> dmi_scan.c"
> patch (commit 3ed3bce8), but it appears that the real issue may be
> further back when
> Ashok Raj marked processor_power_dmi_table as __cpuinitdata in 7ded5689
> with a
> cryptic comment by AK (Andi Kleen?):
>   /* Actually this shouldn't be __cpuinitdata, would be better to fix
> the
>      callers to only run once -AK */
> 
> -Tony

Humm.. originally they were marked as __initdata, but for CPU hotplug we call it
when processor gets hot plugged. So i changed it to __cpuinitdata so that when we use
cpu hotplug they stay resident.

the only reference is to that table is from acpi_processor_power_init(), that gets called 
currently only from acpi_processor_start().

the code is either compiled in kernel (which means it will be all thrown after free
init mem if !HOTPLUG_CPU) or if this is a module code, then __initdata/cpuinit doesnt 
make a difference.

possibly acpi_processor_start(), acpi_processor_power_init() etc should also be
__cpuinit, which would make the warning go away. 

Are there any others i missed Andi? maybe this is a general watch out comment, but 
he knows better.

-- 
Cheers,
Ashok Raj
- Open Source Technology Center

Re: Some section mismatch in acpi_processor_power_init on ia64 build

[Luck, Tony]

On Wed, Mar 29, 2006 at 02:00:55AM +0200, Andi Kleen wrote:
> > WARNING: drivers/acpi/processor.o - Section mismatch: reference to
> > .init.data: from .text between 'acpi_processor_power_init' (at offset
> > 0x5040) and 'acpi_processor_power_exit'
> > WARNING: drivers/acpi/processor.o - Section mismatch: reference to
> > .init.data: from .text between 'acpi_processor_power_init' (at offset
> > 0x5050) and 'acpi_processor_power_exit'
> 
> These functions need to be marked __cpuinit I guess. I doubt they
> run without new CPUs.

Marking acpi_processor_power_init() as __cpuinit produced a complaint about
a section mismatch in acpi_processor_start().  Marking that __cpuinit fixed
things for me (patch at end of this e-mail, only compile tested on one ia64
config).
> >   /* Actually this shouldn't be __cpuinitdata, would be better to fix the
> >      callers to only run once -AK */
> 
> Yes that's me. What is cryptic?

Perhaps it wasn't the comment that was baffling me, I was just generally
confused at this point.


diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
index 713b763..2dedc59 100644
--- a/drivers/acpi/processor_core.c
+++ b/drivers/acpi/processor_core.c
@@ -533,7 +533,7 @@ #endif
 
 static void *processor_device_array[NR_CPUS];
 
-static int acpi_processor_start(struct acpi_device *device)
+static int __cpuinit acpi_processor_start(struct acpi_device *device)
 {
 	int result = 0;
 	acpi_status status = AE_OK;
diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index 80fa434..106d6f3 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -1077,7 +1077,7 @@ static struct file_operations acpi_proce
 	.release = single_release,
 };
 
-int acpi_processor_power_init(struct acpi_processor *pr,
+int __cpuinit acpi_processor_power_init(struct acpi_processor *pr,
 			      struct acpi_device *device)
 {
 	acpi_status status = 0;