public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* RE: [PATCH v2:3/3]Export cpu topology by sysfs
@ 2005-12-26  7:38 Zhang, Yanmin
  2005-12-26  8:28 ` Yanmin Zhang
  2005-12-27  5:35 ` Greg KH
  0 siblings, 2 replies; 17+ messages in thread
From: Zhang, Yanmin @ 2005-12-26  7:38 UTC (permalink / raw)
  To: Greg KH
  Cc: Yanmin Zhang, linux-kernel, discuss, linux-ia64, Siddha, Suresh B,
	Shah, Rajesh, Pallipadi, Venkatesh

>>-----Original Message-----
>>From: Greg KH [mailto:greg@kroah.com]
>>Sent: 2005年12月24日 3:16
>>To: Zhang, Yanmin
>>Cc: Yanmin Zhang; 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
>>
>>On Fri, Dec 23, 2005 at 12:03:27PM +0800, Zhang, Yanmin wrote:
>>> >>Can't you just use an attribute group and attach it to the cpu kobject?
>>> >>That would save an array of kobjects I think.
>>> As you know, current i386/x86_64 arch also export cache info under
>>> /sys/device/system/cpu/cpuX/cache. Is it clearer to export topology
>>> under a new directory than under cpu directly?
>>
>>No, the place in the sysfs tree you are putting this is just fine.  I'm
>>just saying that you do not need to create a new kobject for these
>>attributes.  Just use an attribute group, and you will get the same
>>naming, without the need for an extra kobject (and the whole array of
>>kobjects) at all.
>>
>>Does that make more sense?
Yes, indeed. Now, I used your idea and the patch became simpler. Thanks.


>>
>>> >>> +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
>>> >>
>>> >>Why ifdef?  Isn't it safe to just always have this in?
>>> If no ifdef here, gcc reported a compiling warning when I compiled it
>>> on IA64 with CONFIG_HOTPLUG_CPU=n.
>>
>>Then you should probably go change it so that CPU_DEAD is defined on
>>non-smp builds, otherwise the code gets quite messy like the above :)

Sorry. My previous explanation is confusing. It's a link warning on ia64. On ia64, the kernel vmlinux doesn't include section .exit.text, so ld will report a link warning when a function is in section .exit.text and another function (not in .exit.text) calls the first one. When CONFIG_HOTPLUG_CPU=n, function topology_remove_dev is in section .exit.text, but its caller topology_remove_dev is not in .exit.text.

i386 and x86_64 kernel vmlinux does include .exit.text and discard it only when running, so there is no such warning on i386/x86_64.

There is no other better approach to get rid of the warning unless we change arch/ia64/kernel/vmlinux.lds.S to keep all .exit.text in kernel image.


^ permalink raw reply	[flat|nested] 17+ messages in thread
* RE: [PATCH v2:3/3]Export cpu topology by sysfs
@ 2006-01-09  6:35 Zhang, Yanmin
  2006-01-09 17:41 ` Russ Anderson
  0 siblings, 1 reply; 17+ messages in thread
From: Zhang, Yanmin @ 2006-01-09  6:35 UTC (permalink / raw)
  To: Nathan Lynch
  Cc: Yanmin Zhang, greg, linux-kernel, discuss, linux-ia64,
	Siddha, Suresh B, Shah, Rajesh, Pallipadi, Venkatesh

>>-----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日 12:52
>>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
>>
>>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?
On i386 and x86_64, I think the answer is no, so we could get core and thread id for offline cpu. But there are 2 concerns.
1) Current cpu hotplug supports to convert cpu state between online and cpuline. In the future, the implementation would add support conversion between non-present and present. If a cpu is not present, there might be no its corresponding MADT entry. Later, when the cpu becomes present, ACPI needs provide its apic id dynamically. As for this case, we still can't get the non-present cpu id.
2) On ia64, ia64 manual says that cpu physical id/core id/thread id are got by pal call. It doesn't mention how to divide apic id to get physical id/core id/thread id.

  Is there really no
>>way to discover a cpu's core and thread id without onlining it on
>>these platforms?

^ permalink raw reply	[flat|nested] 17+ messages in thread
* RE: [PATCH v2:3/3]Export cpu topology by sysfs
@ 2005-12-31  8:43 Zhang, Yanmin
  2006-01-09  4:51 ` Nathan Lynch
  0 siblings, 1 reply; 17+ messages in thread
