RE: [PATCH v2:3/3]Export cpu topology by sysfs

RE: [PATCH v2:3/3]Export cpu topology by sysfs

[Zhang, Yanmin]

>>-----Original Message-----
>>From: linux-ia64-owner@vger.kernel.org [mailto:linux-ia64-owner@vger.kernel.org] On Behalf Of Nathan Lynch
>>Sent: 2005年12月29日 3:14
>>To: Zhang, Yanmin
>>Cc: Yanmin Zhang; greg@kroah.com; linux-kernel@vger.kernel.org; discuss@x86-64.org; linux-ia64@vger.kernel.org; Siddha, Suresh B; Shah,
>>Rajesh; Pallipadi, Venkatesh
>>Subject: Re: [PATCH v2:3/3]Export cpu topology by sysfs
>>
>>
>>> >>What about sane default values for the *_id attributes?
>>Not really -- inevitably we'll wind up with an interface that has
>>slightly different semantics on each architecture.
How about below arrangement for default values?
1) physical_package_id: If cpu has no physical package id, the logical cpu id will be picked up.
2) core_id: 0. If cpu doesn't support multi-core, its core id is 0. 
3) thread_siblings: Just include itself, if the cpu doesn't support HT/multi-thread.
4) core_siblings: Just include itself, if the cpu doesn't support multi-core and HT.


>>
>>> >>Hmm, why should thread_id be exported at all?  Is it useful to
>>> >>userspace in a way that the logical cpu id is not?
>>>
>>> Just to make it clearer. Of course, physical_package_id /core_id/
>>> logical cpu id could tell enough info like thread id.
>>
>>Then let's drop thread_id until there's a need for it.
Ok. 


>>> >>> +	switch (action) {
>>> >>> +		case CPU_ONLINE:
>>> >>> +			topology_add_dev(sys_dev);
>>> >>> +			break;
>>> >>> +		case CPU_DEAD:
>>> >>> +			topology_remove_dev(sys_dev);
>>> >>> +			break;
>>> >>> +	}
>>> >>> +	return NOTIFY_OK;
>>> >>> +}
>>> >>
>>> >>I don't think it makes much sense to add and remove the attribute
>>> >>group for cpu online/offline events.  The topology information for an
>>> >>offline cpu is potentially valuable -- it could help the admin decide
>>> >>which processor to online at runtime, for example.
>>> >>
>>> >>I believe the correct time to update the topology information is when
>>> >>the topology actually changes (e.g. physical addition or removal of a
>>> >>processor) -- this is independent of online/offline operations.
>>>
>>> Currently, on i386/x86_64/ia64, a cpu gets its own topology by
>>> itself and fills into a global array. If the cpu is offline since
>>> the machine is booted, we can't get its topology info.
>>
>>Hmm, is this a limitation of those architectures?  On ppc64 (where
>>this feature would be applicable) Open Firmware provides such topology
>>information regardless of the cpus' states; does the firmware or ACPI
>>on these platforms not do the same?
On I386/x86_64/IA64, MADT, an ACPI table, provides apic id for all cpus. But we can't divide it to get core id and thread id becasue we couldn't know how many bits of apic id are for core id and how many bits are for thread id. These info are got only by executing cupid (on i386/x86_64) or PAL call (on ia64) by the online cpu itself.

>>
>>> And when a cpu is offline, current kernel deletes it from the
>>> thread_siblings and core_siblings of other cpu.
>>
>>That's fine -- I'm just arguing against the addition/removal of the
>>attributes when cpus go online and offline.
Based on above discussion, if the attributes of the cpu is not removed when the cpu is offline, the attribute values might be incorrect when the cpu is not up since machine boots.

Re: [PATCH v3]Export cpu topology by sysfs

[Yanmin Zhang]

Here is version 3. Based on Nathan's comments, the main changes are:
1) Drop thread id, so the first 2 patches of version 2 are dropped;
2) Set consistent default values for the 4 attributes.

---Detailed descriptions---
From: Zhang, Yanmin <yanmin.zhang@intel.com>

The patch implements the topology exportation by 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;
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;

To implement it in an architecture-neutral way, a new source file,
driver/base/topology.c, is to export the 5 attributes.

If one architecture wants to support this feature, it just needs to
implement 4 defines, typically in file include/asm-XXX/topology.h.
The 4 defines are:
#define topology_physical_package_id(cpu)
#define topology_core_id(cpu)
#define topology_thread_siblings(cpu)
#define topology_core_siblings(cpu)

The type of **_id is int.
The type of siblings is cpumask_t.

To be consistent on all architectures, the 4 attributes should have
deafult values if their values are unavailable. Below is the rule.
1) physical_package_id: If cpu has no physical package id, -1 is the
default value.
2) core_id: If cpu doesn't support multi-core, its core id is 0.
3) thread_siblings: Just include itself, if the cpu doesn't support
HT/multi-thread.
4) core_siblings: Just include itself, if the cpu doesn't support
multi-core and HT/Multi-thread.

So be careful when declaring the 4 defines in include/asm-XXX/topology.h.

If an attribute isn't defined on an architecture, it won't be exported.

Thank Nathan, Greg, Andi, Paul and Venki.

The patch against 2.6.15-rc5 provides defines for i386/x86_64/ia64.

