Skip to contentSkip to conversation

x86: Adapt CPU topology detection for AMD Magny-Cours

PATCH Superseded Andreas Herrmann 4 participants 18 messages Aug 5, 2009 – Aug 21, 2009

Subsystems (3)
Documentation x86 Driver core

v4 of 6 Superseded Latest is v6 →

lore
Share & tools
lore

Andreas Herrmann

author
#0 lore raw

v4 cover x86: Adapt CPU topology detection for AMD Magny-Cours

Changes to previous patch set:
- fixed allnoconfig compile error and link error if CONFIG_PCI=n
- fixed hotplug issue: cpumask of siblings sharing same L3 were not properly updated
- properly allocate cpu_node_map

Current patch set contains 5 patches:
- patch 1 adapts common code to show cpu_node_id, cpu_node_siblings and cpu_node_siblings_list in /sys/devices/system/cpu/cpu*/topology
- patch 2 prepares arch/x86 to provide cpu_node information
- patch 3 sets up cpu_node information for AMD Magny-Cours CPU
- patch 4 fixes L3 cache information for Magny-Cours
- patch 5 fixes mcheck code for Magny-Cours

Note that scheduler adaptions are still under construction/test. Hope to send first version as RFC by end of this week.

Patches are against tip/master.
Please apply.

Thanks,

Andreas

PS: See http://marc.info/?l=linux-kernel&m=124403937804836 for
previous patch submission.

Signature · 5 lines
Operating | Advanced Micro Devices GmbH
  System  | Karl-Hammerschmidt-Str. 34, 85609 Dornach b. München, Germany
 Research | Geschäftsführer: Thomas M. McCoy, Giuliano Meroni
  Center  | Sitz: Dornach, Gemeinde Aschheim, Landkreis München
  (OSRC)  | Registergericht München, HRB Nr. 43632
author
#1 lore raw

PATCH v4 1/5 topology: Introduce cpu_node information for multi-node processors

This includes
- cpu_node_id (id of the internal node)
- cpu_node_siblings and cpu_node_siblings_list
(siblings on the same internal node)

Signed-off-by Andreas Herrmann <[email protected]>
---
 drivers/base/topology.c  |   10 ++++++++++
 include/linux/topology.h |    9 +++++++++
 2 files changed, 19 insertions(+), 0 deletions(-)
drivers/base/topology.c +10 -0
index bf6b132..1e35a43 100644
@@ -103,6 +103,9 @@ static ssize_t show_##name##_list(struct sys_device *dev,		\
 define_id_show_func(physical_package_id);
 define_one_ro(physical_package_id);
 
+define_id_show_func(cpu_node_id);
+define_one_ro(cpu_node_id);
+
 define_id_show_func(core_id);
 define_one_ro(core_id);
 

@@ -110,6 +113,10 @@ define_siblings_show_func(thread_cpumask);
 define_one_ro_named(thread_siblings, show_thread_cpumask);
 define_one_ro_named(thread_siblings_list, show_thread_cpumask_list);
 
+define_siblings_show_func(cpu_node_cpumask);
+define_one_ro_named(cpu_node_siblings, show_cpu_node_cpumask);
+define_one_ro_named(cpu_node_siblings_list, show_cpu_node_cpumask_list);
+
 define_siblings_show_func(core_cpumask);
 define_one_ro_named(core_siblings, show_core_cpumask);
 define_one_ro_named(core_siblings_list, show_core_cpumask_list);