From: Zhang, Yanmin @ 2005-12-31  8:43 UTC (permalink / raw)
  To: Nathan Lynch
  Cc: Yanmin Zhang, greg, linux-kernel, discuss, linux-ia64,
	Siddha, Suresh B, Shah, Rajesh, Pallipadi, Venkatesh

>>-----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.


^ permalink raw reply	[flat|nested] 17+ messages in thread
* RE: [PATCH v2:3/3]Export cpu topology by sysfs
@ 2005-12-28 13:46 Zhang, Yanmin
  2005-12-28 19:14 ` Nathan Lynch
  0 siblings, 1 reply; 17+ messages in thread
From: Zhang, Yanmin @ 2005-12-28 13:46 UTC (permalink / raw)
  To: Nathan Lynch, Yanmin Zhang
  Cc: greg, linux-kernel, discuss, linux-ia64, Siddha, Suresh B,
	Shah, Rajesh, Pallipadi, Venkatesh

>>-----Original Message-----
>>From: Nathan Lynch [mailto:ntl@pobox.com]
>>Sent: 2005年12月28日 5:20
>>To: Yanmin Zhang
>>Cc: greg@kroah.com; linux-kernel@vger.kernel.org; discuss@x86-64.org; linux-ia64@vger.kernel.org; Siddha, Suresh B; Shah, Rajesh;
>>Pallipadi, Venkatesh; Zhang, Yanmin
>>Subject: Re: [PATCH v2:3/3]Export cpu topology by sysfs
>>
>>Yanmin Zhang wrote:
>>>
>>> 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_id:
>>> represent the cpu thread id  to cpu X;
>>
>>So what is the format of the *_id attributes?  Looks like this is
>>determined by the architecture, which is okay, but it doesn't seem
>>explicit.
The type of *_id is int or u8 (like on x86_64). It's better to convert the value to int.

>>
>>What about sane default values for the *_id attributes?
It depends on the specific architectures. On i386/x86_64, the default vaules of *_id are 0xffu. On ia64, the default value of physical_package_id is -1.

  For example,
>>say I have a uniprocessor PC without HT or multicore -- should all of
>>these attributes have zero values, or some kind of "special" value to
>>mean "not applicable"?
This feature is disabled when CONFIG_SMP=n. I can't make decision that all arch should use the same default values, so let architectures to decide. Is it ok?

>>
>>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.

>>
>>> 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 physical package;
>>>
>>> If one architecture wants to support this feature, it just needs to
>>> implement 5 defines, typically in file include/asm-XXX/topology.h.
>>> The 5 defines are:
>>> #define topology_physical_package_id(cpu)
>>> #define topology_core_id(cpu)
>>> #define topology_thread_id(cpu)
>>> #define topology_thread_siblings(cpu)
>>> #define topology_core_siblings(cpu)
>>>
>>> The type of siblings is cpumask_t.
>>>
>>> If an attribute isn't defined on an architecture, it won't be
>>> exported.
>>
>>Okay, but which combinations of attributes are valid?  E.g. I would
>>think that it's fine for an architecture to define topology_thread_id
>>and topology_thread_siblings without any of the others, correct?
I think topology_physical_package_id/topology_core_id/topology_core_siblings are also needed. For example, admin might want to bind some processes to specific physical cpu, or core.

>>
>>Also I'd rather the architectures have the ability to define these as
>>functions instead of macros.
It's easy. The architectures just need the defines to point to functions in their specific header files, typically in file include/asm-XXX/topology.h.

>>
>><snip>
>>
>>> +/* 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;
>>> +}
>>
>>Can't sysfs_create_group fail?
It might fail, but it doesn't matter. Later when topology_remove_dev is called, sysfs_remove_group will do nothing because the subdir is not created.


>>
>>> +
>>> +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;
>>> +}
>>
>>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.
And when a cpu is offline, current kernel deletes it from the thread_siblings and core_siblings of other cpu.

>>
>>In general, I'm still a little uneasy with exporting the cpu topology
>>in this way.  I can see how the information would be useful, and right
>>now, Linux does not do a great job of exposing to userspace these
>>relationships between cpus.  So I see the need.
I also think there are requirements. Many high-reliable applications, such like in telecom, need bind processes on specific cpus. The topology info is important for admin to do so.

  But the things about
>>this approach which I don't like are:
>>
>>- Attributes which are not applicable to the running system will be
>>  exported anyway.  Discovery at runtime would be less confusing, I
>>  think.
>>
>>- This locks us into exporting a three-level topology (thread, core,
>>  package), with hard-coded names, when it seems probable that there
>>  will be systems with more levels than that in the future.
It's easy to add more levels based on my implementations. 
Hard-coded names might be a problem. Is there any special requirement to change the names arch-specifically? If some architectures really need their specific names, I will change the names from hard-coded to arch-defined.

>>
>>Have you considered basing the exported topology on sched domains?
No. Sched domains consist of far more info. Zou Nanhai wrote a patch to export sched domain info by procfs. I think it's better if we could export sched domain by sysfs.

Thanks,
Yanmin

^ permalink raw reply	[flat|nested] 17+ messages in thread
* RE: [PATCH v2:3/3]Export cpu topology by sysfs
@ 2005-12-27 10:22 Zhang, Yanmin
  2005-12-27 10:41 ` Yanmin Zhang
  0 siblings, 1 reply; 17+ messages in thread