Signed-off-by: Zhang, Yanmin <yanmin.zhang@intel.com>

---

diff -Nraup linux-2.6.15-rc5/arch/ia64/kernel/topology.c linux-2.6.15-rc5_topology/arch/ia64/kernel/topology.c
--- linux-2.6.15-rc5/arch/ia64/kernel/topology.c	2005-11-07 02:34:06.000000000 +0800
+++ linux-2.6.15-rc5_topology/arch/ia64/kernel/topology.c	2006-01-03 22:54:40.000000000 +0800
@@ -98,4 +98,4 @@ out:
 	return err;
 }
 
-__initcall(topology_init);
+subsys_initcall(topology_init);
diff -Nraup linux-2.6.15-rc5/Documentation/cputopology.txt linux-2.6.15-rc5_topology/Documentation/cputopology.txt
--- linux-2.6.15-rc5/Documentation/cputopology.txt	1970-01-01 08:00:00.000000000 +0800
+++ linux-2.6.15-rc5_topology/Documentation/cputopology.txt	2006-01-04 00:54:38.000000000 +0800
@@ -0,0 +1,41 @@
+
+Export cpu topology info by 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;
+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;
+
+To implement it in an architecture-neutral way, a new source file,
+driver/base/topology.c, is to export the 5 attributes.
+
+If one architecture wants to support this feature, it just needs to
+implement 4 defines, typically in file include/asm-XXX/topology.h.
+The 4 defines are:
+#define topology_physical_package_id(cpu)
+#define topology_core_id(cpu)
+#define topology_thread_siblings(cpu)
+#define topology_core_siblings(cpu)
+
+The type of **_id is int.
+The type of siblings is cpumask_t.
+
+To be consistent on all architectures, the 4 attributes should have
+deafult values if their values are unavailable. Below is the rule.
+1) physical_package_id: If cpu has no physical package id, -1 is the
+default value.
+2) core_id: If cpu doesn't support multi-core, its core id is 0.
+3) thread_siblings: Just include itself, if the cpu doesn't support
+HT/multi-thread.
+4) core_siblings: Just include itself, if the cpu doesn't support
+multi-core and HT/Multi-thread.
+
+So be careful when declaring the 4 defines in include/asm-XXX/topology.h.
+
+If an attribute isn't defined on an architecture, it won't be exported.
+
diff -Nraup linux-2.6.15-rc5/drivers/base/Makefile linux-2.6.15-rc5_topology/drivers/base/Makefile
--- linux-2.6.15-rc5/drivers/base/Makefile	2005-12-13 23:07:35.000000000 +0800
+++ linux-2.6.15-rc5_topology/drivers/base/Makefile	2006-01-03 22:54:40.000000000 +0800
@@ -8,6 +8,7 @@ obj-y			+= power/
 obj-$(CONFIG_FW_LOADER)	+= firmware_class.o
 obj-$(CONFIG_NUMA)	+= node.o
 obj-$(CONFIG_MEMORY_HOTPLUG) += memory.o
+obj-$(CONFIG_SMP)	+= topology.o
 
 ifeq ($(CONFIG_DEBUG_DRIVER),y)
 EXTRA_CFLAGS += -DDEBUG
