Re: [PATCH] i386: fix section mismatches for init code with !HOTPLUG_CPU

[Robert Richter <robert.richter@amd.com>]

On 18.08.09 16:41:33, Jan Beulich wrote:
> Commit 0e83815be719d3391bf5ea24b7fe696c07dbd417 changed the section the
> initial_code variable gets allocated in, in an attempt to address a
> section conflict warning. This, however created a new section conflict
> when building without HOTPLUG_CPU. The apparently only (reasonable) way
> to address this is to always use __REFDATA.

Hmm, not sure if I am missing something, but my patch actually
shouldn't have changed the !HOTPLUG_CPU case:

diff --git a/arch/x86/kernel/head_32.S b/arch/x86/kernel/head_32.S
index 8663afb..0d98a01 100644
--- a/arch/x86/kernel/head_32.S
+++ b/arch/x86/kernel/head_32.S
@@ -602,7 +602,11 @@ ignore_int:
 #endif
        iret
 
-.section .cpuinit.data,"wa"
+#ifndef CONFIG_HOTPLUG_CPU
+       __CPUINITDATA
+#else
+       __REFDATA
+#endif
 .align 4
 ENTRY(initial_code)
        .long i386_start_kernel

I compiled -rc6 with HOTPLUG_CPU disabled and got with and without my
patch the following warning:

WARNING: vmlinux.o(.cpuinit.data+0x0): Section mismatch in reference from the variable initial_code to the function .init.text:i386_start_kernel()
The variable __cpuinitdata initial_code references
a function __init i386_start_kernel().
If i386_start_kernel is only used by initial_code then
annotate i386_start_kernel with a matching annotation.

So, the warning was there before.

The problem with that warning is that i386_start_kernel() is in __init
that is calling a chain of other __init functions. Thomas moved
initial_code to __CPUINITDATA (commit 583323b9), and thus, also
i386_start_kernel() should be __cpuinit. But initial_code seems to be
overwritten before with __cpuinit start_secondary() in
smpboot.c. Weird ...

>
> Once at it, also fix a second section mismatch when not using
> HOTPLUG_CPU.
>
> Signed-off-by: Jan Beulich <jbeulich@novell.com>
> Cc: Robert Richter <robert.richter@amd.com>
>
> ---
>  arch/x86/kernel/head_32.S |    8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
>
> --- linux-2.6.31-rc6/arch/x86/kernel/head_32.S        2009-08-18 15:31:16.000000000 +0200
> +++ 2.6.31-rc6-i386-head-sections/arch/x86/kernel/head_32.S   2009-08-18 10:38:31.000000000 +0200
> @@ -261,9 +261,7 @@ page_pde_offset = (__PAGE_OFFSET >> 20);
>   * which will be freed later
>   */
>
> -#ifndef CONFIG_HOTPLUG_CPU
> -.section .init.text,"ax",@progbits
> -#endif
> +__CPUINIT

This was here before (HOTPLUG_CPU case):

.section .text.head,"ax",@progbits

In the case a cpu is enabled again, do __cpuinit sections still exist?

-Robert

>
>  #ifdef CONFIG_SMP
>  ENTRY(startup_32_smp)
> @@ -602,11 +600,7 @@ ignore_int:
>  #endif
>       iret
>
> -#ifndef CONFIG_HOTPLUG_CPU
> -     __CPUINITDATA
> -#else
>       __REFDATA
> -#endif
>  .align 4
>  ENTRY(initial_code)
>       .long i386_start_kernel
>
>
>
>

-- 
Advanced Micro Devices, Inc.
Operating System Research Center
email: robert.richter@amd.com