From: Zhang, Yanmin @ 2005-12-27 10:22 UTC (permalink / raw)
  To: Greg KH
  Cc: Yanmin Zhang, linux-kernel, discuss, linux-ia64, Siddha, Suresh B,
	Shah, Rajesh, Pallipadi, Venkatesh

>>-----Original Message-----
>>From: Greg KH [mailto:greg@kroah.com]
>>Sent: 2005年12月27日 13:35
>>To: Zhang, Yanmin
>>Cc: Yanmin Zhang; 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
>>
>>> Sorry. My previous explanation is confusing. It's a link warning on
>>> ia64. On ia64, the kernel vmlinux doesn't include section .exit.text,
>>> so ld will report a link warning when a function is in section
>>> .exit.text and another function (not in .exit.text) calls the first
>>> one. When CONFIG_HOTPLUG_CPU=n, function topology_remove_dev is in
>>> section .exit.text, but its caller topology_remove_dev is not in
>>> .exit.text.
>>>
>>> i386 and x86_64 kernel vmlinux does include .exit.text and discard it
>>> only when running, so there is no such warning on i386/x86_64.
>>>
>>> There is no other better approach to get rid of the warning unless we
>>> change arch/ia64/kernel/vmlinux.lds.S to keep all .exit.text in kernel
>>> image.
>>
>>Or just move that function to not be __exit, as you are calling it from
>>an __init function.  That would be the best solution.

I will change topology_remove_dev's attribute from __cpuexit to __cpuinit, and the problem will be resolved thoroughly.

Thanks.

