Skip to contentSkip to conversation

x86_64: not set boot cpu in cpu_online_map at smp_prepare_boot_cpu

PATCH In review Yinghai Lu 2 participants 4 messages Nov 26, 2007 x86

lore
Share & tools
lore

Yinghai Lu

author
#0 lore raw

PATCH 1/1 x86_64: not set boot cpu in cpu_online_map at smp_prepare_boot_cpu

[PATCH] x86_64: not set boot cpu in cpu_online_map at smp_prepare_boot_cpu

in init/main.c boot_cpu_init() does that before

Signed-off-by Yinghai Lu <[email protected]>
arch/x86/kernel/smpboot_64.c +1 -1
index 500670c..966d124 100644
@@ -912,7 +912,7 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
 void __init smp_prepare_boot_cpu(void)
 {
 	int me = smp_processor_id();
-	cpu_set(me, cpu_online_map);
+	/* already set me in cpu_online_map in boot_cpu_init() */
 	cpu_set(me, cpu_callout_map);
 	per_cpu(cpu_state, me) = CPU_ONLINE;
 }
#1 lore raw
Quoted 11 lines · Yinghai Lu

[PATCH] x86_64: not set boot cpu in cpu_online_map at smp_prepare_boot_cpu

in init/main.c boot_cpu_init() does that before

Signed-off-by Yinghai Lu <[email protected]>
arch/x86/kernel/smpboot_64.c +0 -0
index 500670c..966d124 100644

arch/x86/kernel/smpboot_64.c void __init smp_prepare_cpus(unsigned int max_cpus) · line 918

Quoted 8 lines
arch/x86/kernel/smpboot_64.c +1 -1
@@ -912,7 +912,7 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
 void __init smp_prepare_boot_cpu(void)
 {
 	int me = smp_processor_id();
-	cpu_set(me, cpu_online_map);
+	/* already set me in cpu_online_map in boot_cpu_init() */
 	cpu_set(me, cpu_callout_map);
 	per_cpu(cpu_state, me) = CPU_ONLINE;
 }

This ordering can be tricky wrt CPU hotplug. Are you sure you are not breaking CPU hotplug? AFAIK, x86_64 has that right and the 32-bit code had it wrong.

Zach

author
#2 lore raw
Quoted 17 lines · Zachary Amsden
Quoted 16 lines · Yinghai Lu

[PATCH] x86_64: not set boot cpu in cpu_online_map at smp_prepare_boot_cpu

in init/main.c boot_cpu_init() does that before

Signed-off-by Yinghai Lu <[email protected]>
arch/x86/kernel/smpboot_64.c +1 -1
index 500670c..966d124 100644
@@ -912,7 +912,7 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
 void __init smp_prepare_boot_cpu(void)
 {
 	int me = smp_processor_id();
-	cpu_set(me, cpu_online_map);
+	/* already set me in cpu_online_map in boot_cpu_init() */
Quoted 8 lines

arch/x86/kernel/smpboot_64.c void __init smp_prepare_cpus(unsigned int max_cpus) · line 918

Quoted 3 lines
arch/x86/kernel/smpboot_64.c +0 -0
 	cpu_set(me, cpu_callout_map);
 	per_cpu(cpu_state, me) = CPU_ONLINE;
 }

This ordering can be tricky wrt CPU hotplug. Are you sure you are not breaking CPU hotplug? AFAIK, x86_64 has that right and the 32-bit code had it wrong.

CPU hot plug path will call smp_prepare_boot_cpu?
it is using __init instead of __cpuinit?

YH

#3 lore raw
Quoted 20 lines · Yinghai Lu
Quoted 10 lines
Quoted 10 lines
arch/x86/kernel/smpboot_64.c +1 -1
index 500670c..966d124 100644
@@ -912,7 +912,7 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
 void __init smp_prepare_boot_cpu(void)
 {
 	int me = smp_processor_id();
-	cpu_set(me, cpu_online_map);
+	/* already set me in cpu_online_map in boot_cpu_init() */
Quoted 8 lines

arch/x86/kernel/smpboot_64.c void __init smp_prepare_cpus(unsigned int max_cpus) · line 918

Quoted 3 lines
arch/x86/kernel/smpboot_64.c +0 -0
 	cpu_set(me, cpu_callout_map);
 	per_cpu(cpu_state, me) = CPU_ONLINE;
 }

This ordering can be tricky wrt CPU hotplug. Are you sure you are not breaking CPU hotplug? AFAIK, x86_64 has that right and the 32-bit code had it wrong.

Quoted 2 lines

CPU hot plug path will call smp_prepare_boot_cpu?
it is using __init instead of __cpuinit?

No, but moving the place where cpu is moved into online map might break hotplug. In any case, I don't see anything wrong with your change now that I look closer.

Zach