Create 'slot' sysfs attribute in /sys/devices/system/cpu/cpuN/topology/
Subsystems (2)
RFC PATCH 1/1 Create 'slot' sysfs attribute in /sys/devices/system/cpu/cpuN/topology/
Hi Len, Tony,
A while back, I submitted a patch to expose the value of the _SUN method for CPUs. On ia64, the "physical id" field in /proc/cpuinfo isn't sufficient for some legacy HP ia64 platforms, as they do not have a modern SAL or PAL.
http://lkml.org/lkml/2007/10/30/529
That patch wasn't so great because as Len pointed out, /proc/acpi is a deprecated interface.
Here is another try that attempts to present a generic "slot" attribute in sysfs that leaves the various architectures free to implement however they like.
Yes, I am aware of the physical_package_id attribute. However, that doesn't necessarily map to a slot on the system board. Additionally, it's broken on the same ia64 systems where /proc/cpuinfo is broken, because it's trying to read the same field. :-/
On ia64, I have a somewhat ugly implementation, where the ACPI processor driver is filling in the answer. I chose this approach because I couldn't figure out how to do it in the base CPU driver, since calling the _SUN method requires the ACPI interpreter to be initialized, and it didn't look like that was the case when we did things like identify_siblings(), for instance.
Finally, there is something that looks odd in struct cpuinfo_ia64, where there are now two #ifdef CONFIG_SMP statements. I did this because adding the new 'slot' member created a 4-byte hole in the struct. It felt a little cleaner to me to move the 'int cpu;' member upwards to fill in the hole, and then my 'slot' could take the spot of 'cpu'.
I'd like to get feedback on both:
- the concept in general of creating a new sysfs attribute
- the ia64 implementation, specifically
Thanks.
/ac
From: Alex Chiang <[email protected]>
Physical slot information for CPUs is important for many reasons, including managability. For instance, if a platform supports physical CPU hotplug, it would be nice for the user to know which physical CPU module to actually unplug after offlining it.
The "physical id" field in /proc/cpuinfo is broken/missing on older ia64 platforms that do not have modern firmware.
/sys/devices/system/cpu/cpuN/topology/physical_package_id exists, but is broken on those same platforms because it's simply reading the same field that would have been presented in /proc/cpuinfo.
Create a 'slot' attribute and define an interface that may be populated via alternate sources, without breaking existing functionality.
Implement this interface on ia64 by invoking the _SUN method on a per cpu basis.
Leave other archs alone for now.
--- arch/ia64/kernel/setup.c | 1 + arch/ia64/kernel/topology.c | 6 ++++++ drivers/acpi/processor_core.c | 5 +++++ drivers/base/topology.c | 9 +++++++++ include/asm-ia64/cpu.h | 1 + include/asm-ia64/processor.h | 7 ++++++- include/asm-ia64/topology.h | 1 + include/asm-x86/cpu.h | 1 + 8 files changed, 30 insertions(+), 1 deletions(-)
arch/ia64/kernel/setup.c +1 -0
@@ -735,6 +735,7 @@ identify_cpu (struct cpuinfo_ia64 *c)*/ c->threads_per_core = c->cores_per_socket = c->num_log =1 ; c->socket_id =-1 ; + c->slot =-1 ;identify_siblings (c);
arch/ia64/kernel/topology.c +6 -0
@@ -27,6 +27,12 @@static struct ia64_cpu *sysfs_cpus; +void arch_set_topology_slot (int num,u32 slot) +{ +cpu_data (num)->slot = slot; +} +EXPORT_SYMBOL (arch_set_topology_slot); +int arch_register_cpu (int num) {#if defined (CONFIG_ACPI ) && defined (CONFIG_HOTPLUG_CPU )
drivers/acpi/processor_core.c +5 -0
@@ -590,6 +590,11 @@ static int acpi_processor_get_info(struct acpi_processor *pr, unsigned has_uid)ACPI_DEBUG_PRINT ((ACPI_DB_INFO ,"Processor [%d:%d]\n" , pr->id, pr->acpi_id)); + status =acpi_evaluate_object (pr->handle,"_SUN" ,NULL , &buffer); +if (ACPI_FAILURE (status)) + object.integer.value =-1 ; +arch_set_topology_slot (pr->id, object.integer.value); +if (!object.processor.pblk_address)ACPI_DEBUG_PRINT ((ACPI_DB_INFO ,"No PBLK (NULL address)\n" ));else if (object.processor.pblk_length !=6 )
drivers/base/topology.c +9 -0
@@ -57,6 +57,14 @@ define_one_ro(physical_package_id);#define ref_physical_package_id_attr#endif +#ifdef topology_slot +define_id_show_func (slot); +define_one_ro (slot); +#define ref_slot_attr &attr_slot.attr, +#else +#define ref_slot_attr +#endif +#ifdef topology_core_iddefine_id_show_func (core_id);define_one_ro (core_id);
@@ -83,6 +91,7 @@ define_one_ro(core_siblings);static struct attribute *default_attrs[] = { ref_physical_package_id_attr + ref_slot_attr ref_core_id_attr ref_thread_siblings_attr ref_core_siblings_attr
include/asm-ia64/cpu.h +1 -0
@@ -14,6 +14,7 @@ DECLARE_PER_CPU(struct ia64_cpu, cpu_devices);DECLARE_PER_CPU (int, cpu_state); +extern void arch_set_topology_slot (int num,u32 slot);extern int arch_register_cpu (int num);#ifdef CONFIG_HOTPLUG_CPU extern void arch_unregister_cpu (int );
include/asm-ia64/processor.h +6 -1
@@ -125,6 +125,10 @@ struct ia64_psr {*/ struct cpuinfo_ia64 {__u32 softirq_pending; + +#ifdef CONFIG_SMP +int cpu; +#endif __u64 itm_delta;/* # of clock cycles between clock ticks */ __u64 itm_next;/* interval timer mask value to use for next clock tick */ __u64 nsec_per_cyc;/* (1000000000<<IA64_NSEC_PER_CYC_SHIFT)/itc_freq */
@@ -140,7 +144,6 @@ struct cpuinfo_ia64 {#ifdef CONFIG_SMP __u64 loops_per_jiffy; -int cpu;__u32 socket_id;/* physical processor socket id */ __u16 core_id;/* core id */ __u16 thread_id;/* thread id */
@@ -150,6 +153,8 @@ struct cpuinfo_ia64 {__u8 threads_per_core;/* Threads per core */ #endif +__u32 slot;/* physical slot; can differ from socket_id */ +/* CPUID-derived information: */ __u64 ppn;__u64 features;
include/asm-ia64/topology.h +1 -0
@@ -110,6 +110,7 @@ void build_cpu_to_node_map(void);#ifdef CONFIG_SMP #define topology_physical_package_id (cpu) (cpu_data(cpu)->socket_id) +#define topology_slot (cpu) (cpu_data(cpu)->slot)#define topology_core_id (cpu) (cpu_data(cpu)->core_id)#define topology_core_siblings (cpu) (cpu_core_map[cpu])#define topology_thread_siblings (cpu) (per_cpu(cpu_sibling_map, cpu))
include/asm-x86/cpu.h +1 -0
@@ -11,6 +11,7 @@ struct x86_cpu {struct cpu cpu; }; +extern void arch_set_topology_slot (int num,u32 slot);#ifdef CONFIG_HOTPLUG_CPU extern int arch_register_cpu (int num);extern void arch_unregister_cpu (int );
Signature · 1 line
1.5.3.1.g1e61
RE: [PATCH, RFC] Create 'slot' sysfs attribute in/sys/devices/system/cpu/cpuN/topology/
Quoted 4 lines
A while back, I submitted a patch to expose the value of the
_SUNmethod for CPUs. On ia64, the "physical id" field in/proc/cpuinfoisn't sufficient for some legacy HP ia64 platforms, as they do not have a modern SAL or PAL.
Do these legacy systems with ancient SAL/PAL actually support processor hot plug?
Perhaps this next rant is beyond the scope of Linux architecture and more on overall systems useabilty. It is just asking for trouble to print a message on the console telling the user to go pull out the board in "slot 2". Do the slot numbers count from "0" or from "1"? Are they numbered left-to-right or right-to-left? What if the rack is non-standard so the system was rotated 90 degrees? Now are they numbered top-to-bottom or bottom-to-top.
Best idea is to have a (blinking) (red) light on the board and instruct the operator to pull out the board with the fail-light (but even then a red-green colour blind operator will still mess it up for you and pull the wrong board :-)
-Tony
Re: [PATCH, RFC] Create 'slot' sysfs attribute in/sys/devices/system/cpu/cpuN/topology/
Quoted 7 lines · Tony
Perhaps this next rant is beyond the scope of Linux architecture and more on overall systems useabilty. It is just asking for trouble to print a message on the console telling the user to go pull out the board in "slot 2". Do the slot numbers count from "0" or from "1"? Are they numbered left-to-right or right-to-left? What if the rack is non-standard so the system was rotated 90 degrees? Now are they numbered top-to-bottom or bottom-to-top.
Some systems (particularly those designed for hotplug) have the numbers silk-screened onto the case. It's like PCI slots ... my beige-box homebrew PCs don't have slot numbers written on them, but the machines I have from HP are very clearly labelled with slot numbers beside each PCI slot.
Admittedly, the flashing red LED is a better UI, but it's not necessarily as confusing as you're painting it.
Signature · 4 lines
Intel are signing my paycheques ... these opinions are still mine "Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step."
Re: [PATCH, RFC] Create 'slot' sysfs attribute in/sys/devices/system/cpu/cpuN/topology/
In reply to Luck, Tony* Luck, Tony <[email protected]>:
Quoted 7 lines
Quoted 4 lines
A while back, I submitted a patch to expose the value of the
_SUNmethod for CPUs. On ia64, the "physical id" field in/proc/cpuinfoisn't sufficient for some legacy HP ia64 platforms, as they do not have a modern SAL or PAL.Do these legacy systems with ancient SAL/PAL actually support processor hot plug?
No, they do not; that was simply the most convenient / obvious example I could think of for someone reading changelogs a few years later and wondering why we created this sysfs attribute.
A real world example would be HP managability software keeping track of CMC/CPE data for CPUs over a long period of time for the purpose of doing long-term failure analysis.
This software can already grab a bunch of info from IPMI, such as CPU serial number, date code, physical location, but cannot correlate it to the kernel's idea of logical CPU number and physloc.
Having the kernel expose the physical slot ties it all together.
This example is something any vendor selling higher level managability software might want to do, and specifically HP wants to do it to take care of our customers who have large deployments of these legacy systems.
Hopefully that is enough justification to warrant at least looking at the patch and telling me why it sucks. ;)
Quoted 7 lines
Perhaps this next rant is beyond the scope of Linux architecture and more on overall systems useabilty. It is just asking for trouble to print a message on the console telling the user to go pull out the board in "slot 2". Do the slot numbers count from "0" or from "1"? Are they numbered left-to-right or right-to-left? What if the rack is non-standard so the system was rotated 90 degrees? Now are they numbered top-to-bottom or bottom-to-top.
Well, at least for HP ia64 machines, we have lots of silk-screening on the backplane, as well as pretty diagrams on chassis covers, and we took care in making sure our what our firmware matches up with those pictures.
But in general, I like the idea of das blinkenlights too. :)
Quoted 4 lines
Best idea is to have a (blinking) (red) light on the board and instruct the operator to pull out the board with the fail-light (but even then a red-green colour blind operator will still mess it up for you and pull the wrong board :-)
Heh, then just make the failure light either blink (for failure) or not blink (default). Then as long as the operator can detect lumens, he/she would just be able to pull the blinky board. :)
Thanks.
/ac
RE: [PATCH, RFC] Create 'slot' sysfs attribute in/sys/devices/system/cpu/cpuN/topology/
In reply to Alex ChiangQuoted 1 line
include/asm-x86/cpu.h| 1 +
I assume this slipped in accidentally (though it would eventually be needed for an x86 version of this patch).
drivers/base/topology.c define_one_ro(physical_package_id); · line 66
Quoted 7 lines
drivers/base/topology.c +7
-0+#ifdef topology_slot +define_id_show_func (slot); +define_one_ro (slot); +#define ref_slot_attr &attr_slot.attr, +#else +#define ref_slot_attr +#endif
This is a bit ugly ... I don't see how to make it better (but I'm sure that akpm will say "You could just ...", and then I'll wonder why I didn't think of that).
Quoted 1 line
+EXPORT_SYMBOL(arch_set_topology_slot);
As the author you get to choose the license ... but there is strong movement to make new exported symbols GPL only.
When I apply this patch and run it on my tiger, I see a slew of
ACPI: Invalid PBLK length [0]
messages on my console during boot and the see
$ cat /sys/devices/system/cpu/cpu0/topology/slot -1
So I guess that the _SUN method isn't there on my machine.
-Tony
Re: [PATCH, RFC] Create 'slot' sysfs attribute in/sys/devices/system/cpu/cpuN/topology/
Hi Tony,
* Luck, Tony <[email protected]>:
Quoted 4 lines
Quoted 1 line
include/asm-x86/cpu.h| 1 +I assume this slipped in accidentally (though it would eventually be needed for an x86 version of this patch).
Well, it wasn't an accident, but I think I did screw up my attempt at making this patch a nop on x86. The patch below should have a better attempt at that.
Quoted 11 lines
Review
Quoted 7 lines
(quoted diff) +7
-0+#ifdef topology_slot +define_id_show_func(slot); +define_one_ro(slot); +#define ref_slot_attr &attr_slot.attr, +#else +#define ref_slot_attr +#endifThis is a bit ugly ... I don't see how to make it better (but I'm sure that akpm will say "You could just ...", and then I'll wonder why I didn't think of that).
Hrm, I was just following a precedent here. I guess I'll wait for enlightenment from akpm too. ;)
Quoted 4 lines
Quoted 1 line
+EXPORT_SYMBOL(arch_set_topology_slot);
As the author you get to choose the license ... but there is strong movement to make new exported symbols GPL only.
Changed.
Quoted 6 lines
When I apply this patch and run it on my tiger, I see a slew of
ACPI: Invalid PBLK length [0]
messages on my console during boot and the see
I don't think this should be coming from my patch. However, I did update so that we're checking for a _SUN method underneath the CPU before blinding stabbing around and trying to call it.
Quoted 4 lines
$ cat /sys/devices/system/cpu/cpu0/topology/slot -1So I guess that the
_SUNmethod isn't there on my machine.
If you don't have _SUN on your machine, this version of the patch should prevent those annoying messages from appearing.
However, you'll still see the sysfs attribute with a value of -1. Personally, I don't think this is so bad, since on my legacy HP machines, I get a -1 when I cat the 'physical_package_id' attribute...
I'll defer to your judgement on this though.
/ac
From: Alex Chiang <[email protected]>
Subject: [PATCH] Create 'slot' sysfs attribute in /sys/devices/system/cpu/cpuN/topology/
/sys/devices/system/cpu/cpuN/topology/physical_package_id exists, but may contain incorrect information, especially on older ia64 platforms that do not have modern firmware.
Create a 'slot' attribute and define an interface that may be populated via alternate sources, without breaking existing functionality.
Implement this interface on ia64 by invoking the _SUN method on a per cpu basis.
Leave other archs alone for now.
v1 -> v2:
- Check for _SUN object on CPU before blindly attempting to call it
--- arch/ia64/kernel/setup.c | 1 + arch/ia64/kernel/topology.c | 6 ++++++ drivers/acpi/processor_core.c | 11 +++++++++++ drivers/base/topology.c | 9 +++++++++ include/asm-ia64/cpu.h | 1 + include/asm-ia64/processor.h | 7 ++++++- include/asm-ia64/topology.h | 3 +++ include/asm-x86/topology.h | 2 ++ 8 files changed, 39 insertions(+), 1 deletions(-)
arch/ia64/kernel/setup.c +1 -0
@@ -735,6 +735,7 @@ identify_cpu (struct cpuinfo_ia64 *c)*/ c->threads_per_core = c->cores_per_socket = c->num_log =1 ; c->socket_id =-1 ; + c->slot =-1 ;identify_siblings (c);
arch/ia64/kernel/topology.c +6 -0
@@ -27,6 +27,12 @@static struct ia64_cpu *sysfs_cpus; +void ia64_set_topology_slot (int num,u32 slot) +{ +cpu_data (num)->slot = slot; +} +EXPORT_SYMBOL_GPL (ia64_set_topology_slot); +int arch_register_cpu (int num) {#if defined (CONFIG_ACPI ) && defined (CONFIG_HOTPLUG_CPU )
drivers/acpi/processor_core.c +11 -0
@@ -509,6 +509,7 @@ static int get_cpu_id(acpi_handle handle, u32 acpi_id)static int acpi_processor_get_info (struct acpi_processor *pr,unsigned has_uid) {acpi_status status =0 ; +acpi_handle handle;union acpi_object object = {0 };struct acpi_buffer buffer = {sizeof (union acpi_object ), &object };int cpu_index;
@@ -590,6 +591,16 @@ static int acpi_processor_get_info(struct acpi_processor *pr, unsigned has_uid)ACPI_DEBUG_PRINT ((ACPI_DB_INFO ,"Processor [%d:%d]\n" , pr->id, pr->acpi_id)); +if (ACPI_SUCCESS (acpi_get_handle (pr->handle,"_SUN" , &handle))) { + status =acpi_evaluate_object (pr->handle, +"_SUN" ,NULL , &buffer); +if (ACPI_FAILURE (status)) + object.integer.value =-1 ; + }else { + object.integer.value =-1 ; + } +arch_set_topology_slot (pr->id, object.integer.value); +if (!object.processor.pblk_address)ACPI_DEBUG_PRINT ((ACPI_DB_INFO ,"No PBLK (NULL address)\n" ));else if (object.processor.pblk_length !=6 )
drivers/base/topology.c +9 -0
@@ -57,6 +57,14 @@ define_one_ro(physical_package_id);#define ref_physical_package_id_attr#endif +#ifdef topology_slot +define_id_show_func (slot); +define_one_ro (slot); +#define ref_slot_attr &attr_slot.attr, +#else +#define ref_slot_attr +#endif +#ifdef topology_core_iddefine_id_show_func (core_id);define_one_ro (core_id);
@@ -83,6 +91,7 @@ define_one_ro(core_siblings);static struct attribute *default_attrs[] = { ref_physical_package_id_attr + ref_slot_attr ref_core_id_attr ref_thread_siblings_attr ref_core_siblings_attr
include/asm-ia64/cpu.h +1 -0
@@ -14,6 +14,7 @@ DECLARE_PER_CPU(struct ia64_cpu, cpu_devices);DECLARE_PER_CPU (int, cpu_state); +extern void arch_set_topology_slot (int num,u32 slot);extern int arch_register_cpu (int num);#ifdef CONFIG_HOTPLUG_CPU extern void arch_unregister_cpu (int );
include/asm-ia64/processor.h +6 -1
@@ -125,6 +125,10 @@ struct ia64_psr {*/ struct cpuinfo_ia64 {__u32 softirq_pending; + +#ifdef CONFIG_SMP +int cpu; +#endif __u64 itm_delta;/* # of clock cycles between clock ticks */ __u64 itm_next;/* interval timer mask value to use for next clock tick */ __u64 nsec_per_cyc;/* (1000000000<<IA64_NSEC_PER_CYC_SHIFT)/itc_freq */
@@ -140,7 +144,6 @@ struct cpuinfo_ia64 {#ifdef CONFIG_SMP __u64 loops_per_jiffy; -int cpu;__u32 socket_id;/* physical processor socket id */ __u16 core_id;/* core id */ __u16 thread_id;/* thread id */
@@ -150,6 +153,8 @@ struct cpuinfo_ia64 {__u8 threads_per_core;/* Threads per core */ #endif +__u32 slot;/* physical slot; can differ from socket_id */ +/* CPUID-derived information: */ __u64 ppn;__u64 features;
include/asm-ia64/topology.h +3 -0
@@ -110,12 +110,15 @@ void build_cpu_to_node_map(void);#ifdef CONFIG_SMP #define topology_physical_package_id (cpu) (cpu_data(cpu)->socket_id) +#define topology_slot (cpu) (cpu_data(cpu)->slot)#define topology_core_id (cpu) (cpu_data(cpu)->core_id)#define topology_core_siblings (cpu) (cpu_core_map[cpu])#define topology_thread_siblings (cpu) (per_cpu(cpu_sibling_map, cpu))#define smt_capable () (smp_num_siblings > 1)#endif +#define arch_set_topology_slot (num, slot) ia64_set_topology_slot(num,slot) +#include <asm-generic/topology.h> #endif /* _ASM_IA64_TOPOLOGY_H */
include/asm-x86/topology.h +2 -0
@@ -180,6 +180,8 @@ extern cpumask_t cpu_coregroup_map(int cpu);#define topology_thread_siblings (cpu) (per_cpu(cpu_sibling_map, cpu))#endif +#define arch_set_topology_slot (num, slot) +#ifdef CONFIG_SMP #define mc_capable () (boot_cpu_data.x86_max_cores > 1)#define smt_capable () (smp_num_siblings > 1)
Signature · 1 line
1.5.3.1.g1e61
RE: [PATCH, RFC] Create 'slot' sysfs attributein/sys/devices/system/cpu/cpuN/topology/
+ if (ACPI_SUCCESS(acpi_get_handle(pr->handle, "_SUN", &handle))) {
+ status = acpi_evaluate_object(pr->handle,
+ "_SUN", NULL, &buffer);
+ if (ACPI_FAILURE(status))
+ object.integer.value = -1;
+ } else {
+ object.integer.value = -1;
+ }
+ arch_set_topology_slot(pr->id, object.integer.value);
+
if (!object.processor.pblk_address)
ACPI_DEBUG_PRINT((ACPI_DB_INFO, "No PBLK (NULL address)\n"));
else if (object.processor.pblk_length != 6)
I'm still seeing these messages:
"ACPI: Invalid PBLK length [0]"
for every cpu. Presumably because "object" is a union and when you did the "object.integer.value = -1;" you scribbled on some bits that were needed in the subsequent code.
-Tony
Re: [PATCH, RFC] Create 'slot' sysfs attributein/sys/devices/system/cpu/cpuN/topology/
Hi Tony,
* Luck, Tony <[email protected]>:
Quoted 14 lines
(quoted diff) +10
-0+ if (ACPI_SUCCESS(acpi_get_handle(pr->handle, "_SUN", &handle))) { + status = acpi_evaluate_object(pr->handle, + "_SUN", NULL, &buffer); + if (ACPI_FAILURE(status)) + object.integer.value = -1; + } else { + object.integer.value = -1; + } + arch_set_topology_slot(pr->id, object.integer.value); + if (!object.processor.pblk_address) ACPI_DEBUG_PRINT((ACPI_DB_INFO, "No PBLK (NULL address)\n")); else if (object.processor.pblk_length != 6)
Quoted 7 lines
I'm still seeing these messages:
"ACPI: Invalid PBLK length [0]"
for every cpu. Presumably because "object" is a union and when you did the "object.integer.value = -1;" you scribbled on some bits that were needed in the subsequent code.
Yes, you're right. Sorry about the sloppy code.
Here is v3. I've verified that:
- it's benign on tiger (does not create errors/console noise). You'll still see the -1 if you cat /sys/devices/system/cpu/cpuN/topology/slot, but you would also get the -1 if you cat physical_package_id
- it does the right thing on platforms that do supply _SUN for processor objects. On my rx2600, physical_package_id is -1, but I get valid values for 'slot' now.
Thanks.
/ac
Subject: [PATCH] Create 'slot' sysfs attribute in /sys/devices/system/cpu/cpuN/topology/
From: Alex Chiang <[email protected]>
/sys/devices/system/cpu/cpuN/topology/physical_package_id exists, but may contain incorrect information, especially on older ia64 platforms that do not have modern firmware.
Create a 'slot' attribute and define an interface that may be populated via alternate sources, without breaking existing functionality.
Implement this interface on ia64 by invoking the _SUN method on a per cpu basis.
Leave other archs alone for now.
v2 -> v3:
- Stop stomping on existing PBLK in the event of _SUN failure
v1 -> v2:
- Check for _SUN object on CPU before blindly attempting to call it
--- arch/ia64/kernel/setup.c | 1 + arch/ia64/kernel/topology.c | 6 ++++++ drivers/acpi/processor_core.c | 8 ++++++++ drivers/base/topology.c | 9 +++++++++ include/asm-ia64/cpu.h | 1 + include/asm-ia64/processor.h | 7 ++++++- include/asm-ia64/topology.h | 3 +++ include/asm-x86/topology.h | 2 ++ 8 files changed, 36 insertions(+), 1 deletions(-)
arch/ia64/kernel/setup.c +1 -0
@@ -735,6 +735,7 @@ identify_cpu (struct cpuinfo_ia64 *c)*/ c->threads_per_core = c->cores_per_socket = c->num_log =1 ; c->socket_id =-1 ; + c->slot =-1 ;identify_siblings (c);
arch/ia64/kernel/topology.c +6 -0
@@ -27,6 +27,12 @@static struct ia64_cpu *sysfs_cpus; +void ia64_set_topology_slot (int num,u32 slot) +{ +cpu_data (num)->slot = slot; +} +EXPORT_SYMBOL_GPL (ia64_set_topology_slot); +int arch_register_cpu (int num) {#if defined (CONFIG_ACPI ) && defined (CONFIG_HOTPLUG_CPU )
drivers/acpi/processor_core.c +8 -0
@@ -612,6 +612,14 @@ static int acpi_processor_get_info(struct acpi_processor *pr, unsigned has_uid)request_region (pr->throttling.address,6 ,"ACPI CPU throttle" ); } +/* +* If ACPI describes a slot number for this CPU, let's fill it in +*/ + status =acpi_evaluate_object (pr->handle,"_SUN" ,NULL , &buffer); +if (ACPI_FAILURE (status)) + object.integer.value =-1 ; +arch_set_topology_slot (pr->id, object.integer.value); +return 0 ; }
drivers/base/topology.c +9 -0
@@ -57,6 +57,14 @@ define_one_ro(physical_package_id);#define ref_physical_package_id_attr#endif +#ifdef topology_slot +define_id_show_func (slot); +define_one_ro (slot); +#define ref_slot_attr &attr_slot.attr, +#else +#define ref_slot_attr +#endif +#ifdef topology_core_iddefine_id_show_func (core_id);define_one_ro (core_id);
@@ -83,6 +91,7 @@ define_one_ro(core_siblings);static struct attribute *default_attrs[] = { ref_physical_package_id_attr + ref_slot_attr ref_core_id_attr ref_thread_siblings_attr ref_core_siblings_attr
include/asm-ia64/cpu.h +1 -0
@@ -14,6 +14,7 @@ DECLARE_PER_CPU(struct ia64_cpu, cpu_devices);DECLARE_PER_CPU (int, cpu_state); +extern void arch_set_topology_slot (int num,u32 slot);extern int arch_register_cpu (int num);#ifdef CONFIG_HOTPLUG_CPU extern void arch_unregister_cpu (int );
include/asm-ia64/processor.h +6 -1
@@ -125,6 +125,10 @@ struct ia64_psr {*/ struct cpuinfo_ia64 {__u32 softirq_pending; + +#ifdef CONFIG_SMP +int cpu; +#endif __u64 itm_delta;/* # of clock cycles between clock ticks */ __u64 itm_next;/* interval timer mask value to use for next clock tick */ __u64 nsec_per_cyc;/* (1000000000<<IA64_NSEC_PER_CYC_SHIFT)/itc_freq */
@@ -140,7 +144,6 @@ struct cpuinfo_ia64 {#ifdef CONFIG_SMP __u64 loops_per_jiffy; -int cpu;__u32 socket_id;/* physical processor socket id */ __u16 core_id;/* core id */ __u16 thread_id;/* thread id */
@@ -150,6 +153,8 @@ struct cpuinfo_ia64 {__u8 threads_per_core;/* Threads per core */ #endif +__u32 slot;/* physical slot; can differ from socket_id */ +/* CPUID-derived information: */ __u64 ppn;__u64 features;
include/asm-ia64/topology.h +3 -0
@@ -110,12 +110,15 @@ void build_cpu_to_node_map(void);#ifdef CONFIG_SMP #define topology_physical_package_id (cpu) (cpu_data(cpu)->socket_id) +#define topology_slot (cpu) (cpu_data(cpu)->slot)#define topology_core_id (cpu) (cpu_data(cpu)->core_id)#define topology_core_siblings (cpu) (cpu_core_map[cpu])#define topology_thread_siblings (cpu) (per_cpu(cpu_sibling_map, cpu))#define smt_capable () (smp_num_siblings > 1)#endif +#define arch_set_topology_slot (num, slot) ia64_set_topology_slot(num,slot) +#include <asm-generic/topology.h> #endif /* _ASM_IA64_TOPOLOGY_H */
include/asm-x86/topology.h +2 -0
@@ -180,6 +180,8 @@ extern cpumask_t cpu_coregroup_map(int cpu);#define topology_thread_siblings (cpu) (per_cpu(cpu_sibling_map, cpu))#endif +#define arch_set_topology_slot (num, slot) +#ifdef CONFIG_SMP #define mc_capable () (boot_cpu_data.x86_max_cores > 1)#define smt_capable () (smp_num_siblings > 1)
Signature · 1 line
1.5.3.1.g1e61
Re: [PATCH, RFC] Create 'slot' sysfs attributein/sys/devices/system/cpu/cpuN/topology/
Hi Tony,
I noticed that this patch wasn't in the git tree you sent to Linus for 2.6.26. I don't remember seeing a NACK though -- is there something that I could rework to make it more acceptable?
Thanks.
/ac
* Alex Chiang <[email protected]>:
Quoted 238 lines
Hi Tony,
* Luck, Tony <[email protected]>:
Quoted 14 lines
(quoted diff) +10
-0+ if (ACPI_SUCCESS(acpi_get_handle(pr->handle, "_SUN", &handle))) { + status = acpi_evaluate_object(pr->handle, + "_SUN", NULL, &buffer); + if (ACPI_FAILURE(status)) + object.integer.value = -1; + } else { + object.integer.value = -1; + } + arch_set_topology_slot(pr->id, object.integer.value); + if (!object.processor.pblk_address) ACPI_DEBUG_PRINT((ACPI_DB_INFO, "No PBLK (NULL address)\n")); else if (object.processor.pblk_length != 6)Quoted 7 lines
I'm still seeing these messages:
"ACPI: Invalid PBLK length [0]"
for every cpu. Presumably because "object" is a union and when you did the "object.integer.value = -1;" you scribbled on some bits that were needed in the subsequent code.
Yes, you're right. Sorry about the sloppy code.
Here is v3. I've verified that:
- it's benign on tiger (does not create errors/console noise). You'll still see the -1 if you cat
/sys/devices/system/cpu/cpuN/topology/slot, but you would also get the -1 if you catphysical_package_id- it does the right thing on platforms that do supply
_SUNfor processor objects. On my rx2600,physical_package_idis -1, but I get valid values for 'slot' now.Thanks.
/ac
Subject: [PATCH] Create 'slot' sysfs attribute in
/sys/devices/system/cpu/cpuN/topology/
From: Alex Chiang <[email protected]>
/sys/devices/system/cpu/cpuN/topology/physical_package_idexists, but may contain incorrect information, especially on older ia64 platforms that do not have modern firmware.Create a 'slot' attribute and define an interface that may be populated via alternate sources, without breaking existing functionality.
Implement this interface on ia64 by invoking the
_SUNmethod on a per cpu basis.Leave other archs alone for now.
v2 -> v3:
- Stop stomping on existing PBLK in the event of_SUNfailurev1 -> v2:
- Check for_SUNobject on CPU before blindly attempting to call itSigned-off-by Alex Chiang <[email protected]>--- arch/ia64/kernel/setup.c | 1 + arch/ia64/kernel/topology.c | 6 ++++++ drivers/acpi/processor_core.c | 8 ++++++++ drivers/base/topology.c | 9 +++++++++ include/asm-ia64/cpu.h | 1 + include/asm-ia64/processor.h | 7 ++++++- include/asm-ia64/topology.h | 3 +++ include/asm-x86/topology.h | 2 ++ 8 files changed, 36 insertions(+), 1 deletions(-)arch/ia64/kernel/setup.c +1
-0@@ -735,6 +735,7 @@ identify_cpu (struct cpuinfo_ia64 *c)*/ c->threads_per_core = c->cores_per_socket = c->num_log =1 ; c->socket_id =-1 ; + c->slot =-1 ;identify_siblings (c);arch/ia64/kernel/topology.c +6
-0@@ -27,6 +27,12 @@static struct ia64_cpu *sysfs_cpus; +void ia64_set_topology_slot (int num,u32 slot) +{ +cpu_data (num)->slot = slot; +} +EXPORT_SYMBOL_GPL (ia64_set_topology_slot); +int arch_register_cpu (int num) {#if defined (CONFIG_ACPI ) && defined (CONFIG_HOTPLUG_CPU )drivers/acpi/processor_core.c +8
-0@@ -612,6 +612,14 @@ static int acpi_processor_get_info(struct acpi_processor *pr, unsigned has_uid)request_region (pr->throttling.address,6 ,"ACPI CPU throttle" ); } +/* +* If ACPI describes a slot number for this CPU, let's fill it in +*/ + status =acpi_evaluate_object (pr->handle,"_SUN" ,NULL , &buffer); +if (ACPI_FAILURE (status)) + object.integer.value =-1 ; +arch_set_topology_slot (pr->id, object.integer.value); +return 0 ; }drivers/base/topology.c +9
-0@@ -57,6 +57,14 @@ define_one_ro(physical_package_id);#define ref_physical_package_id_attr#endif +#ifdef topology_slot +define_id_show_func (slot); +define_one_ro (slot); +#define ref_slot_attr &attr_slot.attr, +#else +#define ref_slot_attr +#endif +#ifdef topology_core_iddefine_id_show_func (core_id);define_one_ro (core_id);@@ -83,6 +91,7 @@ define_one_ro(core_siblings);static struct attribute *default_attrs[] = { ref_physical_package_id_attr + ref_slot_attr ref_core_id_attr ref_thread_siblings_attr ref_core_siblings_attrinclude/asm-ia64/cpu.h +1
-0@@ -14,6 +14,7 @@ DECLARE_PER_CPU(struct ia64_cpu, cpu_devices);DECLARE_PER_CPU (int, cpu_state); +extern void arch_set_topology_slot (int num,u32 slot);extern int arch_register_cpu (int num);#ifdef CONFIG_HOTPLUG_CPU extern void arch_unregister_cpu (int );include/asm-ia64/processor.h +6
-1@@ -125,6 +125,10 @@ struct ia64_psr {*/ struct cpuinfo_ia64 {__u32 softirq_pending; + +#ifdef CONFIG_SMP +int cpu; +#endif __u64 itm_delta;/* # of clock cycles between clock ticks */ __u64 itm_next;/* interval timer mask value to use for next clock tick */ __u64 nsec_per_cyc;/* (1000000000<<IA64_NSEC_PER_CYC_SHIFT)/itc_freq */ @@ -140,7 +144,6 @@ struct cpuinfo_ia64 {#ifdef CONFIG_SMP __u64 loops_per_jiffy; -int cpu;__u32 socket_id;/* physical processor socket id */ __u16 core_id;/* core id */ __u16 thread_id;/* thread id */ @@ -150,6 +153,8 @@ struct cpuinfo_ia64 {__u8 threads_per_core;/* Threads per core */ #endif +__u32 slot;/* physical slot; can differ from socket_id */ +/* CPUID-derived information: */ __u64 ppn;__u64 features;include/asm-ia64/topology.h +3
-0@@ -110,12 +110,15 @@ void build_cpu_to_node_map(void);#ifdef CONFIG_SMP #define topology_physical_package_id (cpu) (cpu_data(cpu)->socket_id) +#define topology_slot (cpu) (cpu_data(cpu)->slot)#define topology_core_id (cpu) (cpu_data(cpu)->core_id)#define topology_core_siblings (cpu) (cpu_core_map[cpu])#define topology_thread_siblings (cpu) (per_cpu(cpu_sibling_map, cpu))#define smt_capable () (smp_num_siblings > 1)#endif +#define arch_set_topology_slot (num, slot) ia64_set_topology_slot(num,slot) +#include <asm-generic/topology.h> #endif /* _ASM_IA64_TOPOLOGY_H */ include/asm-x86/topology.h +2
-0@@ -180,6 +180,8 @@ extern cpumask_t cpu_coregroup_map(int cpu);#define topology_thread_siblings (cpu) (per_cpu(cpu_sibling_map, cpu))#endif +#define arch_set_topology_slot (num, slot) +#ifdef CONFIG_SMP #define mc_capable () (boot_cpu_data.x86_max_cores > 1)#define smt_capable () (smp_num_siblings > 1)Signature · 6 lines
1.5.3.1.g1e61 -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH, RFC] Create 'slot' sysfsattributein/sys/devices/system/cpu/cpuN/topology/
Quoted 3 lines
I noticed that this patch wasn't in the git tree you sent to Linus for 2.6.26. I don't remember seeing a NACK though -- is there something that I could rework to make it more acceptable?
I'm mostly ok with this version of the patch. I didn't see any comments from the linux-kernel crowd on the whether they are fond of the new API ("slot" file in cpu/cpuN/topology/) and hate to make the presumption that because they are silent that they agree.
The "mostly ok" part would transform to "fully ok" if there were a way to make sure the "slot" files only appeared on systems where they are meaningful (i.e. have a value other then -1 in them). If there is an easy way to make this happen, then it would make me happier (less clutter in /sys) and perhaps others too (since this API is only useful on large systems where "slot" is meaningful, and there is generally some bias from the community at large about adding interfaces that aren't needed for normal desktop/laptop systems).
-Tony
Patch follows to save people digging in the archives to see what we are talking about.
Quoted 195 lines
Subject: [PATCH] Create 'slot' sysfs attribute in
/sys/devices/system/cpu/cpuN/topology/
From: Alex Chiang <[email protected]>
/sys/devices/system/cpu/cpuN/topology/physical_package_idexists, but may contain incorrect information, especially on older ia64 platforms that do not have modern firmware.Create a 'slot' attribute and define an interface that may be populated via alternate sources, without breaking existing functionality.
Implement this interface on ia64 by invoking the
_SUNmethod on a per cpu basis.Leave other archs alone for now.
v2 -> v3:
- Stop stomping on existing PBLK in the event of_SUNfailurev1 -> v2:
- Check for_SUNobject on CPU before blindly attempting to call itSigned-off-by Alex Chiang <[email protected]>--- arch/ia64/kernel/setup.c | 1 + arch/ia64/kernel/topology.c | 6 ++++++ drivers/acpi/processor_core.c | 8 ++++++++ drivers/base/topology.c | 9 +++++++++ include/asm-ia64/cpu.h | 1 + include/asm-ia64/processor.h | 7 ++++++- include/asm-ia64/topology.h | 3 +++ include/asm-x86/topology.h | 2 ++ 8 files changed, 36 insertions(+), 1 deletions(-)arch/ia64/kernel/setup.c +1
-0@@ -735,6 +735,7 @@ identify_cpu (struct cpuinfo_ia64 *c)*/ c->threads_per_core = c->cores_per_socket = c->num_log =1 ; c->socket_id =-1 ; + c->slot =-1 ;identify_siblings (c);arch/ia64/kernel/topology.c +6
-0@@ -27,6 +27,12 @@static struct ia64_cpu *sysfs_cpus; +void ia64_set_topology_slot (int num,u32 slot) +{ +cpu_data (num)->slot = slot; +} +EXPORT_SYMBOL_GPL (ia64_set_topology_slot); +int arch_register_cpu (int num) {#if defined (CONFIG_ACPI ) && defined (CONFIG_HOTPLUG_CPU )drivers/acpi/processor_core.c +8
-0@@ -612,6 +612,14 @@ static int acpi_processor_get_info(struct acpi_processor *pr, unsigned has_uid)request_region (pr->throttling.address,6 ,"ACPI CPU throttle" ); } +/* +* If ACPI describes a slot number for this CPU, let's fill it in +*/ + status =acpi_evaluate_object (pr->handle,"_SUN" ,NULL , &buffer); +if (ACPI_FAILURE (status)) + object.integer.value =-1 ; +arch_set_topology_slot (pr->id, object.integer.value); +return 0 ; }drivers/base/topology.c +9
-0@@ -57,6 +57,14 @@ define_one_ro(physical_package_id);#define ref_physical_package_id_attr#endif +#ifdef topology_slot +define_id_show_func (slot); +define_one_ro (slot); +#define ref_slot_attr &attr_slot.attr, +#else +#define ref_slot_attr +#endif +#ifdef topology_core_iddefine_id_show_func (core_id);define_one_ro (core_id);@@ -83,6 +91,7 @@ define_one_ro(core_siblings);static struct attribute *default_attrs[] = { ref_physical_package_id_attr + ref_slot_attr ref_core_id_attr ref_thread_siblings_attr ref_core_siblings_attrinclude/asm-ia64/cpu.h +1
-0@@ -14,6 +14,7 @@ DECLARE_PER_CPU(struct ia64_cpu, cpu_devices);DECLARE_PER_CPU (int, cpu_state); +extern void arch_set_topology_slot (int num,u32 slot);extern int arch_register_cpu (int num);#ifdef CONFIG_HOTPLUG_CPU extern void arch_unregister_cpu (int );include/asm-ia64/processor.h +6
-1@@ -125,6 +125,10 @@ struct ia64_psr {*/ struct cpuinfo_ia64 {__u32 softirq_pending; + +#ifdef CONFIG_SMP +int cpu; +#endif __u64 itm_delta;/* # of clock cycles between clock ticks */ __u64 itm_next;/* interval timer mask value to use for next clock tick */ __u64 nsec_per_cyc;/* (1000000000<<IA64_NSEC_PER_CYC_SHIFT)/itc_freq */ @@ -140,7 +144,6 @@ struct cpuinfo_ia64 {#ifdef CONFIG_SMP __u64 loops_per_jiffy; -int cpu;__u32 socket_id;/* physical processor socket id */ __u16 core_id;/* core id */ __u16 thread_id;/* thread id */ @@ -150,6 +153,8 @@ struct cpuinfo_ia64 {__u8 threads_per_core;/* Threads per core */ #endif +__u32 slot;/* physical slot; can differ from socket_id */ +/* CPUID-derived information: */ __u64 ppn;__u64 features;include/asm-ia64/topology.h +3
-0@@ -110,12 +110,15 @@ void build_cpu_to_node_map(void);#ifdef CONFIG_SMP #define topology_physical_package_id (cpu) (cpu_data(cpu)->socket_id) +#define topology_slot (cpu) (cpu_data(cpu)->slot)#define topology_core_id (cpu) (cpu_data(cpu)->core_id)#define topology_core_siblings (cpu) (cpu_core_map[cpu])#define topology_thread_siblings (cpu) (per_cpu(cpu_sibling_map, cpu))#define smt_capable () (smp_num_siblings > 1)#endif +#define arch_set_topology_slot (num, slot) ia64_set_topology_slot(num,slot) +#include <asm-generic/topology.h> #endif /* _ASM_IA64_TOPOLOGY_H */ include/asm-x86/topology.h +2
-0@@ -180,6 +180,8 @@ extern cpumask_t cpu_coregroup_map(int cpu);#define topology_thread_siblings (cpu) (per_cpu(cpu_sibling_map, cpu))#endif +#define arch_set_topology_slot (num, slot) +#ifdef CONFIG_SMP #define mc_capable () (boot_cpu_data.x86_max_cores > 1)#define smt_capable () (smp_num_siblings > 1)Signature · 6 lines
1.5.3.1.g1e61 -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH, RFC] Create 'slot' sysfsattributein/sys/devices/system/cpu/cpuN/topology/
Hi Tony,
* Luck, Tony <[email protected]>:
Quoted 9 lines
Quoted 3 lines
I noticed that this patch wasn't in the git tree you sent to Linus for 2.6.26. I don't remember seeing a NACK though -- is there something that I could rework to make it more acceptable?
I'm mostly ok with this version of the patch. I didn't see any comments from the linux-kernel crowd on the whether they are fond of the new API ("slot" file in cpu/cpuN/topology/) and hate to make the presumption that because they are silent that they agree.
Quoted 8 lines
The "mostly ok" part would transform to "fully ok" if there were a way to make sure the "slot" files only appeared on systems where they are meaningful (i.e. have a value other then -1 in them). If there is an easy way to make this happen, then it would make me happier (less clutter in /sys) and perhaps others too (since this API is only useful on large systems where "slot" is meaningful, and there is generally some bias from the community at large about adding interfaces that aren't needed for normal desktop/laptop systems).
Yeah, I agree that the first few attempts weren't so great. I don't really like the idea of adding a new 'slot' file to sysfs either.
I reworked this patch to play nicer with the current topology stuff.
I've also included a cleanup patch that you might be happy to see (removes the noisy ia64_sal_pltid failed with -1 printk).
Tested on an hp rx4640 with Madisons and verified that I actually get sane output in /proc/cpuinfo:
[root@max ~]# cat /proc/cpuinfo | grep processor
processor : 0
processor : 1
processor : 2
processor : 3
[root@max ~]# cat /proc/cpuinfo | grep physical
physical id: 0
physical id: 1
physical id: 2
physical id: 3
Also tested on a Tiger and verified that /proc/cpuinfo remains unchanged, and no useless printk's appear during boot.
Thanks.
/ac
PATCH 1/2 ia64: Remove printk noise on unimplemented SAL_PHYSICAL_ID_INFO
Commit 113134fcbca83619be4c68d0ca66db6093777b5d changed the flow of control when calling PAL_LOGICAL_TO_PHYSICAL and SAL_PHYSICAL_ID_INFO. With the change, if a platform did not implement the latter, a useless printk would appear in the boot log:
ia64_sal_pltid failed with -1
So let's check the return code and only printk on a true error, and do not print anything in the unimplemented case. While we're in there, clean up some stylistic issues too.
--- arch/ia64/kernel/smpboot.c | 12 +++++++++--- 1 files changed, 9 insertions(+), 3 deletions(-)
arch/ia64/kernel/smpboot.c +9 -3
@@ -873,7 +873,8 @@ identify_siblings(struct cpuinfo_ia64 *c)u16 pltid;pal_logical_to_physical_t info; -if ((status =ia64_pal_logical_to_phys (-1 , &info)) !=PAL_STATUS_SUCCESS ) { + status =ia64_pal_logical_to_phys (-1 , &info); +if (status !=PAL_STATUS_SUCCESS ) {if (status !=PAL_STATUS_UNIMPLEMENTED ) { printk(KERN_ERR "ia64_pal_logical_to_phys failed with %ld\n",
@@ -885,8 +886,13 @@ identify_siblings(struct cpuinfo_ia64 *c) info.overview_cpp =1 ; info.overview_tpc =1 ; } -if ((status =ia64_sal_physical_id_info (&pltid)) !=PAL_STATUS_SUCCESS ) { -printk (KERN_ERR "ia64_sal_pltid failed with %ld\n" , status); + + status =ia64_sal_physical_id_info (&pltid); +if (status !=PAL_STATUS_SUCCESS ) { +if (status !=PAL_STATUS_UNIMPLEMENTED ) +printk (KERN_ERR +"ia64_sal_pltid failed with %ld\n" , + status);return ; }
Signature · 1 line
1.5.3.1.g1e61
Re: [PATCH 1/2] ia64: Remove printk noise on unimplemented SAL_PHYSICAL_ID_INFO
Ugh, next time, run checkpatch *before* sending out the patches. Sorry for the noise.
/ac
From: Alex Chiang <[email protected]>
Subject: [PATCH 1/2] ia64: Remove printk noise on unimplemented SAL_PHYSICAL_ID_INFO
Commit 113134fcbca83619be4c68d0ca66db6093777b5d changed the flow of control when calling PAL_LOGICAL_TO_PHYSICAL and SAL_PHYSICAL_ID_INFO. With the change, if a platform did not implement the latter, a useless printk would appear in the boot log:
ia64_sal_pltid failed with -1
So let's check the return code and only printk on a true error, and do not print anything in the unimplemented case. While we're in there, clean up some stylistic issues too.
--- arch/ia64/kernel/smpboot.c | 12 +++++++++--- 1 files changed, 9 insertions(+), 3 deletions(-)
arch/ia64/kernel/smpboot.c +9 -3
@@ -873,7 +873,8 @@ identify_siblings(struct cpuinfo_ia64 *c)u16 pltid;pal_logical_to_physical_t info; -if ((status =ia64_pal_logical_to_phys (-1 , &info)) !=PAL_STATUS_SUCCESS ) { + status =ia64_pal_logical_to_phys (-1 , &info); +if (status !=PAL_STATUS_SUCCESS ) {if (status !=PAL_STATUS_UNIMPLEMENTED ) { printk(KERN_ERR "ia64_pal_logical_to_phys failed with %ld\n",
@@ -885,8 +886,13 @@ identify_siblings(struct cpuinfo_ia64 *c) info.overview_cpp =1 ; info.overview_tpc =1 ; } -if ((status =ia64_sal_physical_id_info (&pltid)) !=PAL_STATUS_SUCCESS ) { -printk (KERN_ERR "ia64_sal_pltid failed with %ld\n" , status); + + status =ia64_sal_physical_id_info (&pltid); +if (status !=PAL_STATUS_SUCCESS ) { +if (status !=PAL_STATUS_UNIMPLEMENTED ) +printk (KERN_ERR +"ia64_sal_pltid failed with %ld\n" , + status);return ; }
Signature · 1 line
1.5.3.1.g1e61
PATCH 2/2 ia64: Provide ACPI fixup for /proc/cpuinfo/physical_id
In reply to Alex ChiangLegacy HP ia64 platforms currently cannot provide /proc/cpuinfo/physical_id due to legacy SAL/PAL implementations. However, that physical topology information can be obtained via ACPI.
Provide an interface that gives ACPI one last chance to provide physical_id for these legacy platforms. This logic only comes into play iff:
- ACPI actually provides slot information for the CPU
- we lack a valid socket_id
Otherwise, we don't do anything. Since x86 uses the ACPI processor driver as well, we provide an empty #define.
--- arch/ia64/kernel/topology.c | 7 +++++++ drivers/acpi/processor_core.c | 8 ++++++++ include/asm-ia64/cpu.h | 1 + include/asm-ia64/topology.h | 1 + include/asm-x86/topology.h | 2 ++ 5 files changed, 19 insertions(+), 0 deletions(-)
arch/ia64/kernel/topology.c +7 -0
@@ -27,6 +27,13 @@static struct ia64_cpu *sysfs_cpus; +void ia64_fix_socket_id (int num,u32 slot) +{ +if (cpu_data (num)->socket_id ==-1 ) +cpu_data (num)->socket_id= slot; +} +EXPORT_SYMBOL_GPL (ia64_fix_socket_id); +int arch_register_cpu (int num) {#if defined (CONFIG_ACPI ) && defined (CONFIG_HOTPLUG_CPU )
drivers/acpi/processor_core.c +8 -0
@@ -612,6 +612,14 @@ static int acpi_processor_get_info(struct acpi_processor *pr, unsigned has_uid)request_region (pr->throttling.address,6 ,"ACPI CPU throttle" ); } +/* +* If ACPI describes a slot number for this CPU, we can use it +* to fix a broken topology->physical_package_id. +*/ + status =acpi_evaluate_object (pr->handle,"_SUN" ,NULL , &buffer); +if (ACPI_SUCCESS (status)) +arch_fix_phys_package_id (pr->id, object.integer.value); +return 0 ; }
include/asm-ia64/cpu.h +1 -0
@@ -14,6 +14,7 @@ DECLARE_PER_CPU(struct ia64_cpu, cpu_devices);DECLARE_PER_CPU (int, cpu_state); +extern void arch_fix_phys_package_id (int num,u32 slot);extern int arch_register_cpu (int num);#ifdef CONFIG_HOTPLUG_CPU extern void arch_unregister_cpu (int );
include/asm-ia64/topology.h +1 -0
@@ -116,6 +116,7 @@ void build_cpu_to_node_map(void);#define smt_capable () (smp_num_siblings > 1)#endif +#define arch_fix_phys_package_id (num, slot) ia64_fix_socket_id(num, slot)#define pcibus_to_cpumask (bus) (pcibus_to_node(bus) == -1 ? \ CPU_MASK_ALL : \ node_to_cpumask(pcibus_to_node(bus)) \
include/asm-x86/topology.h +2 -0
@@ -193,6 +193,8 @@ extern cpumask_t cpu_coregroup_map(int cpu);#define topology_thread_siblings (cpu) (per_cpu(cpu_sibling_map, cpu))#endif +#define arch_fix_phys_package_id (num, slot) +#ifdef CONFIG_SMP #define mc_capable () (boot_cpu_data.x86_max_cores > 1)#define smt_capable () (smp_num_siblings > 1)
Signature · 1 line
1.5.3.1.g1e61
Re: [PATCH 2/2] ia64: Provide ACPI fixup for /proc/cpuinfo/physical_id
Fix the checkpatch issues in this patch as well.
/ac
From: Alex Chiang <[email protected]>
Subject: [PATCH 2/2] ia64: Provide ACPI fixup for /proc/cpuinfo/physical_id
Legacy HP ia64 platforms currently cannot provide /proc/cpuinfo/physical_id due to legacy SAL/PAL implementations. However, that physical topology information can be obtained via ACPI.
Provide an interface that gives ACPI one last chance to provide physical_id for these legacy platforms. This logic only comes into play iff:
- ACPI actually provides slot information for the CPU
- we lack a valid socket_id
Otherwise, we don't do anything. Since x86 uses the ACPI processor driver as well, we provide an empty #define.
--- arch/ia64/kernel/topology.c | 7 +++++++ drivers/acpi/processor_core.c | 8 ++++++++ include/asm-ia64/cpu.h | 1 + include/asm-ia64/topology.h | 1 + include/asm-x86/topology.h | 2 ++ 5 files changed, 19 insertions(+), 0 deletions(-)
arch/ia64/kernel/topology.c +7 -0
@@ -27,6 +27,13 @@static struct ia64_cpu *sysfs_cpus; +void ia64_fix_socket_id (int num,u32 slot) +{ +if (cpu_data (num)->socket_id ==-1 ) +cpu_data (num)->socket_id = slot; +} +EXPORT_SYMBOL_GPL (ia64_fix_socket_id); +int arch_register_cpu (int num) {#if defined (CONFIG_ACPI ) && defined (CONFIG_HOTPLUG_CPU )
drivers/acpi/processor_core.c +8 -0
@@ -612,6 +612,14 @@ static int acpi_processor_get_info(struct acpi_processor *pr, unsigned has_uid)request_region (pr->throttling.address,6 ,"ACPI CPU throttle" ); } +/* +* If ACPI describes a slot number for this CPU, we can use it +* to fix a broken topology->physical_package_id. +*/ + status =acpi_evaluate_object (pr->handle,"_SUN" ,NULL , &buffer); +if (ACPI_SUCCESS (status)) +arch_fix_phys_package_id (pr->id, object.integer.value); +return 0 ; }
include/asm-ia64/cpu.h +1 -0
@@ -14,6 +14,7 @@ DECLARE_PER_CPU(struct ia64_cpu, cpu_devices);DECLARE_PER_CPU (int, cpu_state); +extern void arch_fix_phys_package_id (int num,u32 slot);extern int arch_register_cpu (int num);#ifdef CONFIG_HOTPLUG_CPU extern void arch_unregister_cpu (int );
include/asm-ia64/topology.h +1 -0
@@ -116,6 +116,7 @@ void build_cpu_to_node_map(void);#define smt_capable () (smp_num_siblings > 1)#endif +#define arch_fix_phys_package_id (num, slot) ia64_fix_socket_id(num, slot)#define pcibus_to_cpumask (bus) (pcibus_to_node(bus) == -1 ? \ CPU_MASK_ALL : \ node_to_cpumask(pcibus_to_node(bus)) \
include/asm-x86/topology.h +2 -0
@@ -193,6 +193,8 @@ extern cpumask_t cpu_coregroup_map(int cpu);#define topology_thread_siblings (cpu) (per_cpu(cpu_sibling_map, cpu))#endif +#define arch_fix_phys_package_id (num, slot) +#ifdef CONFIG_SMP #define mc_capable () (boot_cpu_data.x86_max_cores > 1)#define smt_capable () (smp_num_siblings > 1)
Signature · 1 line
1.5.3.1.g1e61
RE: [PATCH 2/2] ia64: Provide ACPI fixup for /proc/cpuinfo/physical_id
In reply to Alex ChiangAlex,
I chewed on this a bit ...
+void ia64_fix_socket_id(int num, u32 slot)
+{
+ if (cpu_data(num)->socket_id == -1)
+ cpu_data(num)->socket_id= slot;
+}
+EXPORT_SYMBOL_GPL(ia64_fix_socket_id);
1) socket_id is only defined when CONFIG_SMP=y, so this breaks UP build. Fix: Wrap body of this function inside #ifdef
2) should really just name this function "arch_fix_phys_package_id" and avoid the #define rename in topology.h
drivers/acpi/processor_core.c +0 -0
+ /*
+ * If ACPI describes a slot number for this CPU, we can use it
+ * to fix a broken topology->physical_package_id.
+ */
+ status = acpi_evaluate_object(pr->handle, "_SUN", NULL, &buffer);
+ if (ACPI_SUCCESS(status))
+ arch_fix_phys_package_id(pr->id, object.integer.value);
+
Comment here still refers to what the previous patch was doing. Should now say that we are fixing /proc/cpuinfo "physical id".
include/asm-ia64/topology.h +0 -0
+#define arch_fix_phys_package_id(num, slot) ia64_fix_socket_id(num, slot)
Not needed if we name ia64_fix_socket_id() appropriately. Turn this into an extern definition for arch_fix_phys_package_id() and drop the extern from cpu.h
include/asm-x86/topology.h +0 -0
+#define arch_fix_phys_package_id(num, slot)
Style preference is to now use a "static inline void" function for this to make sure args are correctly typed, and to avoid warnings about unused variables.
updated version of the patch looks like this ... ok???
-Tony
commit fe086a7bea7ab714930bd48addba961ceeef7634
Author: Alex Chiang <[email protected]> Date: Tue Apr 29 15:05:29 2008 -0700
[IA64] Provide ACPI fixup for /proc/cpuinfo/physical_id
Legacy HP ia64 platforms currently cannot provide /proc/cpuinfo/physical_id due to legacy SAL/PAL implementations. However, that physical topology information can be obtained via ACPI.
Provide an interface that gives ACPI one last chance to provide physical_id for these legacy platforms. This logic only comes into play iff:
- ACPI actually provides slot information for the CPU
- we lack a valid socket_id
Otherwise, we don't do anything.
Since x86 uses the ACPI processor driver as well, we provide a nop stub function for arch_fix_phys_package_id() in asm-x86/topology.h
arch/ia64/kernel/topology.c +9 -0
@@ -27,6 +27,15 @@static struct ia64_cpu *sysfs_cpus; +void arch_fix_phys_package_id (int num,u32 slot) +{ +#ifdef CONFIG_SMP +if (cpu_data (num)->socket_id ==-1 ) +cpu_data (num)->socket_id = slot; +#endif +} +EXPORT_SYMBOL_GPL (arch_fix_phys_package_id); +int arch_register_cpu (int num) {#if defined (CONFIG_ACPI ) && defined (CONFIG_HOTPLUG_CPU )
drivers/acpi/processor_core.c +9 -0
@@ -603,6 +603,15 @@ static int acpi_processor_get_info(struct acpi_processor *pr, unsigned has_uid)request_region (pr->throttling.address,6 ,"ACPI CPU throttle" ); } +/* +* If ACPI describes a slot number for this CPU, we can use it +* ensure we get the right value in the "physical id" field +* of /proc/cpuinfo +*/ + status =acpi_evaluate_object (pr->handle,"_SUN" ,NULL , &buffer); +if (ACPI_SUCCESS (status)) +arch_fix_phys_package_id (pr->id, object.integer.value); +return 0 ; }
include/asm-ia64/topology.h +2 -0
@@ -116,6 +116,8 @@ void build_cpu_to_node_map(void);#define smt_capable () (smp_num_siblings > 1)#endif +extern void arch_fix_phys_package_id (int num,u32 slot); +#define pcibus_to_cpumask (bus) (pcibus_to_node(bus) == -1 ? \ CPU_MASK_ALL : \ node_to_cpumask(pcibus_to_node(bus)) \
include/asm-x86/topology.h +4 -0
@@ -193,6 +193,10 @@ extern cpumask_t cpu_coregroup_map(int cpu);#define topology_thread_siblings (cpu) (per_cpu(cpu_sibling_map, cpu))#endif +static inline void arch_fix_phys_package_id (int num,u32 slot) +{ +} +struct pci_bus ;void set_pci_bus_resources_arch_default (struct pci_bus *b);
Re: [PATCH 2/2] ia64: Provide ACPI fixup for /proc/cpuinfo/physical_id
Hi Tony,
* Luck, Tony <[email protected]>:
Quoted 3 lines
Alex,
I chewed on this a bit ...
Thanks for all the fixups -- your version looks a lot nicer!
Quoted 6 lines
include/asm-x86/topology.h +1
-0+#define arch_fix_phys_package_id (num, slot)Style preference is to now use a "static inline void" function for this to make sure args are correctly typed, and to avoid warnings about unused variables.
*nod*
I'll use that convention in the future, thanks.
Quoted 1 line
updated version of the patch looks like this ... ok???
Looks great to me.
/ac
Quoted 99 lines
-Tony
commit
fe086a7bea7ab714930bd48addba961ceeef7634Author: Alex Chiang <[email protected]> Date: Tue Apr 29 15:05:29 2008 -0700[IA64] Provide ACPI fixup for
/proc/cpuinfo/physical_idLegacy HP ia64 platforms currently cannot provide
/proc/cpuinfo/physical_iddue to legacy SAL/PAL implementations. However, that physical topology information can be obtained via ACPI.Provide an interface that gives ACPI one last chance to provide
physical_idfor these legacy platforms. This logic only comes into play iff:- ACPI actually provides slot information for the CPU
- we lack a validsocket_idOtherwise, we don't do anything.
Since x86 uses the ACPI processor driver as well, we provide a nop stub function for
arch_fix_phys_package_id()inasm-x86/topology.hSigned-off-by Alex Chiang <[email protected]>Signed-off-by Tony Luck <[email protected]>arch/ia64/kernel/topology.c +9
-0@@ -27,6 +27,15 @@static struct ia64_cpu *sysfs_cpus; +void arch_fix_phys_package_id (int num,u32 slot) +{ +#ifdef CONFIG_SMP +if (cpu_data (num)->socket_id ==-1 ) +cpu_data (num)->socket_id = slot; +#endif +} +EXPORT_SYMBOL_GPL (arch_fix_phys_package_id); +int arch_register_cpu (int num) {#if defined (CONFIG_ACPI ) && defined (CONFIG_HOTPLUG_CPU )drivers/acpi/processor_core.c +9
-0@@ -603,6 +603,15 @@ static int acpi_processor_get_info(struct acpi_processor *pr, unsigned has_uid)request_region (pr->throttling.address,6 ,"ACPI CPU throttle" ); } +/* +* If ACPI describes a slot number for this CPU, we can use it +* ensure we get the right value in the "physical id" field +* of /proc/cpuinfo +*/ + status =acpi_evaluate_object (pr->handle,"_SUN" ,NULL , &buffer); +if (ACPI_SUCCESS (status)) +arch_fix_phys_package_id (pr->id, object.integer.value); +return 0 ; }include/asm-ia64/topology.h +2
-0@@ -116,6 +116,8 @@ void build_cpu_to_node_map(void);#define smt_capable () (smp_num_siblings > 1)#endif +extern void arch_fix_phys_package_id (int num,u32 slot); +#define pcibus_to_cpumask (bus) (pcibus_to_node(bus) == -1 ? \ CPU_MASK_ALL : \ node_to_cpumask(pcibus_to_node(bus)) \include/asm-x86/topology.h +4
-0@@ -193,6 +193,10 @@ extern cpumask_t cpu_coregroup_map(int cpu);#define topology_thread_siblings (cpu) (per_cpu(cpu_sibling_map, cpu))#endif +static inline void arch_fix_phys_package_id (int num,u32 slot) +{ +} +struct pci_bus ;void set_pci_bus_resources_arch_default (struct pci_bus *b);