[PATCH] x86: setup_trampoline() - fix section mismatch warning

[PATCH] x86: setup_trampoline() - fix section mismatch warning

[Jacek Luczak]

[-- Attachment #1: Type: text/plain, Size: 1510 bytes --]

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.

---

diff --git a/arch/x86/kernel/trampoline_64.S b/arch/x86/kernel/trampoline_64.S
index 4aedd0b..2a07e67 100644
--- a/arch/x86/kernel/trampoline_64.S
+++ b/arch/x86/kernel/trampoline_64.S
@@ -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


[-- Attachment #2: setup_trampoline-fix_section_mismatch.patch --]
[-- Type: text/plain, Size: 440 bytes --]

diff --git a/arch/x86/kernel/trampoline_64.S b/arch/x86/kernel/trampoline_64.S
index 4aedd0b..2a07e67 100644
--- a/arch/x86/kernel/trampoline_64.S
+++ b/arch/x86/kernel/trampoline_64.S
@@ -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

Re: [PATCH] x86: setup_trampoline() - fix section mismatch warning

[Ingo Molnar]

* Jacek Luczak <difrost.kernel@gmail.com> wrote:

> 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.

> 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

Re: [PATCH] x86: setup_trampoline() - fix section mismatch warning

[Sam Ravnborg]

Hi Jacek

On Thu, Apr 10, 2008 at 09:16:41PM +0200, Jacek Luczak wrote:
> 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.
> 
> ---
> 
> diff --git a/arch/x86/kernel/trampoline_64.S b/arch/x86/kernel/trampoline_64.S
> index 4aedd0b..2a07e67 100644
> --- a/arch/x86/kernel/trampoline_64.S
> +++ b/arch/x86/kernel/trampoline_64.S
> @@ -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):

> diff --git a/arch/x86/kernel/trampoline_64.S b/arch/x86/kernel/trampoline_64.S
> index 4aedd0b..2a07e67 100644
> --- a/arch/x86/kernel/trampoline_64.S
> +++ b/arch/x86/kernel/trampoline_64.S
> @@ -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

Re: [PATCH] x86: setup_trampoline() - fix section mismatch warning

[Jacek Luczak]

Hi Sam,

Sam Ravnborg pisze:
[!snip]
>> diff --git a/arch/x86/kernel/trampoline_64.S b/arch/x86/kernel/trampoline_64.S
>> index 4aedd0b..2a07e67 100644
>> --- a/arch/x86/kernel/trampoline_64.S
>> +++ b/arch/x86/kernel/trampoline_64.S
>> @@ -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

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?

> 
> Like this: (hand edited):
> 
>> diff --git a/arch/x86/kernel/trampoline_64.S b/arch/x86/kernel/trampoline_64.S
>> index 4aedd0b..2a07e67 100644
>> --- a/arch/x86/kernel/trampoline_64.S
>> +++ b/arch/x86/kernel/trampoline_64.S
>> @@ -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.

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

-Jacek

Re: [PATCH] x86: setup_trampoline() - fix section mismatch warning

[Sam Ravnborg]

On Sat, Apr 12, 2008 at 01:00:32PM +0200, Jacek Luczak wrote:
> 
> Hi Sam,
> 
> Sam Ravnborg pisze:
> [!snip]
> >> diff --git a/arch/x86/kernel/trampoline_64.S b/arch/x86/kernel/trampoline_64.S
> >> index 4aedd0b..2a07e67 100644
> >> --- a/arch/x86/kernel/trampoline_64.S
> >> +++ b/arch/x86/kernel/trampoline_64.S
> >> @@ -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
> 
> 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