[Linux-ia64] Re: [PATCH] topology for ia64

[Linux-ia64] Re: [PATCH] topology for ia64

[David Mosberger]

>>>>> On Sat, 5 Oct 2002 19:04:22 +0200, Erich Focht <efocht@ess.nec.de> said:

  Erich> Hi David, please find attached a first attempt to implement
  Erich> the topology.h macros/routines for IA64. We need this for the
  Erich> NUMA scheduler setup.

Why does the cpu_to_node_map[] exist for non-NUMA configurations?  It
seems to me that it would be better to make cpu_to_node_map a macro
that uses an array-check for NUMA configurations and a simple test
against phys_cpu_present_map() for non-NUMA.

	--david

[Linux-ia64] Re: [PATCH] topology for ia64

[Erich Focht]

[-- Attachment #1: Type: text/plain, Size: 901 bytes --]

On Tuesday 22 October 2002 02:07, David Mosberger wrote:
> Why does the cpu_to_node_map[] exist for non-NUMA configurations?  It
> seems to me that it would be better to make cpu_to_node_map a macro
> that uses an array-check for NUMA configurations and a simple test
> against phys_cpu_present_map() for non-NUMA.

Attached is a modified patch for implementing the topology stuff on ia64.
It's on top of your 2.5.39 tree including acpi_numa and the acpi_numa fix
which I've sent you separately.

I dropped the cpu_to_node_map array for the non-NUMA case. The macro
__cpu_to_node() returns 0 in this case. In the places where it is used
(e.g. in the NUMA scheduler) we either run on a valid CPU or have
cpu_online() checks before using it, therefore I also removed the
phys_cpu_present_map check when building the cpu to node map.

Hope this can be included now...

Regards,
Erich

[-- Attachment #2: 00_topology-ia64-2.5.39-2.patch --]
[-- Type: text/x-diff, Size: 4397 bytes --]

diff -urNp 2.5.39-ia64-acpi-dmf/arch/ia64/kernel/acpi.c 2.5.39-ia64-acpi-dmf-top/arch/ia64/kernel/acpi.c
--- 2.5.39-ia64-acpi-dmf/arch/ia64/kernel/acpi.c	Mon Oct 14 15:34:40 2002
+++ 2.5.39-ia64-acpi-dmf-top/arch/ia64/kernel/acpi.c	Tue Oct 22 10:44:26 2002
@@ -784,6 +784,9 @@ acpi_boot_init (char *cmdline)
 	smp_boot_data.cpu_count = total_cpus;
 
 	smp_build_cpu_map();
+#ifdef CONFIG_NUMA
+	build_cpu_to_node_map();
+#endif
 #endif
 	/* Make boot-up look pretty */
 	printk("%d CPUs available, %d CPUs total\n", available_cpus, total_cpus);
diff -urNp 2.5.39-ia64-acpi-dmf/arch/ia64/kernel/smpboot.c 2.5.39-ia64-acpi-dmf-top/arch/ia64/kernel/smpboot.c
--- 2.5.39-ia64-acpi-dmf/arch/ia64/kernel/smpboot.c	Fri Sep 27 23:49:16 2002
+++ 2.5.39-ia64-acpi-dmf-top/arch/ia64/kernel/smpboot.c	Tue Oct 22 10:57:22 2002
@@ -16,6 +16,7 @@
 
 #include <linux/config.h>
 
+#include <linux/acpi.h>
 #include <linux/bootmem.h>
 #include <linux/delay.h>
 #include <linux/init.h>
@@ -427,6 +428,32 @@ smp_build_cpu_map (void)
 	}
 }
 
+#ifdef CONFIG_NUMA
+char cpu_to_node_map[NR_CPUS] __cacheline_aligned;
+/*
+ * Build cpu to node mapping.
+ */
+void __init
+build_cpu_to_node_map(void)
+{
+	int cpu, i;
+
+	for(cpu=0; cpu<NR_CPUS; cpu++) {
+		/*
+		 * All Itanium NUMA platforms I know use ACPI, so maybe we
+		 * can drop this ifdef completely.                    [EF] 
+		 */
+#ifdef CONFIG_ACPI_NUMA
+		for(i=0; i<NR_CPUS; i++)
+			if (cpu_physical_id(cpu) == node_cpuid[i].phys_id) {
+				cpu_to_node_map[cpu]=node_cpuid[i].nid;
+				break;
+			}
+#endif
+	}
+}
+#endif
+
 /*
  * Cycle through the APs sending Wakeup IPIs to boot each.
  */
