[PATCH 1/2] Export cpu info by sysfs

[PATCH 1/2] Export cpu info by sysfs

[Zhang, Yanmin]

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

I worked out 2 patches to export cpu topology and cache info by sysfs.

The first patch is to export cpu topology info including below items
(attributes) which are similar to /proc/cpuinfo.

/sys/devices/system/cpu/cpuX/topology/physical_package_id(representing
the physical package id of  cpu X)
/sys/devices/system/cpu/cpuX/topology/core_id (representing the cpu core
id  to cpu X)
/sys/devices/system/cpu/cpuX/topology/thread_id (representing the cpu
thread id  to cpu X)
/sys/devices/system/cpu/cpuX/topology/thread_siblings (representing the
thread siblings to cpu X)
/sys/devices/system/cpu/cpuX/topology/core_siblings (represeting the
core siblings to cpu X)

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


[-- Attachment #2: export_topology_2.6.14_mm1_ia64.v5.patch --]
[-- Type: application/octet-stream, Size: 3855 bytes --]

diff -Nraup linux-2.6.14_mm1/arch/ia64/kernel/topology.c linux-2.6.14_mm1_topology/arch/ia64/kernel/topology.c
--- linux-2.6.14_mm1/arch/ia64/kernel/topology.c	2005-11-07 02:34:06.000000000 +0800
+++ linux-2.6.14_mm1_topology/arch/ia64/kernel/topology.c	2005-12-13 18:25:08.000000000 +0800
@@ -98,4 +98,151 @@ out:
 	return err;
 }
 
-__initcall(topology_init);
+subsys_initcall(topology_init);
+
+
+#if defined(CONFIG_SYSFS)
+
+#if defined(CONFIG_SMP)
+
+/*
+ * Export cpu topology through sysfs
+ */
+
+/* pointer to kobject for cpuX/topology */
+static struct kobject * cpu_topology_kobject;
+
+#define define_one_ro(_name) \
+static struct attribute _name = \
+	{.name = __stringify(_name), .mode = 0444, .owner = THIS_MODULE}
+
+define_one_ro(physical_package_id);
+define_one_ro(core_id);
+define_one_ro(thread_id);
+define_one_ro(thread_siblings);
+define_one_ro(core_siblings);
+
+static struct attribute * topology_default_attrs[] = {
+	&physical_package_id,
+	&core_id,
+	&thread_id,
+	&thread_siblings,
+	&core_siblings,
+	NULL
+};
+
+static ssize_t topology_show(struct kobject * kobj, struct attribute * attr, char * buf)
+{
+	unsigned int cpu;
+	ssize_t len = -1;
+
+	cpu = container_of(kobj->parent, struct sys_device, kobj)->id;
+
+	if (attr == &physical_package_id)
+		return sprintf(buf, "%d\n", cpu_data(cpu)->socket_id);
+	if (attr == &core_id)
+		return sprintf(buf, "%d\n", cpu_data(cpu)->core_id);
+	if (attr == &thread_id)
+		return sprintf(buf, "%d\n", cpu_data(cpu)->thread_id);
+	if (attr == &thread_siblings) {
+		len = cpumask_scnprintf(buf, NR_CPUS+1, cpu_sibling_map[cpu]);
+		len += sprintf (buf + len, "\n");
+		return len;
+	}
+	if (attr == &core_siblings) {
+		len = cpumask_scnprintf(buf, NR_CPUS+1, cpu_core_map[cpu]);
+		len += sprintf (buf + len, "\n");
+		return len;
+	}
+
+	return len;
+}
+
+static ssize_t topology_store(struct kobject * kobj, struct attribute * attr,
+		     const char * buf, size_t count)
+{
+	return 0;
+}
+
+static struct sysfs_ops topology_sysfs_ops = {
+	.show   = topology_show,
+	.store  = topology_store,
+};
+
+static struct kobj_type topology_ktype = {
+	.sysfs_ops	= &topology_sysfs_ops,
+	.default_attrs	= topology_default_attrs,
+};
+
+/* Add/Remove cpu_topology interface for CPU device */
+static int __cpuinit topology_add_dev(struct sys_device * sys_dev)
+{
+	unsigned int cpu = sys_dev->id;
+
+	memset(&cpu_topology_kobject[cpu], 0, sizeof(struct kobject));
+
+	cpu_topology_kobject[cpu].parent = &sys_dev->kobj;
+	kobject_set_name(&cpu_topology_kobject[cpu], "%s", "topology");
+	cpu_topology_kobject[cpu].ktype = &topology_ktype;
+
+	return  kobject_register(&cpu_topology_kobject[cpu]);
+}
+
+static int __cpuexit topology_remove_dev(struct sys_device * sys_dev)
+{
+	unsigned int cpu = sys_dev->id;
+
+	kobject_unregister(&cpu_topology_kobject[cpu]);
+
+	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;
+#ifdef	CONFIG_HOTPLUG_CPU
+		case CPU_DEAD:
+			topology_remove_dev(sys_dev);
+			break;
+#endif
+	}
+	return NOTIFY_OK;
+}
+
+static struct notifier_block topology_cpu_notifier =
+{
+	.notifier_call = topology_cpu_callback,
+};
+
+static int __cpuinit topology_sysfs_init(void)
+{
+	int i;
+
+	cpu_topology_kobject = kmalloc(sizeof(struct kobject) * NR_CPUS,
+			GFP_KERNEL);
+	if (!cpu_topology_kobject)
+		return -ENOMEM;
+
+	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);
+
+#endif //CONFIG_SMP
+#endif	//CONFIG_SYSFS
+

Re: [PATCH 1/2] Export cpu info by sysfs

[Nathan Lynch]

Hello-