diff -Nraup linux-2.6.15-rc5/drivers/base/topology.c linux-2.6.15-rc5_topology/drivers/base/topology.c
--- linux-2.6.15-rc5/drivers/base/topology.c	1970-01-01 08:00:00.000000000 +0800
+++ linux-2.6.15-rc5_topology/drivers/base/topology.c	2006-01-04 00:54:48.000000000 +0800
@@ -0,0 +1,148 @@
+/*
+ * driver/base/topology.c - Populate sysfs with cpu topology information
+ *
+ * Written by: Zhang Yanmin, Intel Corporation
+ *
+ * Copyright (C) 2006, Intel Corp.
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
+ * NON INFRINGEMENT.  See the GNU General Public License for more
+ * details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ *
+ */
+#include <linux/sysdev.h>
+#include <linux/init.h>
+#include <linux/mm.h>
+#include <linux/cpu.h>
+#include <linux/module.h>
+#include <linux/topology.h>
+
+#define define_one_ro(_name) 		\
+static SYSDEV_ATTR(_name, 0444, show_##_name, NULL)
+
+#define define_id_show_func(name)				\
+static ssize_t show_##name(struct sys_device *dev, char *buf)	\
+{								\
+	unsigned int cpu = dev->id;				\
+	return sprintf(buf, "%d\n", topology_##name(cpu));	\
+}
+
+#define define_siblings_show_func(name)					\
+static ssize_t show_##name(struct sys_device *dev, char *buf)		\
+{									\
+	ssize_t len = -1;						\
+	unsigned int cpu = dev->id;					\
+	len = cpumask_scnprintf(buf, NR_CPUS+1, topology_##name(cpu));	\
+	return (len + sprintf(buf + len, "\n"));			\
+}
+
+#ifdef	topology_physical_package_id
+define_id_show_func(physical_package_id);
+define_one_ro(physical_package_id);
+#define ref_physical_package_id_attr	&attr_physical_package_id.attr,
+#else
+#define ref_physical_package_id_attr
+#endif
+
+#ifdef topology_core_id
+define_id_show_func(core_id);
+define_one_ro(core_id);
+#define ref_core_id_attr		&attr_core_id.attr,
+#else
+#define ref_core_id_attr
+#endif
+
+#ifdef topology_thread_siblings
+define_siblings_show_func(thread_siblings);
+define_one_ro(thread_siblings);
+#define ref_thread_siblings_attr	&attr_thread_siblings.attr,
+#else
+#define ref_thread_siblings_attr
+#endif
+
+#ifdef topology_core_siblings
+define_siblings_show_func(core_siblings);
+define_one_ro(core_siblings);
+#define ref_core_siblings_attr		&attr_core_siblings.attr,
+#else
+#define ref_core_siblings_attr
+#endif
+
+static struct attribute * default_attrs[] = {
+	ref_physical_package_id_attr
+	ref_core_id_attr
+	ref_thread_siblings_attr
+	ref_core_siblings_attr
+	NULL
+};
+
+static struct attribute_group topology_attr_group = {
+	.attrs = default_attrs,
+	.name = "topology"
+};
+
+/* Add/Remove cpu_topology interface for CPU device */
+static int __cpuinit topology_add_dev(struct sys_device * sys_dev)
+{
+	sysfs_create_group(&sys_dev->kobj, &topology_attr_group);
+	return 0;
+}
+
+static int __cpuinit topology_remove_dev(struct sys_device * sys_dev)
+{
+	sysfs_remove_group(&sys_dev->kobj, &topology_attr_group);
+	return 0;
+}
+
+static int __cpuinit topology_cpu_callback(struct notifier_block *nfb,
+		unsigned long action, void *hcpu)
+{
+	unsigned int cpu = (unsigned long)hcpu;
+	struct sys_device *sys_dev;
+
+	sys_dev = get_cpu_sysdev(cpu);
+	switch (action) {
+		case CPU_ONLINE:
+			topology_add_dev(sys_dev);
+			break;
+		case CPU_DEAD:
+			topology_remove_dev(sys_dev);
+			break;
+	}
+	return NOTIFY_OK;
+}
+
+static struct notifier_block topology_cpu_notifier =
+{
+	.notifier_call = topology_cpu_callback,
+};
+
+static int __cpuinit topology_sysfs_init(void)
+{
+	int i;
+
+	for_each_online_cpu(i) {
+		topology_cpu_callback(&topology_cpu_notifier, CPU_ONLINE,
+				(void *)(long)i);
+	}
+
+	register_cpu_notifier(&topology_cpu_notifier);
+
+	return 0;
+}
+
+device_initcall(topology_sysfs_init);
+
diff -Nraup linux-2.6.15-rc5/include/asm-i386/topology.h linux-2.6.15-rc5_topology/include/asm-i386/topology.h
--- linux-2.6.15-rc5/include/asm-i386/topology.h	2005-11-07 02:34:08.000000000 +0800
+++ linux-2.6.15-rc5_topology/include/asm-i386/topology.h	2006-01-04 00:16:13.000000000 +0800
@@ -27,6 +27,15 @@
 #ifndef _ASM_I386_TOPOLOGY_H
 #define _ASM_I386_TOPOLOGY_H
 
+#ifdef CONFIG_SMP
+#define topology_physical_package_id(cpu)				\
+	((int)(phys_proc_id[cpu] == BAD_APICID ? -1 : phys_proc_id[cpu]))
+#define topology_core_id(cpu)						\
+	((int)(cpu_core_id[cpu] == BAD_APICID ? 0 : cpu_core_id[cpu]))
+#define topology_core_siblings(cpu)		cpu_core_map[cpu]
+#define topology_thread_siblings(cpu)		cpu_sibling_map[cpu]
+#endif
+
 #ifdef CONFIG_NUMA
 
 #include <asm/mpspec.h>
diff -Nraup linux-2.6.15-rc5/include/asm-ia64/topology.h linux-2.6.15-rc5_topology/include/asm-ia64/topology.h
--- linux-2.6.15-rc5/include/asm-ia64/topology.h	2005-11-07 02:34:08.000000000 +0800
+++ linux-2.6.15-rc5_topology/include/asm-ia64/topology.h	2006-01-04 00:07:47.000000000 +0800
@@ -100,6 +100,13 @@ void build_cpu_to_node_map(void);
 
 #endif /* CONFIG_NUMA */
 
+#ifdef CONFIG_SMP
+#define topology_physical_package_id(cpu)	cpu_data(cpu)->socket_id
+#define topology_core_id(cpu)			cpu_data(cpu)->core_id
+#define topology_core_siblings(cpu)		cpu_core_map[cpu]
+#define topology_thread_siblings(cpu)		cpu_sibling_map[cpu]
+#endif
+
 #include <asm-generic/topology.h>
 
 #endif /* _ASM_IA64_TOPOLOGY_H */
diff -Nraup linux-2.6.15-rc5/include/asm-x86_64/topology.h linux-2.6.15-rc5_topology/include/asm-x86_64/topology.h
--- linux-2.6.15-rc5/include/asm-x86_64/topology.h	2005-12-13 23:07:39.000000000 +0800
+++ linux-2.6.15-rc5_topology/include/asm-x86_64/topology.h	2006-01-04 00:14:22.000000000 +0800
@@ -58,6 +58,15 @@ extern int __node_distance(int, int);
 
 #endif
 
+#ifdef CONFIG_SMP
+#define topology_physical_package_id(cpu)				\
+	((int)(phys_proc_id[cpu] == BAD_APICID ? -1 : phys_proc_id[cpu]))
+#define topology_core_id(cpu)						\
+	((int)(cpu_core_id[cpu] == BAD_APICID ? 0 : cpu_core_id[cpu]))
+#define topology_core_siblings(cpu)		cpu_core_map[cpu]
+#define topology_thread_siblings(cpu)		cpu_sibling_map[cpu]
+#endif
+
 #include <asm-generic/topology.h>
 
 #endif

Re: [PATCH v2:3/3]Export cpu topology by sysfs

[Nathan Lynch]

Zhang, Yanmin wrote:
> >>> >>I don't think it makes much sense to add and remove the attribute
> >>> >>group for cpu online/offline events.  The topology information for an
> >>> >>offline cpu is potentially valuable -- it could help the admin decide
> >>> >>which processor to online at runtime, for example.
> >>> >>
> >>> >>I believe the correct time to update the topology information is when
> >>> >>the topology actually changes (e.g. physical addition or removal of a
> >>> >>processor) -- this is independent of online/offline operations.
> >>>
> >>> Currently, on i386/x86_64/ia64, a cpu gets its own topology by
> >>> itself and fills into a global array. If the cpu is offline since
> >>> the machine is booted, we can't get its topology info.
> >>
> >>Hmm, is this a limitation of those architectures?  On ppc64 (where
> >>this feature would be applicable) Open Firmware provides such topology
> >>information regardless of the cpus' states; does the firmware or ACPI
> >>on these platforms not do the same?
>
> On I386/x86_64/IA64, MADT, an ACPI table, provides apic id for all
> cpus. But we can't divide it to get core id and thread id becasue we
> couldn't know how many bits of apic id are for core id and how many
> bits are for thread id. These info are got only by executing cupid
> (on i386/x86_64) or PAL call (on ia64) by the online cpu itself.

In practice does the division of bits between core and thread in the
apic id differ between cpus in the same system?  Is there really no
way to discover a cpu's core and thread id without onlining it on
these platforms?

Re: [PATCH v3]Export cpu topology by sysfs

[Nathan Lynch]

(sorry for the delay in response)

Yanmin Zhang wrote:
> Here is version 3. Based on Nathan's comments, the main changes are:
> 1) Drop thread id, so the first 2 patches of version 2 are dropped;

Thanks.

> 2) Set consistent default values for the 4 attributes.
> 

<snip>

> If one architecture wants to support this feature, it just needs to
> implement 4 defines, typically in file include/asm-XXX/topology.h.
> The 4 defines are:
> #define topology_physical_package_id(cpu)
> #define topology_core_id(cpu)
> #define topology_thread_siblings(cpu)
> #define topology_core_siblings(cpu)
> 
> The type of **_id is int.
> The type of siblings is cpumask_t.
> 
> To be consistent on all architectures, the 4 attributes should have
> deafult values if their values are unavailable. Below is the rule.
> 1) physical_package_id: If cpu has no physical package id, -1 is the
> default value.
> 2) core_id: If cpu doesn't support multi-core, its core id is 0.