diff -urNp 2.5.39-ia64-acpi-dmf/include/asm-ia64/numa.h 2.5.39-ia64-acpi-dmf-top/include/asm-ia64/numa.h
--- 2.5.39-ia64-acpi-dmf/include/asm-ia64/numa.h	Mon Oct 14 15:34:14 2002
+++ 2.5.39-ia64-acpi-dmf-top/include/asm-ia64/numa.h	Tue Oct 22 11:04:05 2002
@@ -22,6 +22,8 @@
 # define NR_MEMBLKS   (NR_NODES * 8)
 #endif
 
+extern char cpu_to_node_map[NR_CPUS] __cacheline_aligned;
+
 /* Stuff below this line could be architecture independent */
 
 extern int num_memblks;		/* total number of memory chunks */
diff -urNp 2.5.39-ia64-acpi-dmf/include/asm-ia64/topology.h 2.5.39-ia64-acpi-dmf-top/include/asm-ia64/topology.h
--- 2.5.39-ia64-acpi-dmf/include/asm-ia64/topology.h	Thu Jan  1 01:00:00 1970
+++ 2.5.39-ia64-acpi-dmf-top/include/asm-ia64/topology.h	Tue Oct 22 10:43:06 2002
@@ -0,0 +1,79 @@
+/*
+ * linux/include/asm-ia64/topology.h
+ *
+ * Copyright (C) 2002, Erich Focht, NEC
+ *
+ * All rights reserved.          
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+#ifndef _ASM_IA64_TOPOLOGY_H
+#define _ASM_IA64_TOPOLOGY_H
+
+#include <asm/acpi.h>
+#include <asm/numa.h>
+
+/* Returns the number of the node containing CPU 'cpu' */
+#ifdef CONFIG_NUMA
+#define __cpu_to_node(cpu) cpu_to_node_map[cpu]
+#else
+#define __cpu_to_node(cpu) (0)
+#endif
+
+/*
+ * Returns the number of the node containing MemBlk 'memblk'
+ */
+#ifdef CONFIG_ACPI_NUMA
+#define __memblk_to_node(memblk) (node_memblk[memblk].nid)
+#else
+#define __memblk_to_node(memblk) (memblk)
+#endif
+
+/* 
+ * Returns the number of the node containing Node 'nid'.
+ * Not implemented here. Multi-level hierarchies detected with
+ * the help of node_distance().
+ */
+#define __parent_node(nid) (nid)
+
+/* 
+ * Returns the number of the first CPU on Node 'node'.
+ * Slow in the current implementation.
+ * Who needs this?
+ */
+/* #define __node_to_first_cpu(node) pool_cpus[pool_ptr[node]] */
+static inline int __node_to_first_cpu(int node)
+{
+	int i, cpu;
+
+	for (i=0; i<NR_CPUS; i++)
+		if (__cpu_to_node(i)==node)
+			return i;
+	BUG(); /* couldn't find a cpu on given node */
+	return -1;
+}
+
+/* 
+ * Returns a bitmask of CPUs on Node 'node'.
+ */
+static inline unsigned long __node_to_cpu_mask(int node)
+{
+	int cpu;
+	unsigned long mask = 0UL;
+
+	for(cpu=0; cpu<NR_CPUS; cpu++)
+		if (__cpu_to_node(cpu) == node)
+			mask |= 1UL << cpu;
+	return mask;
+}
+
+/*
+ * Returns the number of the first MemBlk on Node 'node'
+ * Should be fixed when IA64 discontigmem goes in.
+ */
+#define __node_to_memblk(node) (node)
+
+#endif /* _ASM_IA64_TOPOLOGY_H */

[Linux-ia64] Re: [PATCH] topology for ia64

[Matthew Dobson]