^ permalink raw reply	[flat|nested] 17+ messages in thread
* RE: [PATCH v2:3/3]Export cpu topology by sysfs
@ 2005-12-23  4:03 Zhang, Yanmin
  2005-12-23 19:16 ` Greg KH
  0 siblings, 1 reply; 17+ messages in thread
From: Zhang, Yanmin @ 2005-12-23  4:03 UTC (permalink / raw)
  To: Greg KH, Yanmin Zhang
  Cc: linux-kernel, discuss, linux-ia64, Siddha, Suresh B, Shah, Rajesh,
	Pallipadi, Venkatesh

>>-----Original Message-----
>>From: Greg KH [mailto:greg@kroah.com]
>>Sent: 2005年12月23日 8:17
>>To: Yanmin Zhang
>>Cc: linux-kernel@vger.kernel.org; discuss@x86-64.org; linux-ia64@vger.kernel.org; Zhang, Yanmin; Siddha, Suresh B; Shah, Rajesh;
>>Pallipadi, Venkatesh
>>Subject: Re: [PATCH v2:3/3]Export cpu topology by sysfs
>>
>>On Tue, Dec 20, 2005 at 06:09:29PM -0800, Yanmin Zhang wrote:
>>> --- 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	2005-12-20 00:33:49.000000000 +0800
>>> @@ -0,0 +1,201 @@
>>> +/*
>>> + * driver/base/topology.c - Populate driverfs with cpu topology information
>>
>>driverfs?  Where did you cut/paste this code from?
>>
>>And why does it have to be in driver/base?
I copied it from driver/base/node.c which implements node exportation by sysfs. I will change it. 
I couldn't find a better place than driver/base.


>>
>>> +struct _topology_attr {
>>> +	struct attribute attr;
>>> +	ssize_t (*show)(int cpu, char *);
>>> +	ssize_t (*store)(const char *, size_t count);
>>> +};
>>
>>Don't put a '_' at the beginning of a structure please.
I will change it.

>>
>>> +static ssize_t topology_show(struct kobject * kobj, struct attribute * attr, char * buf)
>>> +{
>>> +	unsigned int cpu;
>>> +        struct _topology_attr *fattr = to_attr(attr);
>>> +        ssize_t ret;
>>> +
>>> +	cpu = container_of(kobj->parent, struct sys_device, kobj)->id;
>>> +        ret = fattr->show ? fattr->show(cpu, buf): 0;
>>> +        return ret;
>>> +}
>>
>>Mix of spaces and tabs :(
It's a stupid problem. I will fix it.


>>
>>> +static ssize_t topology_store(struct kobject * kobj, struct attribute * attr,
>>> +		     const char * buf, size_t count)
>>> +{
>>> +	return 0;
>>> +}
>>
>>Why is this function even needed?
It's not needed and will be deleted.


>>
>>> +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]);
>>> +}
>>
>>Can't you just use an attribute group and attach it to the cpu kobject?
>>That would save an array of kobjects I think.
As you know, current i386/x86_64 arch also export cache info under /sys/device/system/cpu/cpuX/cache. Is it clearer to export topology under a new directory than under cpu directly?


>>
>>> +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
>>
>>Why ifdef?  Isn't it safe to just always have this in?
If no ifdef here, gcc reported a compiling warning when I compiled it on IA64 with CONFIG_HOTPLUG_CPU=n.


>>
>>thanks,
>>
>>greg k-h
Thank you for the good comments. I will work out a new patch.


^ permalink raw reply	[flat|nested] 17+ messages in thread
* [PATCH v2:3/3]Export cpu topology by sysfs
@ 2005-12-21  2:09 Yanmin Zhang
  2005-12-23  0:16 ` Greg KH
  0 siblings, 1 reply; 17+ messages in thread
From: Yanmin Zhang @ 2005-12-21  2:09 UTC (permalink / raw)
  To: linux-kernel, discuss, linux-ia64
  Cc: yanmin.zhang, suresh.b.siddha, rajesh.shah, venkatesh.pallipadi

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

The third 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_id:
represent the cpu thread 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 physical package;

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

The type of siblings is cpumask_t.

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

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

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

#diffstat export_topology_2.6.15-rc5_v4.patch
 Documentation/cputopology.txt |   31 ++++++
 arch/ia64/kernel/topology.c   |    2
 drivers/base/Makefile         |    1
 drivers/base/topology.c       |  201 ++++++++++++++++++++++++++++++++++++++++++
 include/asm-i386/topology.h   |    8 +
 include/asm-ia64/topology.h   |    8 +
 include/asm-x86_64/topology.h |    8 +
 7 files changed, 258 insertions, 1 deletion