Why not -1 as with the physical package id?  0 could be a valid core
id.

> 3) thread_siblings: Just include itself, if the cpu doesn't support
> HT/multi-thread.
> 4) core_siblings: Just include itself, if the cpu doesn't support
> multi-core and HT/Multi-thread.

Really, I think the least confusing interface would not export those
attributes which are not relevant for the system.  E.g. if the system
isn't multi-core, you don't see core_id and core_siblings attributes.

Failing that, let's at least have consistent, unambiguous values for
the ids which are not applicable.

<snip>
> +static int __cpuinit topology_cpu_callback(struct notifier_block *nfb,
> +		unsigned long action, void *hcpu)
> +{
> +	unsigned int cpu = (unsigned long)hcpu;
> +	struct sys_device *sys_dev;
> +
> +	sys_dev = get_cpu_sysdev(cpu);
> +	switch (action) {
> +		case CPU_ONLINE:
> +			topology_add_dev(sys_dev);
> +			break;
> +		case CPU_DEAD:
> +			topology_remove_dev(sys_dev);
> +			break;
> +	}
> +	return NOTIFY_OK;
> +}

I still oppose this bit.  I want the attributes there for powerpc even
for offline cpus -- the topology information (if obtainable, which it
is on powerpc) is useful regardless of the cpu's online state.  The
attributes should appear when a cpu is made present, and go away when
a cpu is removed.

This week I'll try to do an implementation for powerpc.

RE: [PATCH v3]Export cpu topology by sysfs

[Zhang, Yanmin]