Zhang, Yanmin wrote:
> I worked out 2 patches to export cpu topology and cache info by sysfs.
> 
> The first patch is to export cpu topology info including below items
> (attributes) which are similar to /proc/cpuinfo.
> 
> /sys/devices/system/cpu/cpuX/topology/physical_package_id(representing
> the physical package id of  cpu X)
> /sys/devices/system/cpu/cpuX/topology/core_id (representing the cpu core
> id  to cpu X)
> /sys/devices/system/cpu/cpuX/topology/thread_id (representing the cpu
> thread id  to cpu X)
> /sys/devices/system/cpu/cpuX/topology/thread_siblings (representing the
> thread siblings to cpu X)
> /sys/devices/system/cpu/cpuX/topology/core_siblings (represeting the
> core siblings to cpu X)

I haven't looked at the patches in detail, but I have a concern about
this approach.  How is it that making new architecture-specific
attributes under cpu directories in sysfs is preferable to the already
architecture-specific format of /proc/cpuinfo and other proc entries?

If we're going to create a new user interface for exposing system
topology (cores and threads etc), I would like for it to be as
architecture-neutral as possible.  We already do this for numa, for
example.


Nathan

Re: [PATCH 1/2] Export cpu info by sysfs

[Paul Jackson]

Some comments on your patch ...

1) It's easier for others to read patches if they are inline text,
   or at least attached as text, not as base64.  See further the
   kernel source file: Documentation/SubmittingPatches.  If your
   company's email client has difficulty attaching patches without
   mangling them, then perhaps you will have better luck with a
   dedicated patch sending program, such as one I support:
	http://www.speakeasy.org/~pj99/sgi/sendpatchset

2) > cpumask_scnprintf(buf, NR_CPUS+1, cpu_core_map[cpu]);

   The 2nd arg, "NR_CPUS+1", is wrong.  It should be the length
   of the buffer (1st arg, "buf").  Unfortunately, you probably
   aren't passed that length by sysfs.  Your routine was likely
   passed a page, so assuming a size of PAGE_SIZE/2 would work
   (big enough to print a cpumask, small enough not to allow
   buffer overrun.)

3) The patch needs to include reasonable documentation (not
   just the patch header that goes in the source control log,
   but also documentation that will into the source file and/or
   into the Documentation directory.)  Unfortunately, it seems
   that the rest of /sys/devices/system is not directly documented
   under Documentation, except as it pertains to such subjects as
   cpufreq, laptop, numastat and hotplug.  So until someone takes
   on the challenge of documenting the rest of this /sys hierarchy,
   I see no obvious place to add such items as you propose.

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@sgi.com> 1.925.600.0401

RE: [PATCH 1/2] Export cpu info by sysfs

[Zhang, Yanmin]

>>-----Original Message-----
>>From: Paul Jackson [mailto:pj@sgi.com]
>>Sent: 2005年12月14日 18:12
>>To: Zhang, Yanmin
>>Cc: linux-kernel@vger.kernel.org; Pallipadi, Venkatesh
>>Subject: Re: [PATCH 1/2] Export cpu info by sysfs
>>
>>Some comments on your patch ...
I really appreciate your comments.

>>
>>1) It's easier for others to read patches if they are inline text,
>>   or at least attached as text, not as base64.  See further the
>>   kernel source file: Documentation/SubmittingPatches.  If your
>>   company's email client has difficulty attaching patches without
>>   mangling them, then perhaps you will have better luck with a
>>   dedicated patch sending program, such as one I support:
>>	http://www.speakeasy.org/~pj99/sgi/sendpatchset
Thanks. I use outlook and tried many approaches before, but patches always have unexpected line breaks when I attach them as inline text. Anyway, I found a new approach to avoid that. Next time, I will paste patches as inline.

>>
>>2) > cpumask_scnprintf(buf, NR_CPUS+1, cpu_core_map[cpu]);
>>
>>   The 2nd arg, "NR_CPUS+1", is wrong.  It should be the length
>>   of the buffer (1st arg, "buf").  Unfortunately, you probably
>>   aren't passed that length by sysfs.  Your routine was likely
>>   passed a page, so assuming a size of PAGE_SIZE/2 would work
>>   (big enough to print a cpumask, small enough not to allow
>>   buffer overrun.)
In theory, it's a problem which doesn't exist in fact. The smallest page size on IA64 is 4KB (default is 16KB) and cpumask_scnprintf uses hex format, so one page could store cpumask of (4K-1)*4 cpu. I can't imagine a machine has more than (4K-1)*4 cpu.

>>
>>3) The patch needs to include reasonable documentation (not
>>   just the patch header that goes in the source control log,
>>   but also documentation that will into the source file and/or
>>   into the Documentation directory.)  Unfortunately, it seems
>>   that the rest of /sys/devices/system is not directly documented
>>   under Documentation, except as it pertains to such subjects as
>>   cpufreq, laptop, numastat and hotplug.  So until someone takes
>>   on the challenge of documenting the rest of this /sys hierarchy,
>>   I see no obvious place to add such items as you propose.
I agree with you.

Re: [PATCH 1/2] Export cpu info by sysfs

[Paul Jackson]

Yanmin wrote:
> cpumask_scnprintf(buf, NR_CPUS+1, cpu_core_map[cpu]);

Paul wrote:
> The 2nd arg, "NR_CPUS+1", is wrong.  It should be the length
> of the buffer

Yanmin replied:
> In theory, it's a problem which doesn't exist in fact.

Aha - you are right.  My confusion was that I had forgotten
the format used by cpumask_scnprintf, and thought it was one
of the other formats, not a hex mask, that could overflow
NR_CPUS+1 characters of output.  Please nevermind my confusion.

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@sgi.com> 1.925.600.0401