Erich Focht wrote:
> On Tuesday 22 October 2002 02:07, David Mosberger wrote:
> 
>>Why does the cpu_to_node_map[] exist for non-NUMA configurations?  It
>>seems to me that it would be better to make cpu_to_node_map a macro
>>that uses an array-check for NUMA configurations and a simple test
>>against phys_cpu_present_map() for non-NUMA.
> 
> Attached is a modified patch for implementing the topology stuff on ia64.
> It's on top of your 2.5.39 tree including acpi_numa and the acpi_numa fix
> which I've sent you separately.
> 
> I dropped the cpu_to_node_map array for the non-NUMA case. The macro
> __cpu_to_node() returns 0 in this case. In the places where it is used
> (e.g. in the NUMA scheduler) we either run on a valid CPU or have
> cpu_online() checks before using it, therefore I also removed the
> phys_cpu_present_map check when building the cpu to node map.

Hi Erich!  Apologies for the long response delay...  I think our mail 
server must be a bit lagged.  ;)

It looks good to me.  As far as this comment:
+/*
+ * Returns the number of the first CPU on Node 'node'.
+ * Slow in the current implementation.
+ * Who needs this?
+ */
+/* #define __node_to_first_cpu(node) pool_cpus[pool_ptr[node]] */
+static inline int __node_to_first_cpu(int node)

No one is using it now.  I think that I will probably deprecate this 
function in the near future as it is pretty useless.  Anyone looking for 
that functionality can just do an __ffs(__node_to_cpu_mask(node)) 
instead, and hope that there is a reasonably quick implementation of 
__node_to_cpu_mask.


> Hope this can be included now...

I agree!  Linus or another maintainer, please pick this up.  These 
macros should be implemented intelligently on as many architectures as 
possible, now that they're beginning to be used in more and more places.

Cheers!

-Matt

> 
> Regards,
> Erich
 >
 > <patch snip>
 >

Re: [Linux-ia64] Re: [PATCH] topology for ia64

[Michael Hohnbaum]

On Tue, 2002-10-29 at 14:19, Matthew Dobson wrote:
> Erich Focht wrote:
> +/*
> + * Returns the number of the first CPU on Node 'node'.
> + * Slow in the current implementation.
> + * Who needs this?
> + */
> +/* #define __node_to_first_cpu(node) pool_cpus[pool_ptr[node]] */
> +static inline int __node_to_first_cpu(int node)
> 
> No one is using it now.  I think that I will probably deprecate this 
> function in the near future as it is pretty useless.  Anyone looking for 
> that functionality can just do an __ffs(__node_to_cpu_mask(node)) 
> instead, and hope that there is a reasonably quick implementation of 
> __node_to_cpu_mask.
> 
I'm using this in the simple NUMA scheduler.  This is quite useful
for iterating through a specific node's CPUs.  Yes, the functionality
can be obtained in a different manner, but is less obvious.

-- 

Michael Hohnbaum                      503-578-5486
hohnbaum@us.ibm.com                   T/L 775-5486

[Linux-ia64] Re: [PATCH] topology for ia64

[Erich Focht]

On Tuesday 29 October 2002 23:19, Matthew Dobson wrote:
> Hi Erich!  Apologies for the long response delay...  I think our mail
> server must be a bit lagged.  ;)

Should I use another email address?

> It looks good to me.  As far as this comment:
> +/*
> + * Returns the number of the first CPU on Node 'node'.
> + * Slow in the current implementation.
> + * Who needs this?
> + */
> +/* #define __node_to_first_cpu(node) pool_cpus[pool_ptr[node]] */
> +static inline int __node_to_first_cpu(int node)
>
> No one is using it now.  I think that I will probably deprecate this
> function in the near future as it is pretty useless.  Anyone looking for
> that functionality can just do an __ffs(__node_to_cpu_mask(node))
> instead, and hope that there is a reasonably quick implementation of
> __node_to_cpu_mask.

Yes, I know of one usage meanwhile. The problem I see is: as far as I
understand the CPUs are not sorted by the node numbers. The NUMA API
doesn't require that, I think. So finding the first CPU in a node is
not really useful for looping over the CPUs of only one node.

When you think of further developments of the NUMA API, I'd suggest
two:

1: Add a sorted list of the nodes and a pointer array into that list
pointing to the first CPU in the node. Like

int node_cpus[NR_CPUS];
int node_first_ptr[MAX_NUMNODES+1];

