Skip to contentSkip to conversation

x86: setup_trampoline() - fix section mismatch warning

PATCH In review Jacek Luczak 3 participants 5 messages Apr 10, 2008 – Apr 12, 2008 x86

lore
Share & tools
lore

Jacek Luczak

author
#0 lore raw

PATCH 1/1 x86: setup_trampoline() - fix section mismatch warning

Hi Ingo,

this patch fixes section mismatch warnings (on x86_64 host) in setup_trampoline(), which was referencing __initdata variables trampoline_data and trampoline_end.

Patch is against x86/latest.

Warning messages:
WARNING: arch/x86/kernel/built-in.o(.cpuinit.text+0x2b6a): Section mismatch in reference from the function setup_trampoline() to the variable .init.data:trampoline_data
The function __cpuinit setup_trampoline() references
a variable __initdata trampoline_data.
If trampoline_data is only used by setup_trampoline then
annotate trampoline_data with a matching annotation.

WARNING: arch/x86/kernel/built-in.o(.cpuinit.text+0x2b71): Section mismatch in reference from the function setup_trampoline() to the variable .init.data:trampoline_end
The function __cpuinit setup_trampoline() references
a variable __initdata trampoline_end.
If trampoline_end is only used by setup_trampoline then
annotate trampoline_end with a matching annotation.

-Jacek

PS: Hope that my email client won't break patch this time. In case it does - patch attached.

---

arch/x86/kernel/trampoline_64.S +1 -1
index 4aedd0b..2a07e67 100644
@@ -32,7 +32,7 @@
 
 /* We can free up trampoline after bootup if cpu hotplug is not supported. */
 #ifndef CONFIG_HOTPLUG_CPU
-.section .init.data, "aw", @progbits
+.section .cpuinit.data, "aw", @progbits
 #else
 .section .rodata, "a", @progbits
 #endif
arch/x86/kernel/trampoline_64.S +1 -1
index 4aedd0b..2a07e67 100644
@@ -32,7 +32,7 @@
 
 /* We can free up trampoline after bootup if cpu hotplug is not supported. */
 #ifndef CONFIG_HOTPLUG_CPU
-.section .init.data, "aw", @progbits
+.section .cpuinit.data, "aw", @progbits
 #else
 .section .rodata, "a", @progbits
 #endif
#1 lore raw
Quoted 7 lines · Jacek Luczak

Hi Ingo,

this patch fixes section mismatch warnings (on x86_64 host) in setup_trampoline(), which was referencing __initdata variables trampoline_data and trampoline_end.

Patch is against x86/latest.

thanks Jacek, applied.

Quoted 2 lines

PS: Hope that my email client won't break patch this time. In case it does - patch attached.

it worked just fine. (another minor nit: in future patches please include a Signed-off-by line.)

Ingo

#2 lore raw
In reply to Jacek Luczak

Hi Jacek

Quoted 33 lines · Jacek Luczak

Hi Ingo,

this patch fixes section mismatch warnings (on x86_64 host) in setup_trampoline(), which was referencing __initdata variables trampoline_data and trampoline_end.

Patch is against x86/latest.

Warning messages:
WARNING: arch/x86/kernel/built-in.o(.cpuinit.text+0x2b6a): Section mismatch in reference from the function setup_trampoline() to the variable .init.data:trampoline_data
The function __cpuinit setup_trampoline() references
a variable __initdata trampoline_data.
If trampoline_data is only used by setup_trampoline then
annotate trampoline_data with a matching annotation.

WARNING: arch/x86/kernel/built-in.o(.cpuinit.text+0x2b71): Section mismatch in reference from the function setup_trampoline() to the variable .init.data:trampoline_end
The function __cpuinit setup_trampoline() references
a variable __initdata trampoline_end.
If trampoline_end is only used by setup_trampoline then
annotate trampoline_end with a matching annotation.

-Jacek

PS: Hope that my email client won't break patch this time. In case it does - patch attached.

---

arch/x86/kernel/trampoline_64.S +0 -0
index 4aedd0b..2a07e67 100644

arch/x86/kernel/trampoline_64.S line 38

Quoted 8 lines
arch/x86/kernel/trampoline_64.S +1 -1
@@ -32,7 +32,7 @@
 
 /* We can free up trampoline after bootup if cpu hotplug is not supported. */
 #ifndef CONFIG_HOTPLUG_CPU