>>-----Original Message-----
>>From: linux-ia64-owner@vger.kernel.org [mailto:linux-ia64-owner@vger.kernel.org] On Behalf Of Nathan Lynch
>>Sent: 2006年1月9日 13:10
>>To: Yanmin Zhang
>>Cc: linux-kernel@vger.kernel.org; greg@kroah.com; discuss@x86-64.org; linux-ia64@vger.kernel.org; Siddha, Suresh B; Shah, Rajesh;
>>Pallipadi, Venkatesh
>>Subject: Re: [PATCH v3]Export cpu topology by sysfs
>>> 2) Set consistent default values for the 4 attributes.
>>>
>>
>><snip>
>>
>>> If one architecture wants to support this feature, it just needs to
>>> implement 4 defines, typically in file include/asm-XXX/topology.h.
>>> The 4 defines are:
>>> #define topology_physical_package_id(cpu)
>>> #define topology_core_id(cpu)
>>> #define topology_thread_siblings(cpu)
>>> #define topology_core_siblings(cpu)
>>>
>>> The type of **_id is int.
>>> The type of siblings is cpumask_t.
>>>
>>> To be consistent on all architectures, the 4 attributes should have
>>> deafult values if their values are unavailable. Below is the rule.
>>> 1) physical_package_id: If cpu has no physical package id, -1 is the
>>> default value.
>>> 2) core_id: If cpu doesn't support multi-core, its core id is 0.
>>
>>Why not -1 as with the physical package id?  0 could be a valid core
>>id.
If the cpu has only 1 core, we could call it single-core, so it's reasonable to use 0 as its core id.


>>
>>> 3) thread_siblings: Just include itself, if the cpu doesn't support
>>> HT/multi-thread.
>>> 4) core_siblings: Just include itself, if the cpu doesn't support
>>> multi-core and HT/Multi-thread.
>>
>>Really, I think the least confusing interface would not export those
>>attributes which are not relevant for the system.  E.g. if the system
>>isn't multi-core, you don't see core_id and core_siblings attributes.
>>
>>Failing that, let's at least have consistent, unambiguous values for
>>the ids which are not applicable.
Current kernel will output core id by /proc/cpuinfo if a physical cpu has 2 threads, no matter if it's a multi-core, or just a multi-thread. To be consistent with /proc/cpuinfo, I think we need export core id and its default value is 0.

>>
>><snip>
>>> +static int __cpuinit topology_cpu_callback(struct notifier_block *nfb,
>>> +		unsigned long action, void *hcpu)
>>> +{
>>> +	unsigned int cpu = (unsigned long)hcpu;
>>> +	struct sys_device *sys_dev;
>>> +
>>> +	sys_dev = get_cpu_sysdev(cpu);
>>> +	switch (action) {
>>> +		case CPU_ONLINE:
>>> +			topology_add_dev(sys_dev);
>>> +			break;
>>> +		case CPU_DEAD:
>>> +			topology_remove_dev(sys_dev);
>>> +			break;
>>> +	}
>>> +	return NOTIFY_OK;
>>> +}
>>
>>I still oppose this bit.  I want the attributes there for powerpc even
>>for offline cpus -- the topology information (if obtainable, which it
>>is on powerpc) is useful regardless of the cpu's online state.  The
>>attributes should appear when a cpu is made present, and go away when
>>a cpu is removed.
As my previous email says, there are concerns/issues to do so. A compromise is that the patch could register a sysdev driver. When the cpu becomes offline from online, we don't delete the topology kobj. The compromise has a defect. If the cpu is never online since machine boots, the topology info of the cpu is incorrect. 

>>
>>This week I'll try to do an implementation for powerpc.

RE: [PATCH v3]Export cpu topology by sysfs

[Zhang, Yanmin]