@@ -119,6 +126,9 @@ static struct attribute *default_attrs[] = {
 	&attr_core_id.attr,
 	&attr_thread_siblings.attr,
 	&attr_thread_siblings_list.attr,
+	&attr_cpu_node_id.attr,
+	&attr_cpu_node_siblings.attr,
+	&attr_cpu_node_siblings_list.attr,
 	&attr_core_siblings.attr,
 	&attr_core_siblings_list.attr,
 	NULL
include/linux/topology.h +9 -0
index 7402c1a..976a130 100644
@@ -180,6 +180,9 @@ int arch_update_cpu_topology(void);
 #ifndef topology_physical_package_id
 #define topology_physical_package_id(cpu)	((void)(cpu), -1)
 #endif
+#ifndef topology_cpu_node_id
+#define topology_cpu_node_id(cpu)		((void)(cpu), 0)
+#endif
 #ifndef topology_core_id
 #define topology_core_id(cpu)			((void)(cpu), 0)
 #endif

@@ -189,12 +192,18 @@ int arch_update_cpu_topology(void);
 #ifndef topology_core_siblings
 #define topology_core_siblings(cpu)		cpumask_of_cpu(cpu)
 #endif
+#ifndef topology_cpu_node_siblings
+#define topology_cpu_node_siblings(cpu)		topology_core_siblings(cpu)
+#endif
 #ifndef topology_thread_cpumask
 #define topology_thread_cpumask(cpu)		cpumask_of(cpu)
 #endif
 #ifndef topology_core_cpumask
 #define topology_core_cpumask(cpu)		cpumask_of(cpu)
 #endif
+#ifndef topology_cpu_node_cpumask
+#define topology_cpu_node_cpumask(cpu)		topology_core_cpumask(cpu)
+#endif
 
 /* Returns the number of the current Node. */
 #ifndef numa_node_id
Signature · 1 line
1.6.3.3
author
#2 lore raw

PATCH v4 2/5 x86: Provide CPU topology information for multi-node processors

In reply to Andreas Herrmann

Provide topology_cpu_node_id, topology_cpu_node_mask and cpu_node_map. CPUs with matching phys_proc_id and cpu_node_id belong to the same cpu_node.

Signed-off-by Andreas Herrmann <[email protected]>
---
 arch/x86/include/asm/processor.h |    2 ++
 arch/x86/include/asm/smp.h       |    6 ++++++
 arch/x86/include/asm/topology.h  |    2 ++
 arch/x86/kernel/cpu/common.c     |    2 ++
 arch/x86/kernel/cpu/proc.c       |    1 +
 arch/x86/kernel/smpboot.c        |   13 +++++++++++++
 6 files changed, 26 insertions(+), 0 deletions(-)
arch/x86/include/asm/processor.h +2 -0
index 2b03f70..0e0b363 100644
@@ -107,6 +107,8 @@ struct cpuinfo_x86 {
 	u16			booted_cores;
 	/* Physical processor id: */
 	u16			phys_proc_id;
+	/* Node id in case of multi-node processor: */
+	u16			cpu_node_id;
 	/* Core id: */
 	u16			cpu_core_id;
 	/* Index into per_cpu list: */
arch/x86/include/asm/smp.h +6 -0
index 6a84ed1..aad37c6 100644
@@ -22,6 +22,7 @@ extern int smp_num_siblings;
 extern unsigned int num_processors;
 
 DECLARE_PER_CPU(cpumask_var_t, cpu_sibling_map);
+DECLARE_PER_CPU(cpumask_var_t, cpu_node_map);
 DECLARE_PER_CPU(cpumask_var_t, cpu_core_map);
 DECLARE_PER_CPU(u16, cpu_llc_id);
 DECLARE_PER_CPU(int, cpu_number);

@@ -31,6 +32,11 @@ static inline struct cpumask *cpu_sibling_mask(int cpu)
 	return per_cpu(cpu_sibling_map, cpu);
 }
 
+static inline struct cpumask *cpu_node_mask(int cpu)
+{
+	return per_cpu(cpu_node_map, cpu);
+}
+
 static inline struct cpumask *cpu_core_mask(int cpu)
 {
 	return per_cpu(cpu_core_map, cpu);
arch/x86/include/asm/topology.h +2 -0
index 066ef59..9eddb69 100644
@@ -190,6 +190,8 @@ extern const struct cpumask *cpu_coregroup_mask(int cpu);
 #define topology_core_id(cpu)			(cpu_data(cpu).cpu_core_id)
 #define topology_core_cpumask(cpu)		(per_cpu(cpu_core_map, cpu))
 #define topology_thread_cpumask(cpu)		(per_cpu(cpu_sibling_map, cpu))
+#define topology_cpu_node_id(cpu)		(cpu_data(cpu).cpu_node_id)
+#define topology_cpu_node_cpumask(cpu)		(per_cpu(cpu_node_map, cpu))
 
 /* indicates that pointers to the topology cpumask_t maps are valid */
 #define arch_provides_topology_pointers		yes
arch/x86/kernel/cpu/common.c +2 -0
index 6210c84..6b3c67e 100644
@@ -478,6 +478,8 @@ out:
 	if ((c->x86_max_cores * smp_num_siblings) > 1) {
 		printk(KERN_INFO  "CPU: Physical Processor ID: %d\n",
 		       c->phys_proc_id);
+		printk(KERN_INFO  "CPU: Processor Node ID: %d\n",
+		       c->cpu_node_id);
 		printk(KERN_INFO  "CPU: Processor Core ID: %d\n",
 		       c->cpu_core_id);
 	}
arch/x86/kernel/cpu/proc.c +1 -0
index 62ac8cb..a098d78 100644
@@ -15,6 +15,7 @@ static void show_cpuinfo_core(struct seq_file *m, struct cpuinfo_x86 *c,
 		seq_printf(m, "physical id\t: %d\n", c->phys_proc_id);
 		seq_printf(m, "siblings\t: %d\n",
 			   cpumask_weight(cpu_core_mask(cpu)));
+		seq_printf(m, "node id\t\t: %d\n", c->cpu_node_id);
 		seq_printf(m, "core id\t\t: %d\n", c->cpu_core_id);
 		seq_printf(m, "cpu cores\t: %d\n", c->booted_cores);
 		seq_printf(m, "apicid\t\t: %d\n", c->apicid);
arch/x86/kernel/smpboot.c +13 -0
index dee0f3d..f50af56 100644
@@ -109,6 +109,10 @@ EXPORT_PER_CPU_SYMBOL(cpu_sibling_map);
 DEFINE_PER_CPU(cpumask_var_t, cpu_core_map);
 EXPORT_PER_CPU_SYMBOL(cpu_core_map);
 
+/* representing node silbings on multi-node CPU */
+DEFINE_PER_CPU(cpumask_var_t, cpu_node_map);
+EXPORT_PER_CPU_SYMBOL(cpu_node_map);
+
 /* Per CPU bogomips and other parameters */
 DEFINE_PER_CPU_SHARED_ALIGNED(struct cpuinfo_x86, cpu_info);
 EXPORT_PER_CPU_SYMBOL(cpu_info);

@@ -403,6 +407,11 @@ void __cpuinit set_cpu_sibling_map(int cpu)
 			cpumask_set_cpu(i, c->llc_shared_map);
 			cpumask_set_cpu(cpu, cpu_data(i).llc_shared_map);
 		}
+		if ((c->phys_proc_id == cpu_data(i).phys_proc_id) &&
+		    (c->cpu_node_id == cpu_data(i).cpu_node_id)) {
+			cpumask_set_cpu(i, cpu_node_mask(cpu));
+			cpumask_set_cpu(cpu, cpu_node_mask(i));
+		}
 		if (c->phys_proc_id == cpu_data(i).phys_proc_id) {
 			cpumask_set_cpu(i, cpu_core_mask(cpu));
 			cpumask_set_cpu(cpu, cpu_core_mask(i));

@@ -1061,8 +1070,10 @@ void __init native_smp_prepare_cpus(unsigned int max_cpus)
 	for_each_possible_cpu(i) {
 		alloc_cpumask_var(&per_cpu(cpu_sibling_map, i), GFP_KERNEL);
 		alloc_cpumask_var(&per_cpu(cpu_core_map, i), GFP_KERNEL);
+		alloc_cpumask_var(&per_cpu(cpu_node_map, i), GFP_KERNEL);
 		alloc_cpumask_var(&cpu_data(i).llc_shared_map, GFP_KERNEL);
 		cpumask_clear(per_cpu(cpu_core_map, i));
+		cpumask_clear(per_cpu(cpu_node_map, i));
 		cpumask_clear(per_cpu(cpu_sibling_map, i));
 		cpumask_clear(cpu_data(i).llc_shared_map);
 	}

@@ -1210,6 +1221,7 @@ static void remove_siblinginfo(int cpu)
 
 	for_each_cpu(sibling, cpu_core_mask(cpu)) {
 		cpumask_clear_cpu(cpu, cpu_core_mask(sibling));
+		cpumask_clear_cpu(cpu, cpu_node_mask(sibling));
 		/*/
 		 * last thread sibling in this cpu core going down
 		 */

@@ -1222,6 +1234,7 @@ static void remove_siblinginfo(int cpu)
 	cpumask_clear(cpu_sibling_mask(cpu));
 	cpumask_clear(cpu_core_mask(cpu));
 	c->phys_proc_id = 0;
+	c->cpu_node_id = 0;
 	c->cpu_core_id = 0;
 	cpumask_clear_cpu(cpu, cpu_sibling_setup_mask);
 }
Signature · 1 line
1.6.3.3
author
#6 lore raw

PATCH v4 3/5 x86: Add cpu_node topology detection for AMD Magny-Cours

In reply to Andreas Herrmann

This adapts CPU topology detection for AMD Magny-Cours.

Here is example output from two cores on same package but different internal cpu_nodes:

/sys/devices/system/cpu/cpu5:

  physical_package_id        : 0
  core_id                    : 5
  thread_siblings            : 00000020
  thread_siblings_list       : 5
  cpu_node_id                : 0
  cpu_node_siblings          : 0000003f
  cpu_node_siblings_list     : 0-5
  core_siblings              : 00000fff
  core_siblings_list         : 0-11
/sys/devices/system/cpu/cpu6:
  physical_package_id        : 0
  core_id                    : 0
  thread_siblings            : 00000040
  thread_siblings_list       : 6
  cpu_node_id                : 1
  cpu_node_siblings          : 00000fc0
  cpu_node_siblings_list     : 6-11
  core_siblings              : 00000fff
  core_siblings_list         : 0-11

(Included allnoconfig compile fix by Nicolas Palix <[email protected]>.)

Signed-off-by Andreas Herrmann <[email protected]>
---
 arch/x86/include/asm/cpufeature.h |    1 +
 arch/x86/kernel/cpu/amd.c         |   69 +++++++++++++++++++++++++++++++++++-
 2 files changed, 68 insertions(+), 2 deletions(-)
arch/x86/include/asm/cpufeature.h +1 -0
index 4a28d22..847fee6 100644
@@ -95,6 +95,7 @@
 #define X86_FEATURE_NONSTOP_TSC	(3*32+24) /* TSC does not stop in C states */
 #define X86_FEATURE_CLFLUSH_MONITOR (3*32+25) /* "" clflush reqd with monitor */
 #define X86_FEATURE_EXTD_APICID	(3*32+26) /* has extended APICID (8 bits) */
+#define X86_FEATURE_AMD_DCM     (3*32+27) /* multi-node processor */
 
 /* Intel-defined CPU features, CPUID level 0x00000001 (ecx), word 4 */
 #define X86_FEATURE_XMM3	(4*32+ 0) /* "pni" SSE-3 */
arch/x86/kernel/cpu/amd.c +67 -2

88 lines not on this page · raw message

Signature · 1 line
1.6.3.3
author
#8 lore raw

PATCH v4 5/5 x86, mcheck: Make use of cpu_node_mask instead of cpu_core_mask

In reply to Andreas Herrmann

This fixes threshold_bank4 support on multi-node processors.

We need to create two sets of symlinks for sibling shared banks -- one set for each internal node, symlinks of each set should target the first core on the same internal node.

Currently only one set is created where all symlinks are targeting the first core of the entire socket.

Signed-off-by Andreas Herrmann <[email protected]>
---
 arch/x86/kernel/cpu/mcheck/mce_amd.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
arch/x86/kernel/cpu/mcheck/mce_amd.c +3 -3
index ddae216..595cbe5 100644
@@ -494,7 +494,7 @@ static __cpuinit int threshold_create_bank(unsigned int cpu, unsigned int bank)
 
 #ifdef CONFIG_SMP
 	if (cpu_data(cpu).cpu_core_id && shared_bank[bank]) {	/* symlink */
-		i = cpumask_first(cpu_core_mask(cpu));
+		i = cpumask_first(cpu_node_mask(cpu));
 
 		/* first core not up yet */
 		if (cpu_data(i).cpu_core_id)

@@ -514,7 +514,7 @@ static __cpuinit int threshold_create_bank(unsigned int cpu, unsigned int bank)
 		if (err)
 			goto out;
 
-		cpumask_copy(b->cpus, cpu_core_mask(cpu));
+		cpumask_copy(b->cpus, cpu_node_mask(cpu));
 		per_cpu(threshold_banks, cpu)[bank] = b;
 
 		goto out;

@@ -539,7 +539,7 @@ static __cpuinit int threshold_create_bank(unsigned int cpu, unsigned int bank)
 #ifndef CONFIG_SMP
 	cpumask_setall(b->cpus);
 #else
-	cpumask_copy(b->cpus, cpu_core_mask(cpu));
+	cpumask_copy(b->cpus, cpu_node_mask(cpu));
 #endif
 
 	per_cpu(threshold_banks, cpu)[bank] = b;
Signature · 1 line
1.6.3.3
author
#7 lore raw

PATCH v4 4/5 x86, cacheinfo: Fixup L3 cache information for AMD multi-node processors

In reply to Andreas Herrmann

L3 cache size, associativity and shared_cpu information need to be adapted to show information for an internal node instead of the entire physical package.

Signed-off-by Andreas Herrmann <[email protected]>
---
 arch/x86/kernel/cpu/intel_cacheinfo.c |   30 ++++++++++++++++++++----------
 1 files changed, 20 insertions(+), 10 deletions(-)
arch/x86/kernel/cpu/intel_cacheinfo.c +20 -10
index 306bf0d..17f4c8b 100644
@@ -241,7 +241,7 @@ amd_cpuid4(int leaf, union _cpuid4_leaf_eax *eax,
 	case 0:
 		if (!l1->val)
 			return;
-		assoc = l1->assoc;
+		assoc = assocs[l1->assoc];
 		line_size = l1->line_size;
 		lines_per_tag = l1->lines_per_tag;
 		size_in_kb = l1->size_in_kb;

@@ -249,7 +249,7 @@ amd_cpuid4(int leaf, union _cpuid4_leaf_eax *eax,
 	case 2:
 		if (!l2.val)
 			return;
-		assoc = l2.assoc;
+		assoc = assocs[l2.assoc];
 		line_size = l2.line_size;
 		lines_per_tag = l2.lines_per_tag;
 		/* cpu_data has errata corrections for K7 applied */

@@ -258,10 +258,14 @@ amd_cpuid4(int leaf, union _cpuid4_leaf_eax *eax,
 	case 3:
 		if (!l3.val)
 			return;
-		assoc = l3.assoc;
+		assoc = assocs[l3.assoc];
 		line_size = l3.line_size;
 		lines_per_tag = l3.lines_per_tag;
 		size_in_kb = l3.size_encoded * 512;
+		if (boot_cpu_has(X86_FEATURE_AMD_DCM)) {
+			size_in_kb = size_in_kb >> 1;
+			assoc = assoc >> 1;
+		}
 		break;
 	default:
 		return;

@@ -270,18 +274,14 @@ amd_cpuid4(int leaf, union _cpuid4_leaf_eax *eax,
 	eax->split.is_self_initializing = 1;
 	eax->split.type = types[leaf];
 	eax->split.level = levels[leaf];
-	if (leaf == 3)
-		eax->split.num_threads_sharing =
-			current_cpu_data.x86_max_cores - 1;
-	else
-		eax->split.num_threads_sharing = 0;
+	eax->split.num_threads_sharing = 0;
 	eax->split.num_cores_on_die = current_cpu_data.x86_max_cores - 1;
 
 
-	if (assoc == 0xf)
+	if (assoc == 0xffff)
 		eax->split.is_fully_associative = 1;
 	ebx->split.coherency_line_size = line_size - 1;
-	ebx->split.ways_of_associativity = assocs[assoc] - 1;
+	ebx->split.ways_of_associativity = assoc - 1;
 	ebx->split.physical_line_partition = lines_per_tag - 1;
 	ecx->split.number_of_sets = (size_in_kb * 1024) / line_size /
 		(ebx->split.ways_of_associativity + 1) - 1;

@@ -523,6 +523,16 @@ static void __cpuinit cache_shared_cpu_map_setup(unsigned int cpu, int index)
 	int index_msb, i;
 	struct cpuinfo_x86 *c = &cpu_data(cpu);
 
+	if ((index == 3) && (c->x86_vendor == X86_VENDOR_AMD)) {
+		for_each_online_cpu(i) {
+			if (!per_cpu(cpuid4_info, i))
+				continue;
+			this_leaf = CPUID4_INFO_IDX(i, index);
+			cpumask_copy(to_cpumask(this_leaf->shared_cpu_map),
+				     topology_cpu_node_cpumask(i));
+		}
+		return;
+	}
 	this_leaf = CPUID4_INFO_IDX(cpu, index);
 	num_threads_sharing = 1 + this_leaf->eax.split.num_threads_sharing;
Signature · 1 line
1.6.3.3
#9 lore raw
In reply to Andreas Herrmann
Quoted 6 lines · Andreas Herrmann

Changes to previous patch set:
- fixed allnoconfig compile error and link error if CONFIG_PCI=n
- fixed hotplug issue: cpumask of siblings sharing same L3 were not properly updated
- properly allocate cpu_node_map

Quoted 9 lines

Current patch set contains 5 patches:
- patch 1 adapts common code to show cpu_node_id, cpu_node_siblings and cpu_node_siblings_list in /sys/devices/system/cpu/cpu*/topology
- patch 2 prepares arch/x86 to provide cpu_node information
- patch 3 sets up cpu_node information for AMD Magny-Cours CPU
- patch 4 fixes L3 cache information for Magny-Cours
- patch 5 fixes mcheck code for Magny-Cours

Hello Andreas,

Reading your first submission I find something disturbing. You say that we'll have the following sibling information:

  Level        | Set of CPUs
 --------------|---------------
  phys_package | core_siblings
  cpu_node     | cpu_node_siblings
  core         | thread_siblings
  thread       | one CPU

This breaks the existing convention/semantics. Currently core/thread_siblings contains the cpumask covering *all* siblings of current core/thread object. What you're adding only shows the cpumask of current "cpu_node" object in cpu_node_siblings. I don't have any preference between both semantics, but I think "cpu_node" should use the semantics that "core" and "thread" do. So the above should be changed into:

  Level        | Set of CPUs
 --------------|---------------
  phys_package | cpu_node_siblings
  cpu_node     | core_siblings
  core         | thread_siblings
  thread       | one CPU

Brice

#3 lore raw

Re: [PATCH 2/5] x86: Provide CPU topology information for multi-node processors

In reply to Andreas Herrmann

Hi Andrea,

arch/x86/kernel/smpboot.c void __init native_smp_prepare_cpus(unsigned int max_cpus) · line 1076

Quoted 9 lines · On Wed, 5 Aug 2009 17:48:11 +0200 Andreas Herrmann <[email protected]>…
arch/x86/kernel/smpboot.c +2 -0
@@ -1061,8 +1070,10 @@ void __init native_smp_prepare_cpus(unsigned int max_cpus)
 	for_each_possible_cpu(i) {
 		alloc_cpumask_var(&per_cpu(cpu_sibling_map, i), GFP_KERNEL);
 		alloc_cpumask_var(&per_cpu(cpu_core_map, i), GFP_KERNEL);
+		alloc_cpumask_var(&per_cpu(cpu_node_map, i), GFP_KERNEL);
 		alloc_cpumask_var(&cpu_data(i).llc_shared_map, GFP_KERNEL);
 		cpumask_clear(per_cpu(cpu_core_map, i));
+		cpumask_clear(per_cpu(cpu_node_map, i));

I noticed this in linux-next ... you can use zalloc_cpumask_var() instead of alloc_cpumask_var() followed by cpumask_clear().

Signature · 3 lines
Cheers,
Stephen Rothwell                    [email protected]
http://www.canb.auug.org.au/~sfr/
author
#10 lore raw
In reply to Brice Goglin
Quoted 22 lines · Brice Goglin
Quoted 6 lines · Andreas Herrmann

Changes to previous patch set:
- fixed allnoconfig compile error and link error if CONFIG_PCI=n
- fixed hotplug issue: cpumask of siblings sharing same L3 were not properly updated
- properly allocate cpu_node_map

Quoted 9 lines

Current patch set contains 5 patches:
- patch 1 adapts common code to show cpu_node_id, cpu_node_siblings and cpu_node_siblings_list in /sys/devices/system/cpu/cpu*/topology
- patch 2 prepares arch/x86 to provide cpu_node information
- patch 3 sets up cpu_node information for AMD Magny-Cours CPU
- patch 4 fixes L3 cache information for Magny-Cours
- patch 5 fixes mcheck code for Magny-Cours

Hello Andreas,

Reading your first submission I find something disturbing. You say that we'll have the following sibling information:

Quoted 8 lines
  Level        | Set of CPUs
 --------------|---------------
  phys_package | core_siblings
  cpu_node     | cpu_node_siblings
  core         | thread_siblings
  thread       | one CPU

This breaks the existing convention/semantics.

Isn't the existing convention that core_siblings denotes all CPUs on same socket.

Quoted 7 lines

Currently core/thread_siblings contains the cpumask covering *all* siblings of current core/thread object. What you're adding only shows the cpumask of current "cpu_node" object in cpu_node_siblings. I don't have any preference between both semantics, but I think "cpu_node" should use the semantics that "core" and "thread" do. So the above should be changed into:

Quoted 6 lines
  Level        | Set of CPUs
 --------------|---------------
  phys_package | cpu_node_siblings
  cpu_node     | core_siblings
  core         | thread_siblings
  thread       | one CPU

Of course I thought also to implement it this way because it looks more consistent, but IMHO the patches are less intrusive if this scheme is _not_ used. Instead I kept core_siblings as is ("for historic reasons", nobody needs to accustom to new semantics). And use cpu_node_siblings where it really matters.

But this reminds me that some documentation is required to describe the new attributes.

What do others think?

Thanks,
Andreas

Signature · 5 lines
Operating | Advanced Micro Devices GmbH
  System  | Karl-Hammerschmidt-Str. 34, 85609 Dornach b. München, Germany
 Research | Geschäftsführer: Thomas M. McCoy, Giuliano Meroni
  Center  | Sitz: Dornach, Gemeinde Aschheim, Landkreis München
  (OSRC)  | Registergericht München, HRB Nr. 43632
#11 lore raw
Quoted 6 lines · Andreas Herrmann

Of course I thought also to implement it this way because it looks more consistent, but IMHO the patches are less intrusive if this scheme is _not_ used. Instead I kept core_siblings as is ("for historic reasons", nobody needs to accustom to new semantics). And use cpu_node_siblings where it really matters.

Well, core_siblings and cpu_node_sibling will only be different on Magny-Cours anyway. So even if core_siblings becomes "all cores in cpu_node" instead of "all cores in socket", it won't actually break any existing setup. I personally prefer having the same kind of semantics for all foo_siblings rather than having something with a different meaning between core and thread.

Brice

author
#12 lore raw
Quoted 8 lines · Brice Goglin
Quoted 6 lines · Andreas Herrmann

Of course I thought also to implement it this way because it looks more consistent, but IMHO the patches are less intrusive if this scheme is _not_ used. Instead I kept core_siblings as is ("for historic reasons", nobody needs to accustom to new semantics). And use cpu_node_siblings where it really matters.

Quoted 6 lines

Well, core_siblings and cpu_node_sibling will only be different on Magny-Cours anyway. So even if core_siblings becomes "all cores in cpu_node" instead of "all cores in socket", it won't actually break any existing setup. I personally prefer having the same kind of semantics for all foo_siblings rather than having something with a different meaning between core and thread.

You just want to have the user interface adapted to your semantics. I.e. you want to have following attributes:
(from Documentation/cputopology.txt)

1) /sys/devices/system/cpu/cpuX/topology/physical_package_id:
represent the physical package id of cpu X;
2) /sys/devices/system/cpu/cpuX/topology/core_id:
represent the cpu core id to cpu X;
3) /sys/devices/system/cpu/cpuX/topology/cpu_node_id:
represent the processor internal node_id to cpu X;
4) /sys/devices/system/cpu/cpuX/topology/thread_siblings:
represent the thread siblings to cpu X in the same core;
5) /sys/devices/system/cpu/cpuX/topology/core_siblings:
represent the thread siblings to cpu X in the same processor internal node;
6) /sys/devices/system/cpu/cpuX/topology/cpu_node_siblings:
represent the thread siblings to cpu X in the same physical package;

Note: In case of multi-node processors (e.g. Magny-Cours) 5 and 6 differ. For all other CPUs 5 and 6 provide similar values and cpu_node_id is 0.

Ok, I can modify the first two patches accordingly to swap the two attributes and also provide an update for the documentation file.

(But I will keep the internal identifiers for the cpu_node stuff as is.)

Thanks,
Andreas

Signature · 5 lines
Operating | Advanced Micro Devices GmbH
  System  | Karl-Hammerschmidt-Str. 34, 85609 Dornach b. München, Germany
 Research | Geschäftsführer: Thomas M. McCoy, Giuliano Meroni
  Center  | Sitz: Dornach, Gemeinde Aschheim, Landkreis München
  (OSRC)  | Registergericht München, HRB Nr. 43632
author
#4 lore raw

Re: [PATCH 2/5] x86: Provide CPU topology information for multi-node processors

In reply to Stephen Rothwell
Quoted 4 lines · Stephen Rothwell

Hi Andrea,

Quoted 1 line · On Wed, 5 Aug 2009 17:48:11 +0200 Andreas Herrmann <[email protected]>…
Quoted 11 lines

arch/x86/kernel/smpboot.c void __init native_smp_prepare_cpus(unsigned int max_cpus) · line 1076

Quoted 8 lines
arch/x86/kernel/smpboot.c +2 -0
@@ -1061,8 +1070,10 @@ void __init native_smp_prepare_cpus(unsigned int max_cpus)
 	for_each_possible_cpu(i) {
 		alloc_cpumask_var(&per_cpu(cpu_sibling_map, i), GFP_KERNEL);
 		alloc_cpumask_var(&per_cpu(cpu_core_map, i), GFP_KERNEL);
+		alloc_cpumask_var(&per_cpu(cpu_node_map, i), GFP_KERNEL);
 		alloc_cpumask_var(&cpu_data(i).llc_shared_map, GFP_KERNEL);
 		cpumask_clear(per_cpu(cpu_core_map, i));
+		cpumask_clear(per_cpu(cpu_node_map, i));

I noticed this in linux-next ... you can use zalloc_cpumask_var() instead of alloc_cpumask_var() followed by cpumask_clear().

I know, there is a collision with a patch in linux-next that replaced alloc_cpumask_var/cpumask_clear with the zalloc version.

(a) Either that patch should be adapted to change also the new allocation.
(b) I can also change all those allocation to zalloc with my patch.

Make your choice:

[ ] (a)
[ ] (b)

Thanks,

Andreas

Signature · 5 lines
Operating | Advanced Micro Devices GmbH
  System  | Karl-Hammerschmidt-Str. 34, 85609 Dornach b. München, Germany
 Research | Geschäftsführer: Thomas M. McCoy, Giuliano Meroni
  Center  | Sitz: Dornach, Gemeinde Aschheim, Landkreis München
  (OSRC)  | Registergericht München, HRB Nr. 43632
author
#13 lore raw

PATCH 1/1 x86, topology: Swap semantic of core_siblings and cpu_node_siblings

In reply to Andreas Herrmann

Brice Goglin suggested to use the following semantic for CPU topology information

  Level        | Set of CPUs
 --------------|---------------
  phys_package | cpu_node_siblings
  cpu_node     | core_siblings
  core         | thread_siblings
  thread       | one CPU

Done that.

CC Brice Goglin <[email protected]>
Signed-off-by Andreas Herrmann <[email protected]>
---
 arch/x86/include/asm/topology.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

Patch is on top of the 5 previous patches.
Documentation update will follow.

Regards,
Andreas

arch/x86/include/asm/topology.h +2 -2
index 9eddb69..d53ef91 100644
@@ -188,10 +188,10 @@ extern const struct cpumask *cpu_coregroup_mask(int cpu);
 #ifdef ENABLE_TOPO_DEFINES
 #define topology_physical_package_id(cpu)	(cpu_data(cpu).phys_proc_id)
 #define topology_core_id(cpu)			(cpu_data(cpu).cpu_core_id)
-#define topology_core_cpumask(cpu)		(per_cpu(cpu_core_map, cpu))
+#define topology_core_cpumask(cpu)		(per_cpu(cpu_node_map, cpu))
 #define topology_thread_cpumask(cpu)		(per_cpu(cpu_sibling_map, cpu))
 #define topology_cpu_node_id(cpu)		(cpu_data(cpu).cpu_node_id)
-#define topology_cpu_node_cpumask(cpu)		(per_cpu(cpu_node_map, cpu))
+#define topology_cpu_node_cpumask(cpu)		(per_cpu(cpu_core_map, cpu))
 
 /* indicates that pointers to the topology cpumask_t maps are valid */
 #define arch_provides_topology_pointers		yes
Signature · 1 line
1.6.3.3
author
#5 lore raw

PATCH 1/1 x86, smpboot: use zalloc_cpumask_var instead of alloc/clear

In reply to Andreas Herrmann
Quoted 20 lines · Andreas Herrmann
Quoted 4 lines · Stephen Rothwell

Hi Andrea,

Quoted 1 line · On Wed, 5 Aug 2009 17:48:11 +0200 Andreas Herrmann <[email protected]>…
Quoted 11 lines

Review void __init native_smp_prepare_cpus(unsigned int max_cpus) · line 1076

Quoted 8 lines
(quoted diff) +2 -0
@@ -1061,8 +1070,10 @@ void __init native_smp_prepare_cpus(unsigned int max_cpus)
 	for_each_possible_cpu(i) {
 		alloc_cpumask_var(&per_cpu(cpu_sibling_map, i), GFP_KERNEL);
 		alloc_cpumask_var(&per_cpu(cpu_core_map, i), GFP_KERNEL);
+		alloc_cpumask_var(&per_cpu(cpu_node_map, i), GFP_KERNEL);
 		alloc_cpumask_var(&cpu_data(i).llc_shared_map, GFP_KERNEL);
 		cpumask_clear(per_cpu(cpu_core_map, i));
+		cpumask_clear(per_cpu(cpu_node_map, i));

I noticed this in linux-next ... you can use zalloc_cpumask_var() instead of alloc_cpumask_var() followed by cpumask_clear().

I know, there is a collision with a patch in linux-next that replaced alloc_cpumask_var/cpumask_clear with the zalloc version.

Quoted 7 lines

(a) Either that patch should be adapted to change also the new allocation.
(b) I can also change all those allocation to zalloc with my patch.

Make your choice:

[ ] (a)
[ ] (b)

My choice is
[X] (c) Do this in a separate patch.

See below.

Regards,
Andreas

Quoted 1 line

From 5d7e6138d4bc2d8c5191e6bf9cfbe9d5a27d79b0 Mon Sep 17 00:00:00 2001

From: Andreas Herrmann <[email protected]>
Date: Thu, 6 Aug 2009 19:31:58 +0200
Subject: [PATCH] x86, smpboot: use zalloc_cpumask_var instead of alloc/clear

Suggested-by Stephen Rothwell <[email protected]>
Signed-off-by Andreas Herrmann <[email protected]>
---
 arch/x86/kernel/smpboot.c |   12 ++++--------
 1 files changed, 4 insertions(+), 8 deletions(-)
arch/x86/kernel/smpboot.c +4 -8
index f50af56..f797214 100644
@@ -1068,14 +1068,10 @@ void __init native_smp_prepare_cpus(unsigned int max_cpus)
 #endif
 	current_thread_info()->cpu = 0;  /* needed? */
 	for_each_possible_cpu(i) {
-		alloc_cpumask_var(&per_cpu(cpu_sibling_map, i), GFP_KERNEL);
-		alloc_cpumask_var(&per_cpu(cpu_core_map, i), GFP_KERNEL);
-		alloc_cpumask_var(&per_cpu(cpu_node_map, i), GFP_KERNEL);
-		alloc_cpumask_var(&cpu_data(i).llc_shared_map, GFP_KERNEL);
-		cpumask_clear(per_cpu(cpu_core_map, i));
-		cpumask_clear(per_cpu(cpu_node_map, i));
-		cpumask_clear(per_cpu(cpu_sibling_map, i));
-		cpumask_clear(cpu_data(i).llc_shared_map);
+		zalloc_cpumask_var(&per_cpu(cpu_sibling_map, i), GFP_KERNEL);
+		zalloc_cpumask_var(&per_cpu(cpu_core_map, i), GFP_KERNEL);
+		zalloc_cpumask_var(&per_cpu(cpu_node_map, i), GFP_KERNEL);
+		zalloc_cpumask_var(&cpu_data(i).llc_shared_map, GFP_KERNEL);
 	}
 	set_cpu_sibling_map(0);
Signature · 1 line
1.6.3.3
author
#14 lore raw

PATCH 1/1 topology: Update CPU topology documentation

In reply to Andreas Herrmann

Mention new attributes introduced for multi-node processor support.

Signed-off-by Andreas Herrmann <[email protected]>
---
 Documentation/cputopology.txt |   35 +++++++++++++++++++++++++++++------
 1 files changed, 29 insertions(+), 6 deletions(-)
Documentation/cputopology.txt +29 -6
index b41f3e5..48914b1 100644
@@ -3,13 +3,32 @@ Export cpu topology info via sysfs. Items (attributes) are similar
 to /proc/cpuinfo.
 
 1) /sys/devices/system/cpu/cpuX/topology/physical_package_id:
-represent the physical package id of  cpu X;
+   represents the physical package id of cpu X;
 2) /sys/devices/system/cpu/cpuX/topology/core_id:
-represent the cpu core id to cpu X;
-3) /sys/devices/system/cpu/cpuX/topology/thread_siblings:
-represent the thread siblings to cpu X in the same core;
-4) /sys/devices/system/cpu/cpuX/topology/core_siblings:
-represent the thread siblings to cpu X in the same physical package;
+   represents the cpu core id of cpu X;
+3) /sys/devices/system/cpu/cpuX/topology/cpu_node_id:
+   represents the processor internal node_id of cpu X;
+4) /sys/devices/system/cpu/cpuX/topology/thread_siblings:
+   represents the thread siblings of cpu X in the same core;
+5) /sys/devices/system/cpu/cpuX/topology/core_siblings:
+   represents the thread siblings of cpu X in the same processor
+   internal node;
+6) /sys/devices/system/cpu/cpuX/topology/cpu_node_siblings:
+   represents the thread siblings of cpu X in the same physical
+   package;
+
+Note: cpu_node_siblings and core_siblings differ only on
+      multi-node processors. On all other processors they are
+      identical and then cpu_node_id is always 0.
+
+With that CPU topology is mapped into following hierarchy:
+
+  Level        | Set of CPUs
+ --------------|---------------
+  phys_package | cpu_node_siblings
+  cpu_node     | core_siblings
+  core         | thread_siblings
+  thread       | one CPU
 
 To implement it in an architecture-neutral way, a new source file,
 drivers/base/topology.c, is to export the 4 attributes.