(or macros, doesn't matter).

Example: 2 nodes:
         node_cpus : 0 1 4 5 2 3 6 7
              node : 0 0 0 0 1 1 1 1
           pointer : ^       ^       ^
=> node_first_ptr[]: 0       4       8

One can initialize this easilly by using the __cpu_to_node()
macro. And with this you can loop over the cpus of one node by doing:

       for (i=node_first_ptr[node]; i<node_first_ptr[node+1]; i++) {
           cpu = node_cpus[i];
           ... do stuff with cpu ...
       }


2: In ACPI there is a table describing the latency ratios between the
nodes. It is called SLIT (System Locality Information Table). On an 8
node system (NEC TX7) this is a matrix of dimension numnodes*numnodes:

int __node_distance[ 8 * 8]    = { 10, 15, 15, 15, 20, 20, 20, 20,
                                   15, 10, 15, 15, 20, 20, 20, 20,
                                   15, 15, 10, 15, 20, 20, 20, 20,
                                   15, 15, 15, 10, 20, 20, 20, 20,
                                   20, 20, 20, 20, 10, 15, 15, 15,
                                   20, 20, 20, 20, 15, 10, 15, 15,
                                   20, 20, 20, 20, 15, 15, 10, 15,
                                   20, 20, 20, 20, 15, 15, 15, 10 };

#define node_distance(i,j)  __node_distance[i*8+j]

This means:
   node_distance(i,i) = 10   (i==j: same node, lowest latency)
   node_distance(i,j) = 15   (i!=j: different node, same supernode)
   node_distance(i,j) = 20   (i!=j: different node, different
                              supernode)

This macro or function describes the NUMA topology of a multi-level
system very well. As the table comes for free with NUMA systems with
ACPI (sooner or later they'll all have this, especially if they want
to run Windows, too), it would be easy to take it into the topology.h.

Regards,
Erich

Re: [Linux-ia64] Re: [PATCH] topology for ia64

[Matthew Dobson]

Michael Hohnbaum wrote:
> On Tue, 2002-10-29 at 14:19, Matthew Dobson wrote:
> 
>>Erich Focht wrote:
>>+/*
>>+ * Returns the number of the first CPU on Node 'node'.
>>+ * Slow in the current implementation.
>>+ * Who needs this?
>>+ */
>>+/* #define __node_to_first_cpu(node) pool_cpus[pool_ptr[node]] */
>>+static inline int __node_to_first_cpu(int node)
>>
>>No one is using it now.  I think that I will probably deprecate this 
>>function in the near future as it is pretty useless.  Anyone looking for 
>>that functionality can just do an __ffs(__node_to_cpu_mask(node)) 
>>instead, and hope that there is a reasonably quick implementation of 
>>__node_to_cpu_mask.
> 
> I'm using this in the simple NUMA scheduler.  This is quite useful
> for iterating through a specific node's CPUs.  Yes, the functionality
> can be obtained in a different manner, but is less obvious.

Hmmm...  This is true, but I'd like to keep the topology functions as 
minimal as possible.  Since the functionality is so easily duplicated, 
and in almost every arch, is implemented almost identically, it seems a 
waste.  In most architectures, they itterate across cpus, either 
returning the first one on the node, or adding each to a mask and 
returning that.

Besides, it's not gone yet... ;)

Cheers!

-Matt

Re: [Linux-ia64] Re: [PATCH] topology for ia64

[Matthew Dobson]

William Lee Irwin III wrote:
> On Tue, Oct 29, 2002 at 02:19:25PM -0800, Matthew Dobson wrote:
> +/*
> + * Returns the number of the first CPU on Node 'node'.
> + * Slow in the current implementation.
> + * Who needs this?
> + */
> +/* #define __node_to_first_cpu(node) pool_cpus[pool_ptr[node]] */
> +static inline int __node_to_first_cpu(int node)
> 
> So far so safe... though no obvious use of it.

Yep...


> On Tue, Oct 29, 2002 at 02:19:25PM -0800, Matthew Dobson wrote:
> 
>>No one is using it now.  I think that I will probably deprecate this 
>>function in the near future as it is pretty useless.  Anyone looking for 
>>that functionality can just do an __ffs(__node_to_cpu_mask(node)) 
>>instead, and hope that there is a reasonably quick implementation of 
>>__node_to_cpu_mask.
> 
> 
> This assumes the value returned by __node_to_cpu_mask() is a single word.

Which is the case right now.  When (not if) that changes, we'll come up 
with more flexible ffs macros, or a better way to count variable length 
bitmasks...  especially as there will be a TON of them.

Cheers!

-Matt