>>-----Original Message-----
>>From: linux-ia64-owner@vger.kernel.org [mailto:linux-ia64-owner@vger.kernel.org] On Behalf Of Zhang, Yanmin
>>Sent: 2006年1月9日 14:48
>>To: Nathan Lynch; Yanmin Zhang
>>Cc: linux-kernel@vger.kernel.org; greg@kroah.com; discuss@x86-64.org; linux-ia64@vger.kernel.org; Siddha, Suresh B; Shah, Rajesh;
>>Pallipadi, Venkatesh
>>Subject: RE: [PATCH v3]Export cpu topology by sysfs
>>
>>>>-----Original Message-----
>>>>From: linux-ia64-owner@vger.kernel.org [mailto:linux-ia64-owner@vger.kernel.org] On Behalf Of Nathan Lynch
>>>>Sent: 2006年1月9日 13:10
>>>>To: Yanmin Zhang
>>>>Cc: linux-kernel@vger.kernel.org; greg@kroah.com; discuss@x86-64.org; linux-ia64@vger.kernel.org; Siddha, Suresh B; Shah, Rajesh;
>>>>Pallipadi, Venkatesh
>>>>Subject: Re: [PATCH v3]Export cpu topology by sysfs
>>>>> 2) Set consistent default values for the 4 attributes.
>>>>>
>>>>
>>>><snip>
>>>>
>>>>> If one architecture wants to support this feature, it just needs to
>>>>> implement 4 defines, typically in file include/asm-XXX/topology.h.
>>>>> The 4 defines are:
>>>>> #define topology_physical_package_id(cpu)
>>>>> #define topology_core_id(cpu)
>>>>> #define topology_thread_siblings(cpu)
>>>>> #define topology_core_siblings(cpu)
>>>>>
>>>>> The type of **_id is int.
>>>>> The type of siblings is cpumask_t.
>>>>>
>>>>> To be consistent on all architectures, the 4 attributes should have
>>>>> deafult values if their values are unavailable. Below is the rule.
>>>>> 1) physical_package_id: If cpu has no physical package id, -1 is the
>>>>> default value.
>>>>> 2) core_id: If cpu doesn't support multi-core, its core id is 0.
>>>>
>>>>Why not -1 as with the physical package id?  0 could be a valid core
>>>>id.
>>If the cpu has only 1 core, we could call it single-core, so it's reasonable to use 0 as its core id.
>>
>>
>>>>
>>>>> 3) thread_siblings: Just include itself, if the cpu doesn't support
>>>>> HT/multi-thread.
>>>>> 4) core_siblings: Just include itself, if the cpu doesn't support
>>>>> multi-core and HT/Multi-thread.
>>>>
>>>>Really, I think the least confusing interface would not export those
>>>>attributes which are not relevant for the system.  E.g. if the system
>>>>isn't multi-core, you don't see core_id and core_siblings attributes.
>>>>
>>>>Failing that, let's at least have consistent, unambiguous values for
>>>>the ids which are not applicable.
>>Current kernel will output core id by /proc/cpuinfo if a physical cpu has 2 threads, no matter if it's a multi-core, or just a multi-thread.
>>To be consistent with /proc/cpuinfo, I think we need export core id and its default value is 0.
>>
>>>>
>>>><snip>
>>>>> +static int __cpuinit topology_cpu_callback(struct notifier_block *nfb,
>>>>> +		unsigned long action, void *hcpu)
>>>>> +{
>>>>> +	unsigned int cpu = (unsigned long)hcpu;
>>>>> +	struct sys_device *sys_dev;
>>>>> +
>>>>> +	sys_dev = get_cpu_sysdev(cpu);
>>>>> +	switch (action) {
>>>>> +		case CPU_ONLINE:
>>>>> +			topology_add_dev(sys_dev);
>>>>> +			break;
>>>>> +		case CPU_DEAD:
>>>>> +			topology_remove_dev(sys_dev);
>>>>> +			break;
>>>>> +	}
>>>>> +	return NOTIFY_OK;
>>>>> +}
>>>>
>>>>I still oppose this bit.  I want the attributes there for powerpc even
>>>>for offline cpus -- the topology information (if obtainable, which it
>>>>is on powerpc) is useful regardless of the cpu's online state.  The
>>>>attributes should appear when a cpu is made present, and go away when
>>>>a cpu is removed.
>>As my previous email says, there are concerns/issues to do so. A compromise is that the patch could register a sysdev driver. When the
>>cpu becomes offline from online, we don't delete the topology kobj. The compromise has a defect. If the cpu is never online since machine
>>boots, the topology info of the cpu is incorrect.
>>
>>>>
>>>>This week I'll try to do an implementation for powerpc.
Nathan,

Is your implementation for powerpc ready?

Andrew,
Could the patch of version 3 be put into mm tree firstly?

Thanks,
Yanmin

Re: [PATCH v3]Export cpu topology by sysfs

[Yanmin Zhang]

Resend the patch and comments because guys might miss it.

---Detailed descriptions---
From: Zhang, Yanmin <yanmin.zhang@intel.com>

The patch implements cpu topology exportation by 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;
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;

To implement it in an architecture-neutral way, a new source file,
driver/base/topology.c, is to export the 5 attributes.

If one architecture wants to support this feature, it just needs to
implement 4 defines, typically in file include/asm-XXX/topology.h.
The 4 defines are:
#define topology_physical_package_id(cpu)
#define topology_core_id(cpu)
#define topology_thread_siblings(cpu)
#define topology_core_siblings(cpu)

The type of **_id is int.
The type of siblings is cpumask_t.

To be consistent on all architectures, the 4 attributes should have
deafult values if their values are unavailable. Below is the rule.
1) physical_package_id: If cpu has no physical package id, -1 is the
default value.
2) core_id: If cpu doesn't support multi-core, its core id is 0.
3) thread_siblings: Just include itself, if the cpu doesn't support
HT/multi-thread.
4) core_siblings: Just include itself, if the cpu doesn't support
multi-core and HT/Multi-thread.

So be careful when declaring the 4 defines in include/asm-XXX/topology.h.

If an attribute isn't defined on an architecture, it won't be exported.

Thank Nathan, Greg, Andi, Paul and Venki.

The patch against 2.6.15-rc5 provides defines for i386/x86_64/ia64.

Signed-off-by: Zhang, Yanmin <yanmin.zhang@intel.com>

---

diff -Nraup linux-2.6.15-rc5/arch/ia64/kernel/topology.c linux-2.6.15-rc5_topology/arch/ia64/kernel/topology.c
--- linux-2.6.15-rc5/arch/ia64/kernel/topology.c	2005-11-07 02:34:06.000000000 +0800
+++ linux-2.6.15-rc5_topology/arch/ia64/kernel/topology.c	2006-01-03 22:54:40.000000000 +0800
@@ -98,4 +98,4 @@ out:
 	return err;
 }
 
