* [PATCH] x86: setup_trampoline() - fix section mismatch warning
@ 2008-04-10 19:16 Jacek Luczak
2008-04-11 9:03 ` Ingo Molnar
2008-04-11 17:50 ` Sam Ravnborg
0 siblings, 2 replies; 5+ messages in thread
From: Jacek Luczak @ 2008-04-10 19:16 UTC (permalink / raw
To: Ingo Molnar, LKML, tglx
[-- 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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] x86: setup_trampoline() - fix section mismatch warning
2008-04-10 19:16 [PATCH] x86: setup_trampoline() - fix section mismatch warning Jacek Luczak
@ 2008-04-11 9:03 ` Ingo Molnar
2008-04-11 17:50 ` Sam Ravnborg
1 sibling, 0 replies; 5+ messages in thread
From: Ingo Molnar @ 2008-04-11 9:03 UTC (permalink / raw
To: Jacek Luczak; +Cc: LKML, tglx
* 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] x86: setup_trampoline() - fix section mismatch warning
2008-04-10 19:16 [PATCH] x86: setup_trampoline() - fix section mismatch warning Jacek Luczak
2008-04-11 9:03 ` Ingo Molnar
@ 2008-04-11 17:50 ` Sam Ravnborg
2008-04-12 11:00 ` Jacek Luczak
1 sibling, 1 reply; 5+ messages in thread
From: Sam Ravnborg @ 2008-04-11 17:50 UTC (permalink / raw
To: Jacek Luczak; +Cc: Ingo Molnar, LKML, tglx
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] x86: setup_trampoline() - fix section mismatch warning
2008-04-11 17:50 ` Sam Ravnborg
@ 2008-04-12 11:00 ` Jacek Luczak
2008-04-12 16:56 ` Sam Ravnborg
0 siblings, 1 reply; 5+ messages in thread
From: Jacek Luczak @ 2008-04-12 11:00 UTC (permalink / raw
To: Sam Ravnborg; +Cc: Ingo Molnar, LKML, tglx
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] x86: setup_trampoline() - fix section mismatch warning
2008-04-12 11:00 ` Jacek Luczak
@ 2008-04-12 16:56 ` Sam Ravnborg
0 siblings, 0 replies; 5+ messages in thread
From: Sam Ravnborg @ 2008-04-12 16:56 UTC (permalink / raw
To: Jacek Luczak; +Cc: Ingo Molnar, LKML, tglx
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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2008-04-12 16:56 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-04-10 19:16 [PATCH] x86: setup_trampoline() - fix section mismatch warning Jacek Luczak
2008-04-11 9:03 ` Ingo Molnar
2008-04-11 17:50 ` Sam Ravnborg
2008-04-12 11:00 ` Jacek Luczak
2008-04-12 16:56 ` Sam Ravnborg
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).