wrongly marked __init/__initdata for CPU hotplug

wrongly marked __init/__initdata for CPU hotplug

[Ashok Raj]

Hi Andrew

attached patch is 2 more cases i found via running the reference_init.pl
script. These were easy to spot just knowing the file names. There is 
one another about init/main.c that i cant exactly zero in. (partly 
because i dont know how to interpret the data thats spewed out of the tool).

If there is a small example on how to co-related the data and find the culprit
would be real handy. 

Maybe Keith could help parse/give an example for me.

PS: There is another tool i noticed from Randy Dunlap that claims is fixed
for 2.6, and it give me couple more pci functions.

I still get the following:

[araj@araj-sfield linux-2.6.16-rc1-mm2]$ perl scripts/reference_init.pl
Finding objects, 1137 objects, ignoring 0 module(s)
Finding conglomerates, ignoring 139 conglomerate(s)
Scanning objects
Error: ./drivers/char/hw_random.o .data refers to 0000000000000028 R_X86_64_64       .init.text
Error: ./drivers/char/hw_random.o .data refers to 0000000000000050 R_X86_64_64       .init.text+0x00000000000000a0
Error: ./drivers/char/hw_random.o .data refers to 0000000000000078 R_X86_64_64       .init.text+0x000000000000017a
Error: ./init/main.o .text refers to 00000000000001dc R_X86_64_PC32     .init.data+0x000000000000015b
Done

-- 
Cheers,
Ashok Raj
- Open Source Technology Center


data/functions wrongly marked as __init with cpu hotplug.

Signed-off-by: Ashok Raj <ashok.raj@intel.com>
-----------------------------------------------
 arch/x86_64/kernel/mce.c      |    2 +-
 drivers/acpi/processor_idle.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Index: linux-2.6.16-rc1-mm2/arch/x86_64/kernel/mce.c