-__initcall(topology_init);
+subsys_initcall(topology_init);
diff -Nraup linux-2.6.15-rc5/Documentation/cputopology.txt linux-2.6.15-rc5_topology/Documentation/cputopology.txt
--- linux-2.6.15-rc5/Documentation/cputopology.txt	1970-01-01 08:00:00.000000000 +0800
+++ linux-2.6.15-rc5_topology/Documentation/cputopology.txt	2006-01-04 00:54:38.000000000 +0800
@@ -0,0 +1,41 @@
+
+Export cpu topology info by 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;
+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;
+
+To implement it in an architecture-neutral way, a new source file,
+driver/base/topology.c, is to export the 5 attributes.
+
+If one architecture wants to support this feature, it just needs to
+implement 4 defines, typically in file include/asm-XXX/topology.h.
+The 4 defines are:
+#define topology_physical_package_id(cpu)
+#define topology_core_id(cpu)
+#define topology_thread_siblings(cpu)
+#define topology_core_siblings(cpu)
+
+The type of **_id is int.
+The type of siblings is cpumask_t.
+
+To be consistent on all architectures, the 4 attributes should have
+deafult values if their values are unavailable. Below is the rule.
+1) physical_package_id: If cpu has no physical package id, -1 is the
+default value.
+2) core_id: If cpu doesn't support multi-core, its core id is 0.
+3) thread_siblings: Just include itself, if the cpu doesn't support
+HT/multi-thread.
+4) core_siblings: Just include itself, if the cpu doesn't support
+multi-core and HT/Multi-thread.
+
+So be careful when declaring the 4 defines in include/asm-XXX/topology.h.
+
+If an attribute isn't defined on an architecture, it won't be exported.
+
diff -Nraup linux-2.6.15-rc5/drivers/base/Makefile linux-2.6.15-rc5_topology/drivers/base/Makefile
--- linux-2.6.15-rc5/drivers/base/Makefile	2005-12-13 23:07:35.000000000 +0800
+++ linux-2.6.15-rc5_topology/drivers/base/Makefile	2006-01-03 22:54:40.000000000 +0800
@@ -8,6 +8,7 @@ obj-y			+= power/
 obj-$(CONFIG_FW_LOADER)	+= firmware_class.o
 obj-$(CONFIG_NUMA)	+= node.o
 obj-$(CONFIG_MEMORY_HOTPLUG) += memory.o
+obj-$(CONFIG_SMP)	+= topology.o
 
 ifeq ($(CONFIG_DEBUG_DRIVER),y)
 EXTRA_CFLAGS += -DDEBUG