@@ -20,6 +39,8 @@ these macros in include/asm-XXX/topology.h:
 #define topology_core_id(cpu)
 #define topology_thread_cpumask(cpu)
 #define topology_core_cpumask(cpu)
+#define topology_cpu_node_id(cpu)
+#define topology_cpu_node_siblings(cpu)
 
 The type of **_id is int.
 The type of siblings is (const) struct cpumask *.

@@ -31,6 +52,8 @@ not defined by include/asm-XXX/topology.h:
 2) core_id: 0
 3) thread_siblings: just the given CPU
 4) core_siblings: just the given CPU
+5) cpu_node_id: 0
+6) cpu_node_siblings: identical to core_siblings
 
 Additionally, cpu topology information is provided under
 /sys/devices/system/cpu and includes these files.  The internal
Signature · 1 line
1.6.3.3
#15 lore raw
In reply to Andreas Herrmann
Quoted 2 lines · Andreas Herrmann

Note that scheduler adaptions are still under construction/test. Hope to send first version as RFC by end of this week.

Ok - i'll wait for those, and they really put all this info into real use so bugs/inefficiencies can be noticed.

You also indicated that you'll address the feedback from Brice Goglin to make the semantics (and naming) more consistent across the kernel.

Ingo

#16 lore raw
Quoted 4 lines · Ingo Molnar

