[PATCH] i386 !NUMA node_to_cpumask broken in early boot

[PATCH] i386 !NUMA node_to_cpumask broken in early boot

[Zwane Mwaikambo]

node_to_cpumask on non NUMA systems is broken if used before all the 
processors have been brought up as it returns cpu_online_map, as opposed 
to NUMA i386 systems which does it earlier than AP bringup. So return 
which processors responded via cpu_present_map and switch to 
cpu_online_map during normal runtime. This was found whilst testing a 
patch which does node dependent work before APs have all gone online.

Signed-off-by: Zwane Mwaikambo <zwane@arm.linux.org.uk>

Index: linux-2.6.13-rc5-mm1/include/asm-i386/topology.h
===================================================================
RCS file: /home/cvsroot/linux-2.6.13-rc5-mm1/include/asm-i386/topology.h,v
retrieving revision 1.1.1.1
diff -u -p -B -r1.1.1.1 topology.h
--- linux-2.6.13-rc5-mm1/include/asm-i386/topology.h	7 Aug 2005 21:38:36 -0000	1.1.1.1
+++ linux-2.6.13-rc5-mm1/include/asm-i386/topology.h	19 Aug 2005 01:35:07 -0000
@@ -99,6 +99,15 @@ extern unsigned long node_remap_size[];
  * above macros here.
  */
 
+#define node_to_cpumask(node)	_node_to_cpumask(node)
+static inline cpumask_t _node_to_cpumask(int node)
+{
+	if (unlikely(system_state == SYSTEM_BOOTING))
+		return cpu_present_map;
+
+	return cpu_online_map;
+}
+
 #include <asm-generic/topology.h>
 
 #endif /* CONFIG_NUMA */

Re: [PATCH] i386 !NUMA node_to_cpumask broken in early boot

[Andi Kleen]

On Thu, Aug 18, 2005 at 08:07:53PM -0600, Zwane Mwaikambo wrote:
> node_to_cpumask on non NUMA systems is broken if used before all the 
> processors have been brought up as it returns cpu_online_map, as opposed 
> to NUMA i386 systems which does it earlier than AP bringup. So return 
> which processors responded via cpu_present_map and switch to 
> cpu_online_map during normal runtime. This was found whilst testing a 
> patch which does node dependent work before APs have all gone online.

Very ugly and will probably cause code bloat.

If anything define a special early_node_to ... function for this.

-Andi

Re: [PATCH] i386 !NUMA node_to_cpumask broken in early boot

[Zwane Mwaikambo]

On Fri, 19 Aug 2005, Andi Kleen wrote:

> On Thu, Aug 18, 2005 at 08:07:53PM -0600, Zwane Mwaikambo wrote:
> > node_to_cpumask on non NUMA systems is broken if used before all the 
> > processors have been brought up as it returns cpu_online_map, as opposed 
> > to NUMA i386 systems which does it earlier than AP bringup. So return 
> > which processors responded via cpu_present_map and switch to 
> > cpu_online_map during normal runtime. This was found whilst testing a 
> > patch which does node dependent work before APs have all gone online.
> 
> Very ugly and will probably cause code bloat.
> 
> If anything define a special early_node_to ... function for this.

Thanks for the feedback, ugly indeed, i was really trying to avoid adding 
a new API function or extra cpu_* variables. Ok, here is an 
early_node_to_cpumask instead.

Signed-off-by: Zwane Mwaikambo <zwane@arm.linux.org.uk>

Index: linux-2.6.13-rc5-mm1/include/asm-i386/topology.h
===================================================================
RCS file: /home/cvsroot/linux-2.6.13-rc5-mm1/include/asm-i386/topology.h,v
retrieving revision 1.1.1.1
diff -u -p -B -r1.1.1.1 topology.h
--- linux-2.6.13-rc5-mm1/include/asm-i386/topology.h	7 Aug 2005 21:38:36 -0000	1.1.1.1
+++ linux-2.6.13-rc5-mm1/include/asm-i386/topology.h	19 Aug 2005 02:47:10 -0000
@@ -92,6 +92,7 @@ extern unsigned long node_end_pfn[];
 extern unsigned long node_remap_size[];
 
 #define node_has_online_mem(nid) (node_start_pfn[nid] != node_end_pfn[nid])
+#define early_node_to_cpumask(nid)	node_to_cpumask(nid)
 
 #else /* !CONFIG_NUMA */
 /*
@@ -99,6 +100,14 @@ extern unsigned long node_remap_size[];
  * above macros here.
  */
 
+static inline cpumask_t early_node_to_cpumask(int nid)
+{
+	if (unlikely(system_state == SYSTEM_BOOTING))
+		return cpu_present_map;
+
+	return cpu_online_map;
+}
+
 #include <asm-generic/topology.h>
 
 #endif /* CONFIG_NUMA */

Re: [PATCH] i386 !NUMA node_to_cpumask broken in early boot

[Andi Kleen]

> Thanks for the feedback, ugly indeed, i was really trying to avoid adding 
> a new API function or extra cpu_* variables. Ok, here is an 
> early_node_to_cpumask instead.

Thinking about it again it's most likely broken with CPU hotplug anyways
whatever you're doing. So how does your code handle adding new 
CPUs?  If it does can the normal CPU bootup be handled in the 
same way. Then this wouldn't be needed at all.

-Andi

Re: [PATCH] i386 !NUMA node_to_cpumask broken in early boot

[Zwane Mwaikambo]

On Fri, 19 Aug 2005, Andi Kleen wrote:

> > Thanks for the feedback, ugly indeed, i was really trying to avoid adding 
> > a new API function or extra cpu_* variables. Ok, here is an 
> > early_node_to_cpumask instead.
> 
> Thinking about it again it's most likely broken with CPU hotplug anyways
> whatever you're doing. So how does your code handle adding new 
> CPUs?  If it does can the normal CPU bootup be handled in the 
> same way. Then this wouldn't be needed at all.

The code is populating IDTs before APs are online, so for a given set of 
processors i would want to install new IDT entries like so;

node_set_intr_gate()
{
	cpumask_t mask = node_to_cpumask(node);
	for_each_cpu_mask(cpu, mask)
		set_intr_gate(per_cpu_ptr(cpu_idt_table, cpu)....
	....
}

Which before resulted in only setting the IDT entry for the BSP.

During boot, i prepare all processor IDTs so they are always 
ready for processors coming online and should take care of hotplug cpu 
too. The only problem with normal bootup is that the node_to_cpumask is
unreliable.

	Zwane