===================================================================
--- linux-2.6.16-rc1-mm2.orig/arch/x86_64/kernel/mce.c
+++ linux-2.6.16-rc1-mm2/arch/x86_64/kernel/mce.c
@@ -380,7 +380,7 @@ static void __cpuinit mce_cpu_features(s
  */
 void __cpuinit mcheck_init(struct cpuinfo_x86 *c)
 {
-	static cpumask_t mce_cpus __initdata = CPU_MASK_NONE;
+	static cpumask_t mce_cpus = CPU_MASK_NONE;
 
 	mce_cpu_quirks(c); 
 
Index: linux-2.6.16-rc1-mm2/drivers/acpi/processor_idle.c
===================================================================
--- linux-2.6.16-rc1-mm2.orig/drivers/acpi/processor_idle.c
+++ linux-2.6.16-rc1-mm2/drivers/acpi/processor_idle.c
@@ -94,7 +94,7 @@ static int set_max_cstate(struct dmi_sys
 	return 0;
 }
 
-static struct dmi_system_id __initdata processor_power_dmi_table[] = {
+static struct dmi_system_id __cpuinitdata processor_power_dmi_table[] = {
 	{ set_max_cstate, "IBM ThinkPad R40e", {
 	  DMI_MATCH(DMI_BIOS_VENDOR,"IBM"),
 	  DMI_MATCH(DMI_BIOS_VERSION,"1SET60WW")}, (void *)1},

Re: wrongly marked __init/__initdata for CPU hotplug

[Andrew Morton]

Ashok Raj <ashok.raj@intel.com> wrote:
>
>  void __cpuinit mcheck_init(struct cpuinfo_x86 *c)
>   {
>  -	static cpumask_t mce_cpus __initdata = CPU_MASK_NONE;
>  +	static cpumask_t mce_cpus = CPU_MASK_NONE;

Should that be __cpuinitdata?

Re: wrongly marked __init/__initdata for CPU hotplug

[Ashok Raj]

On Wed, Jan 25, 2006 at 12:13:17PM -0800, Andrew Morton wrote:
> Ashok Raj <ashok.raj@intel.com> wrote:
> >
> >  void __cpuinit mcheck_init(struct cpuinfo_x86 *c)
> >   {
> >  -	static cpumask_t mce_cpus __initdata = CPU_MASK_NONE;
> >  +	static cpumask_t mce_cpus = CPU_MASK_NONE;
> 
> Should that be __cpuinitdata?

Yes.. i sent an updated one to Andrew, but missed replying to group.

-- 
Cheers,
Ashok Raj
- Open Source Technology Center

Re: wrongly marked __init/__initdata for CPU hotplug

[Keith Owens]

Ashok Raj (on Wed, 25 Jan 2006 12:02:53 -0800) wrote:
>There is 
>one another about init/main.c that i cant exactly zero in. (partly 
>because i dont know how to interpret the data thats spewed out of the tool).
>
>If there is a small example on how to co-related the data and find the culprit
>would be real handy. 
>
>Maybe Keith could help parse/give an example for me.

# reference_init.pl
Error: init/main.o .text refers to 00000000000001dc R_X86_64_PC32     .init.data+0x000000000000015b

Grep for the combination of the offset (1dc) and the target section
type (.init.data), also list the function names.

# objdump -Sr init/main.o | egrep   '1dc: .*init.data|<.*>:'
0000000000000000 <rest_init>:
000000000000002a <run_init_process>:
0000000000000044 <init>:
                        1dc: R_X86_64_PC32      .init.data+0x15b
0000000000000000 <nosmp>:
0000000000000010 <maxcpus>:
000000000000002b <debug_kernel>:
000000000000003f <quiet_kernel>:
0000000000000053 <loglevel>:
000000000000006f <unknown_bootoption>:
0000000000000278 <init_setup>:
000000000000029f <rdinit_setup>:
00000000000002c6 <do_early_param>:
000000000000031d <parse_early_param>:
0000000000000369 <start_kernel>:
000000000000054f <initcall_debug_setup>:

So the reference is coming from the init function.

Next dump the symbols of the .init.data section.

# objdump -d -j .init.data init/main.o | fgrep '>:'

0000000000000000 <__setup_str_initcall_debug_setup>:
000000000000000f <__setup_str_rdinit_setup>:
0000000000000017 <__setup_str_init_setup>:
000000000000001d <__setup_str_loglevel>:
0000000000000027 <__setup_str_quiet_kernel>:
000000000000002d <__setup_str_debug_kernel>:
0000000000000033 <__setup_str_maxcpus>:
000000000000003c <__setup_str_nosmp>:
0000000000000060 <tmp_cmdline.23394>:
0000000000000160 <done.23393>:
0000000000000164 <initcall_debug>:

That would normally list the name of the offending variable, but
sometimes (like now), it does not.  There is no symbol listed for
.init.data+15b.  Very strange.

Try disassembling the code around the offending reference.

# objdump -Sr init/main.o | grep -B10 -A10   '1dc: .*init.data'
                        1b3: R_X86_64_PC32      sysctl_init+0xfffffffffffffffc
 1b7:   65 48 8b 04 25 10 00    mov    %gs:0x10,%rax
 1be:   00 00
 1c0:   8b a8 44 e0 ff ff       mov    0xffffffffffffe044(%rax),%ebp
 1c6:   48 c7 c3 00 00 00 00    mov    $0x0,%rbx
                        1c9: R_X86_64_32S       __initcall_start
 1cd:   48 81 fb 00 00 00 00    cmp    $0x0,%rbx
                        1d0: R_X86_64_32S       __initcall_end
 1d4:   0f 83 96 00 00 00       jae    270 <init+0x22c>
 1da:   83 3d 00 00 00 00 00    cmpl   $0x0,0(%rip)        # 1e1 <init+0x19d>
                        1dc: R_X86_64_PC32      .init.data+0x15b			<===
 1e1:   74 2e                   je     211 <init+0x1cd>
 1e3:   48 8b 33                mov    (%rbx),%rsi
 1e6:   48 c7 c7 00 00 00 00    mov    $0x0,%rdi
                        1e9: R_X86_64_32S       .rodata.str1.1+0x17b
 1ed:   31 c0                   xor    %eax,%eax
 1ef:   e8 00 00 00 00          callq  1f4 <init+0x1b0>
                        1f0: R_X86_64_PC32      printk+0xfffffffffffffffc
 1f4:   48 8b 33                mov    (%rbx),%rsi
 1f7:   48 c7 c7 00 00 00 00    mov    $0x0,%rdi
                        1fa: R_X86_64_32S       .rodata.str1.1+0x194

So the reference is after the use of __initcall_start/__initcall_end
and before the printk().  Get the cpp generated code.

# make init/main.i

View init/main.i, find __initcall_start/__initcall_end.  The only
references are in do_initcalls() which is defined as .init.text.  But
the objdump that listed function names showed the reference was from
init(), not from do_initcalls().

This is nasty.  init() calls do_basic_setup() which calls
do_initcalls().  init is normal text.  do_basic_setup and do_initcalls
are .init.text.  gcc has inlined do_basic_setup and do_initcalls into
init, even though they have different section attributes.  Naughty gcc.

This was using GCC: (GNU) 4.0.2 20050901 (prerelease) (SUSE Linux).
Log a gcc bug.  Not a good omen for the idea of letting gcc decide when
to inline!

Looking at the C code for do_initcalls(), the reference is obviously to
initcall_debug.  I am puzzled about why the objdump lists
.init.data+0x15b when initcall_debug is really at .init.data+0x164.

BTW, does anybody know why init() is not defined as __init?

Re: wrongly marked __init/__initdata for CPU hotplug

[Ashok Raj]

On Thu, Jan 26, 2006 at 01:35:17PM +1100, Keith Owens wrote:
> Ashok Raj (on Wed, 25 Jan 2006 12:02:53 -0800) wrote:
> >There is 
> >one another about init/main.c that i cant exactly zero in. (partly 
> >because i dont know how to interpret the data thats spewed out of the tool).
> >
> >If there is a small example on how to co-related the data and find the culprit
> >would be real handy. 
> >
> >Maybe Keith could help parse/give an example for me.
> 
> # reference_init.pl
> Error: init/main.o .text refers to 00000000000001dc R_X86_64_PC32     .init.data+0x000000000000015b
> 

Awesome annotation... thanks very much keith... 

Re: wrongly marked __init/__initdata for CPU hotplug

[Andi Kleen]

On Wed, Jan 25, 2006 at 12:02:53PM -0800, Ashok Raj wrote:
> Hi Andrew
> 
> attached patch is 2 more cases i found via running the reference_init.pl
> script. These were easy to spot just knowing the file names. There is 
> one another about init/main.c that i cant exactly zero in. (partly 
> because i dont know how to interpret the data thats spewed out of the tool).

I think that's the reference to __initcall_start / end 
That one is unavoidable, but not a bug.

-Andi

Re: wrongly marked __init/__initdata for CPU hotplug

[Andi Kleen]

> This is nasty.  init() calls do_basic_setup() which calls
> do_initcalls().  init is normal text.  do_basic_setup and do_initcalls
> are .init.text.  gcc has inlined do_basic_setup and do_initcalls into
> init, even though they have different section attributes.  Naughty gcc.
> 
> This was using GCC: (GNU) 4.0.2 20050901 (prerelease) (SUSE Linux).
> Log a gcc bug.  Not a good omen for the idea of letting gcc decide when
> to inline!

Someone should file a bug in the gcc bugzilla then I guess.
Can you do that or should I?

> 
> Looking at the C code for do_initcalls(), the reference is obviously to
> initcall_debug.  I am puzzled about why the objdump lists
> .init.data+0x15b when initcall_debug is really at .init.data+0x164.

Ah thanks - ok i mislooked then. Anyways, it's not a bug.
 
> BTW, does anybody know why init() is not defined as __init?

Because it would crash then after returning from free_initmem.

-Andi

Re: wrongly marked __init/__initdata for CPU hotplug

[Randy.Dunlap]

On 26 Jan 2006 05:26:03 +0100 Andi Kleen wrote:

> On Wed, Jan 25, 2006 at 12:02:53PM -0800, Ashok Raj wrote:
> > Hi Andrew
> > 
> > attached patch is 2 more cases i found via running the reference_init.pl
> > script. These were easy to spot just knowing the file names. There is 
> > one another about init/main.c that i cant exactly zero in. (partly 
> > because i dont know how to interpret the data thats spewed out of the tool).
> 
> I think that's the reference to __initcall_start / end 
> That one is unavoidable, but not a bug.

Yes, that's the conclusion that I came to also.

---
~Randy