-.section .init.data, "aw", @progbits
+.section .cpuinit.data, "aw", @progbits
 #else
 .section .rodata, "a", @progbits
 #endif

The correct fix would be to drop the
preprocessor directves and use __CPUINITDATA

Like this: (hand edited):

arch/x86/kernel/trampoline_64.S line 35

Quoted 13 lines
arch/x86/kernel/trampoline_64.S +1 -4
index 4aedd0b..2a07e67 100644
@@ -32,7 +32,7 @@
 
 /* We can free up trampoline after bootup if cpu hotplug is not supported. */
 #ifndef CONFIG_HOTPLUG_CPU
-.section .init.data, "aw", @progbits
-#else
-.section .rodata, "a", @progbits
-#endif
+__CPUINITDATA

If your testing confirms this then please apply similar fix to the _32.S version of the file too.

Nore that this looses the @progbits annotation but it should be OK to do so. I may later add it to init.h.

Sam

author
#3 lore raw

Hi Sam,

Sam Ravnborg pisze:
[!snip]

Quoted 6 lines
Quoted 6 lines
arch/x86/kernel/trampoline_64.S +0 -0
index 4aedd0b..2a07e67 100644
@@ -32,7 +32,7 @@
Quoted 10 lines

arch/x86/kernel/trampoline_64.S line 38

Quoted 7 lines
arch/x86/kernel/trampoline_64.S +1 -1
 /* We can free up trampoline after bootup if cpu hotplug is not supported. */
 #ifndef CONFIG_HOTPLUG_CPU
-.section .init.data, "aw", @progbits
+.section .cpuinit.data, "aw", @progbits
 #else
 .section .rodata, "a", @progbits
 #endif

The correct fix would be to drop the
preprocessor directves and use __CPUINITDATA

Hmm...IMO this should stay in that form at least now.
I will make some tests with __CPUINITDATA (look into objects, etc.) and later on we can switch. Is it OK for you?

Quoted 9 lines

Like this: (hand edited):

Quoted 6 lines
arch/x86/kernel/trampoline_64.S +0 -0
index 4aedd0b..2a07e67 100644
@@ -32,7 +32,7 @@
Quoted 10 lines

arch/x86/kernel/trampoline_64.S line 35

Quoted 7 lines
arch/x86/kernel/trampoline_64.S +1 -4
 /* We can free up trampoline after bootup if cpu hotplug is not supported. */
 #ifndef CONFIG_HOTPLUG_CPU
-.section .init.data, "aw", @progbits
-#else
-.section .rodata, "a", @progbits
-#endif
+__CPUINITDATA

If your testing confirms this then please apply similar fix to the _32.S version of the file too.

Yep, I'm now working on 32bit stuff.

-Jacek

#4 lore raw
Quoted 22 lines · Jacek Luczak

Hi Sam,

Sam Ravnborg pisze:
[!snip]

Quoted 6 lines
Quoted 6 lines
arch/x86/kernel/trampoline_64.S +0 -0
index 4aedd0b..2a07e67 100644
@@ -32,7 +32,7 @@
Quoted 10 lines

arch/x86/kernel/trampoline_64.S line 38

Quoted 7 lines
arch/x86/kernel/trampoline_64.S +1 -1
 /* We can free up trampoline after bootup if cpu hotplug is not supported. */
 #ifndef CONFIG_HOTPLUG_CPU
-.section .init.data, "aw", @progbits
+.section .cpuinit.data, "aw", @progbits
 #else
 .section .rodata, "a", @progbits
 #endif

The correct fix would be to drop the
preprocessor directves and use __CPUINITDATA

Quoted 3 lines

Hmm...IMO this should stay in that form at least now.
I will make some tests with __CPUINITDATA (look into objects, etc.) and later on we can switch. Is it OK for you?

I should maybe give you a little more background. The __cpuinitdata section is properly discarded after init has completed in case CPUHOTPLUG is not needed. But in case it is needed then the section is kept.

So the only issue with introducing the patch as I suggest is that we loose the readonly. We need to introduce __CPUINITCONST to cover that case.
But loosing readonly is not a big deal here.

If you insist on the readonly part then unconditionally use the section named .cpuinit.rodata
But the preprocessor directive should in any case be dropped as it is of no real use today.

Sam