diff -Nraup linux-2.6.15-rc5/drivers/base/topology.c linux-2.6.15-rc5_topology/drivers/base/topology.c
--- linux-2.6.15-rc5/drivers/base/topology.c	1970-01-01 08:00:00.000000000 +0800
+++ linux-2.6.15-rc5_topology/drivers/base/topology.c	2006-01-04 00:54:48.000000000 +0800
@@ -0,0 +1,148 @@
+/*
+ * driver/base/topology.c - Populate sysfs with cpu topology information
+ *
+ * Written by: Zhang Yanmin, Intel Corporation
+ *
+ * Copyright (C) 2006, Intel Corp.
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
+ * NON INFRINGEMENT.  See the GNU General Public License for more
+ * details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ *
+ */
+#include <linux/sysdev.h>
+#include <linux/init.h>
+#include <linux/mm.h>
+#include <linux/cpu.h>
+#include <linux/module.h>
+#include <linux/topology.h>
+
+#define define_one_ro(_name) 		\
+static SYSDEV_ATTR(_name, 0444, show_##_name, NULL)
+
+#define define_id_show_func(name)				\
+static ssize_t show_##name(struct sys_device *dev, char *buf)	\
+{								\
+	unsigned int cpu = dev->id;				\
+	return sprintf(buf, "%d\n", topology_##name(cpu));	\
+}
+
+#define define_siblings_show_func(name)					\
+static ssize_t show_##name(struct sys_device *dev, char *buf)		\
+{									\
+	ssize_t len = -1;						\
+	unsigned int cpu = dev->id;					\
+	len = cpumask_scnprintf(buf, NR_CPUS+1, topology_##name(cpu));	\
+	return (len + sprintf(buf + len, "\n"));			\
+}
+
+#ifdef	topology_physical_package_id
+define_id_show_func(physical_package_id);
+define_one_ro(physical_package_id);
+#define ref_physical_package_id_attr	&attr_physical_package_id.attr,
+#else
+#define ref_physical_package_id_attr
+#endif
+
+#ifdef topology_core_id
+define_id_show_func(core_id);
+define_one_ro(core_id);
+#define ref_core_id_attr		&attr_core_id.attr,
+#else
+#define ref_core_id_attr
+#endif
+
+#ifdef topology_thread_siblings
+define_siblings_show_func(thread_siblings);
+define_one_ro(thread_siblings);
+#define ref_thread_siblings_attr	&attr_thread_siblings.attr,
+#else
+#define ref_thread_siblings_attr
+#endif
+
+#ifdef topology_core_siblings
+define_siblings_show_func(core_siblings);
+define_one_ro(core_siblings);
+#define ref_core_siblings_attr		&attr_core_siblings.attr,
+#else
+#define ref_core_siblings_attr
+#endif
+
+static struct attribute * default_attrs[] = {
+	ref_physical_package_id_attr
+	ref_core_id_attr
+	ref_thread_siblings_attr
+	ref_core_siblings_attr
+	NULL
+};
+
+static struct attribute_group topology_attr_group = {
+	.attrs = default_attrs,
+	.name = "topology"
+};
+
+/* Add/Remove cpu_topology interface for CPU device */
+static int __cpuinit topology_add_dev(struct sys_device * sys_dev)
+{
+	sysfs_create_group(&sys_dev->kobj, &topology_attr_group);
+	return 0;
+}
+
+static int __cpuinit topology_remove_dev(struct sys_device * sys_dev)
+{
+	sysfs_remove_group(&sys_dev->kobj, &topology_attr_group);
+	return 0;
+}
+
+static int __cpuinit topology_cpu_callback(struct notifier_block *nfb,
+		unsigned long action, void *hcpu)
+{
+	unsigned int cpu = (unsigned long)hcpu;
+	struct sys_device *sys_dev;
+
+	sys_dev = get_cpu_sysdev(cpu);
+	switch (action) {
+		case CPU_ONLINE:
+			topology_add_dev(sys_dev);
+			break;
+		case CPU_DEAD:
+			topology_remove_dev(sys_dev);
+			break;
+	}
+	return NOTIFY_OK;
+}
+
+static struct notifier_block topology_cpu_notifier =
+{
+	.notifier_call = topology_cpu_callback,
+};
+
+static int __cpuinit topology_sysfs_init(void)
+{
+	int i;
+
+	for_each_online_cpu(i) {
+		topology_cpu_callback(&topology_cpu_notifier, CPU_ONLINE,
+				(void *)(long)i);
+	}
+
+	register_cpu_notifier(&topology_cpu_notifier);
+
+	return 0;
+}
+
+device_initcall(topology_sysfs_init);
+
diff -Nraup linux-2.6.15-rc5/include/asm-i386/topology.h linux-2.6.15-rc5_topology/include/asm-i386/topology.h
--- linux-2.6.15-rc5/include/asm-i386/topology.h	2005-11-07 02:34:08.000000000 +0800
+++ linux-2.6.15-rc5_topology/include/asm-i386/topology.h	2006-01-04 00:16:13.000000000 +0800
@@ -27,6 +27,15 @@
 #ifndef _ASM_I386_TOPOLOGY_H
 #define _ASM_I386_TOPOLOGY_H
 
+#ifdef CONFIG_SMP
+#define topology_physical_package_id(cpu)				\
+	((int)(phys_proc_id[cpu] == BAD_APICID ? -1 : phys_proc_id[cpu]))
+#define topology_core_id(cpu)						\
+	((int)(cpu_core_id[cpu] == BAD_APICID ? 0 : cpu_core_id[cpu]))
+#define topology_core_siblings(cpu)		cpu_core_map[cpu]
+#define topology_thread_siblings(cpu)		cpu_sibling_map[cpu]
+#endif
+
 #ifdef CONFIG_NUMA
 
 #include <asm/mpspec.h>
diff -Nraup linux-2.6.15-rc5/include/asm-ia64/topology.h linux-2.6.15-rc5_topology/include/asm-ia64/topology.h
--- linux-2.6.15-rc5/include/asm-ia64/topology.h	2005-11-07 02:34:08.000000000 +0800
+++ linux-2.6.15-rc5_topology/include/asm-ia64/topology.h	2006-01-04 00:07:47.000000000 +0800
@@ -100,6 +100,13 @@ void build_cpu_to_node_map(void);
 
 #endif /* CONFIG_NUMA */
 
+#ifdef CONFIG_SMP
+#define topology_physical_package_id(cpu)	cpu_data(cpu)->socket_id
+#define topology_core_id(cpu)			cpu_data(cpu)->core_id
+#define topology_core_siblings(cpu)		cpu_core_map[cpu]
+#define topology_thread_siblings(cpu)		cpu_sibling_map[cpu]
+#endif
+
 #include <asm-generic/topology.h>
 
 #endif /* _ASM_IA64_TOPOLOGY_H */
diff -Nraup linux-2.6.15-rc5/include/asm-x86_64/topology.h linux-2.6.15-rc5_topology/include/asm-x86_64/topology.h
--- linux-2.6.15-rc5/include/asm-x86_64/topology.h	2005-12-13 23:07:39.000000000 +0800
+++ linux-2.6.15-rc5_topology/include/asm-x86_64/topology.h	2006-01-04 00:14:22.000000000 +0800
@@ -58,6 +58,15 @@ extern int __node_distance(int, int);
 
 #endif
 
+#ifdef CONFIG_SMP
+#define topology_physical_package_id(cpu)				\
+	((int)(phys_proc_id[cpu] == BAD_APICID ? -1 : phys_proc_id[cpu]))
+#define topology_core_id(cpu)						\
+	((int)(cpu_core_id[cpu] == BAD_APICID ? 0 : cpu_core_id[cpu]))
+#define topology_core_siblings(cpu)		cpu_core_map[cpu]
+#define topology_thread_siblings(cpu)		cpu_sibling_map[cpu]
+#endif
+
 #include <asm-generic/topology.h>
 
 #endif

Re: [PATCH v3]Export cpu topology by sysfs

[Nathan Lynch]

Zhang, Yanmin wrote:
> >>>>This week I'll try to do an implementation for powerpc.

> Is your implementation for powerpc ready?

No, I've been too busy with other things, sorry.