You also indicated that you'll address the feedback from Brice Goglin to make the semantics (and naming) more consistent across the kernel.

It's done in the updated patchset that Andreas sent yesterday (in patch 4/8).

Brice

author
#17 lore raw
In reply to Ingo Molnar
Quoted 8 lines · Ingo Molnar
Quoted 2 lines · Andreas Herrmann

Note that scheduler adaptions are still under construction/test. Hope to send first version as RFC by end of this week.

Ok - i'll wait for those, and they really put all this info into real use so bugs/inefficiencies can be noticed.

The scheduler adaptions were submitted yesterday.
(http://lkml.org/lkml/2009/8/7/210)

Quoted 3 lines

You also indicated that you'll address the feedback from Brice Goglin to make the semantics (and naming) more consistent across the kernel.

That's done in the v5 patch set, see
http://lkml.org/lkml/2009/8/7/210

Please reconsider to apply the topology stuff to tip-tree.

Thanks,

Andreas

Signature · 5 lines
Operating | Advanced Micro Devices GmbH
  System  | Karl-Hammerschmidt-Str. 34, 85609 Dornach b. München, Germany
 Research | Geschäftsführer: Thomas M. McCoy, Giuliano Meroni
  Center  | Sitz: Dornach, Gemeinde Aschheim, Landkreis München
  (OSRC)  | Registergericht München, HRB Nr. 43632