---

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	2005-12-14 21:20:54.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	2005-12-20 17:55:26.000000000 +0800
@@ -0,0 +1,31 @@
+
+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_id:
+represent the cpu thread 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 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 5 defines, typically in file include/asm-XXX/topology.h.
+The 5 defines are:
+#define topology_physical_package_id(cpu)
+#define topology_core_id(cpu)
+#define topology_thread_id(cpu)
+#define topology_thread_siblings(cpu)
+#define topology_core_siblings(cpu)
+
+The type of siblings is cpumask_t.
+
+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	2005-12-14 20:47:00.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	2005-12-20 00:33:49.000000000 +0800
@@ -0,0 +1,201 @@
+/*
+ * driver/base/topology.c - Populate driverfs with cpu topology information
+ *
+ * Written by: Zhang Yanmin, Intel Corporation
+ *
+ * Copyright (C) 2005, 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>
+
+struct _topology_attr {
+	struct attribute attr;
+	ssize_t (*show)(int cpu, char *);
+	ssize_t (*store)(const char *, size_t count);
+};
+
+#define to_attr(a) container_of(a, struct _topology_attr, attr)
+
+/* pointer to kobject for cpuX/topology */
+static struct kobject cpu_topology_kobject[NR_CPUS];
+
+#define define_one_ro(_name) 		\
+static struct _topology_attr _name =	\
+	__ATTR(_name, 0444, show_##_name, NULL)
+
+#define define_id_show_func(name)				\
+static ssize_t show_##name(int cpu, char *buf)			\
+{								\
+	return sprintf(buf, "%d\n", topology_##name(cpu));	\
+}
+
+#define define_siblings_show_func(name)					\
+static ssize_t show_##name(int cpu, char *buf)				\
+{									\
+	ssize_t len = -1;						\
+	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	&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		&core_id.attr,
+#else
+#define ref_core_id_attr
+#endif
+
+#ifdef topology_thread_id
+define_id_show_func(thread_id);
+define_one_ro(thread_id);
+#define ref_thread_id_attr		&thread_id.attr,
+#else
+#define ref_thread_id_attr
+#endif
+
+#ifdef topology_thread_siblings
+define_siblings_show_func(thread_siblings);
+define_one_ro(thread_siblings);
+#define ref_thread_siblings_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		&core_siblings.attr,
+#else
+#define ref_core_siblings_attr
+#endif
+
+static struct attribute * topology_default_attrs[] = {
+	ref_physical_package_id_attr
+	ref_core_id_attr
+	ref_thread_id_attr
+	ref_thread_siblings_attr
+	ref_core_siblings_attr
+	NULL
+};
+
+static ssize_t topology_show(struct kobject * kobj, struct attribute * attr, char * buf)
+{
+	unsigned int cpu;
+        struct _topology_attr *fattr = to_attr(attr);
+        ssize_t ret;
+
+	cpu = container_of(kobj->parent, struct sys_device, kobj)->id;
+        ret = fattr->show ? fattr->show(cpu, buf): 0;
+        return ret;
+}
+
+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;
+
+	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	2005-12-15 19:15:47.000000000 +0800
@@ -27,6 +27,14 @@
 #ifndef _ASM_I386_TOPOLOGY_H
 #define _ASM_I386_TOPOLOGY_H
 
+#ifdef CONFIG_SMP
+#define topology_physical_package_id(cpu)	phys_proc_id[cpu]
+#define topology_core_id(cpu)			cpu_core_id[cpu]
+#define topology_thread_id(cpu)			cpu_thread_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	2005-12-14 23:21:58.000000000 +0800
@@ -100,6 +100,14 @@ 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_thread_id(cpu)		cpu_data(cpu)->thread_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	2005-12-15 19:16:05.000000000 +0800
@@ -58,6 +58,14 @@ extern int __node_distance(int, int);
 
 #endif
 
+#ifdef CONFIG_SMP
+#define topology_physical_package_id(cpu)	phys_proc_id[cpu]
+#define topology_core_id(cpu)			cpu_core_id[cpu]
+#define topology_thread_id(cpu)			cpu_thread_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

^ permalink raw reply	[flat|nested] 17+ messages in thread
end of thread, other threads:[~2006-01-09 17:41 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-12-26  7:38 [PATCH v2:3/3]Export cpu topology by sysfs Zhang, Yanmin
2005-12-26  8:28 ` Yanmin Zhang
2005-12-27  5:36   ` Greg KH
2005-12-27  5:35 ` Greg KH
  -- strict thread matches above, loose matches on Subject: below --
2006-01-09  6:35 Zhang, Yanmin
2006-01-09 17:41 ` Russ Anderson
2005-12-31  8:43 Zhang, Yanmin
2006-01-09  4:51 ` Nathan Lynch
2005-12-28 13:46 Zhang, Yanmin
2005-12-28 19:14 ` Nathan Lynch
2005-12-27 10:22 Zhang, Yanmin
2005-12-27 10:41 ` Yanmin Zhang
2005-12-27 21:19   ` Nathan Lynch
2005-12-23  4:03 Zhang, Yanmin
2005-12-23 19:16 ` Greg KH
2005-12-21  2:09 Yanmin Zhang
2005-12-23  0:16 ` Greg KH

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox