Login | Register For Free | Help
Search for: (Advanced)

Mailing List Archive: Linux: Kernel

[PATCH, RFC] Create 'slot' sysfs attribute in /sys/devices/system/cpu/cpuN/topology/

 

 

Linux kernel RSS feed   Index | Next | Previous | View Threaded


achiang at hp

Mar 10, 2008, 3:27 PM

Post #1 of 11 (1133 views)
Permalink
[PATCH, RFC] 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 <achiang [at] hp>

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.

Signed-off-by: Alex Chiang <achiang [at] hp>
---
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(-)

diff --git a/arch/ia64/kernel/setup.c b/arch/ia64/kernel/setup.c
index ebd1a09..34de68a 100644
--- a/arch/ia64/kernel/setup.c
+++ b/arch/ia64/kernel/setup.c
@@ -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);

diff --git a/arch/ia64/kernel/topology.c b/arch/ia64/kernel/topology.c
index a2484fc..3810b48 100644
--- a/arch/ia64/kernel/topology.c
+++ b/arch/ia64/kernel/topology.c
@@ -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)
diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
index a3cc8a9..89a9ad5 100644
--- a/drivers/acpi/processor_core.c
+++ b/drivers/acpi/processor_core.c
@@ -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)
diff --git a/drivers/base/topology.c b/drivers/base/topology.c
index e1d3ad4..4fcffca 100644
--- a/drivers/base/topology.c
+++ b/drivers/base/topology.c
@@ -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_id
define_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
diff --git a/include/asm-ia64/cpu.h b/include/asm-ia64/cpu.h
index e87fa32..393e545 100644
--- a/include/asm-ia64/cpu.h
+++ b/include/asm-ia64/cpu.h
@@ -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);
diff --git a/include/asm-ia64/processor.h b/include/asm-ia64/processor.h
index 741f7ec..066553c 100644
--- a/include/asm-ia64/processor.h
+++ b/include/asm-ia64/processor.h
@@ -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;
diff --git a/include/asm-ia64/topology.h b/include/asm-ia64/topology.h
index 2d67b72..3b0ab1c 100644
--- a/include/asm-ia64/topology.h
+++ b/include/asm-ia64/topology.h
@@ -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))
diff --git a/include/asm-x86/cpu.h b/include/asm-x86/cpu.h
index 73f2ea8..43132a2 100644
--- a/include/asm-x86/cpu.h
+++ b/include/asm-x86/cpu.h
@@ -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);
--
1.5.3.1.g1e61

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


tony.luck at intel

Mar 11, 2008, 10:31 AM

Post #2 of 11 (1100 views)
Permalink
RE: [PATCH, RFC] Create 'slot' sysfs attribute in/sys/devices/system/cpu/cpuN/topology/ [In reply to]

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

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


matthew at wil

Mar 11, 2008, 10:48 AM

Post #3 of 11 (1112 views)
Permalink
Re: [PATCH, RFC] Create 'slot' sysfs attribute in/sys/devices/system/cpu/cpuN/topology/ [In reply to]

On Tue, Mar 11, 2008 at 10:31:43AM -0700, Luck, Tony wrote:
> 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.

--
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."
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


achiang at hp

Mar 12, 2008, 8:45 AM

Post #4 of 11 (1128 views)
Permalink
Re: [PATCH, RFC] Create 'slot' sysfs attribute in/sys/devices/system/cpu/cpuN/topology/ [In reply to]

* Luck, Tony <tony.luck [at] intel>:
> > 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.
>
> 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. ;)

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

> 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


tony.luck at intel

Mar 12, 2008, 2:42 PM

Post #5 of 11 (1098 views)
Permalink
RE: [PATCH, RFC] Create 'slot' sysfs attribute in/sys/devices/system/cpu/cpuN/topology/ [In reply to]

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

> +#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).

> +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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


achiang at hp

Mar 19, 2008, 4:31 PM

Post #6 of 11 (1103 views)
Permalink
Re: [PATCH, RFC] Create 'slot' sysfs attribute in/sys/devices/system/cpu/cpuN/topology/ [In reply to]

Hi Tony,

* Luck, Tony <tony.luck [at] intel>:
> > 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.

> > +#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).

Hrm, I was just following a precedent here. I guess I'll wait for
enlightenment from akpm too. ;)

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

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

> $ cat /sys/devices/system/cpu/cpu0/topology/slot
> -1
>
> So I guess that the _SUN method 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 <achiang [at] hp>
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

Signed-off-by: Alex Chiang <achiang [at] hp>
---
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(-)

diff --git a/arch/ia64/kernel/setup.c b/arch/ia64/kernel/setup.c
index 4aa9eae..228d960 100644
--- a/arch/ia64/kernel/setup.c
+++ b/arch/ia64/kernel/setup.c
@@ -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);

diff --git a/arch/ia64/kernel/topology.c b/arch/ia64/kernel/topology.c
index a2484fc..512de14 100644
--- a/arch/ia64/kernel/topology.c
+++ b/arch/ia64/kernel/topology.c
@@ -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)
diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
index 36a68fa..49fb2a0 100644
--- a/drivers/acpi/processor_core.c
+++ b/drivers/acpi/processor_core.c
@@ -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)
diff --git a/drivers/base/topology.c b/drivers/base/topology.c
index e1d3ad4..4fcffca 100644
--- a/drivers/base/topology.c
+++ b/drivers/base/topology.c
@@ -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_id
define_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
diff --git a/include/asm-ia64/cpu.h b/include/asm-ia64/cpu.h
index e87fa32..393e545 100644
--- a/include/asm-ia64/cpu.h
+++ b/include/asm-ia64/cpu.h
@@ -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);
diff --git a/include/asm-ia64/processor.h b/include/asm-ia64/processor.h
index 741f7ec..066553c 100644
--- a/include/asm-ia64/processor.h
+++ b/include/asm-ia64/processor.h
@@ -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;
diff --git a/include/asm-ia64/topology.h b/include/asm-ia64/topology.h
index 2d67b72..f986693 100644
--- a/include/asm-ia64/topology.h
+++ b/include/asm-ia64/topology.h
@@ -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 */
diff --git a/include/asm-x86/topology.h b/include/asm-x86/topology.h
index 8af05a9..d4c922c 100644
--- a/include/asm-x86/topology.h
+++ b/include/asm-x86/topology.h
@@ -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)
--
1.5.3.1.g1e61

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


tony.luck at intel

Mar 21, 2008, 4:58 PM

Post #7 of 11 (1101 views)
Permalink
RE: [PATCH, RFC] Create 'slot' sysfs attributein/sys/devices/system/cpu/cpuN/topology/ [In reply to]

+ 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


achiang at hp

Mar 26, 2008, 11:59 AM

Post #8 of 11 (1090 views)
Permalink
Re: [PATCH, RFC] Create 'slot' sysfs attributein/sys/devices/system/cpu/cpuN/topology/ [In reply to]

Hi Tony,

* Luck, Tony <tony.luck [at] intel>:
> + 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.

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 <achiang [at] hp>

/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

Signed-off-by: Alex Chiang <achiang [at] hp>
---
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(-)

diff --git a/arch/ia64/kernel/setup.c b/arch/ia64/kernel/setup.c
index 4aa9eae..228d960 100644
--- a/arch/ia64/kernel/setup.c
+++ b/arch/ia64/kernel/setup.c
@@ -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);

diff --git a/arch/ia64/kernel/topology.c b/arch/ia64/kernel/topology.c
index a2484fc..512de14 100644
--- a/arch/ia64/kernel/topology.c
+++ b/arch/ia64/kernel/topology.c
@@ -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)
diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
index 36a68fa..a3a740d 100644
--- a/drivers/acpi/processor_core.c
+++ b/drivers/acpi/processor_core.c
@@ -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;
}

diff --git a/drivers/base/topology.c b/drivers/base/topology.c
index e1d3ad4..4fcffca 100644
--- a/drivers/base/topology.c
+++ b/drivers/base/topology.c
@@ -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_id
define_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
diff --git a/include/asm-ia64/cpu.h b/include/asm-ia64/cpu.h
index e87fa32..393e545 100644
--- a/include/asm-ia64/cpu.h
+++ b/include/asm-ia64/cpu.h
@@ -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);
diff --git a/include/asm-ia64/processor.h b/include/asm-ia64/processor.h
index 741f7ec..066553c 100644
--- a/include/asm-ia64/processor.h
+++ b/include/asm-ia64/processor.h
@@ -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;
diff --git a/include/asm-ia64/topology.h b/include/asm-ia64/topology.h
index 2d67b72..f986693 100644
--- a/include/asm-ia64/topology.h
+++ b/include/asm-ia64/topology.h
@@ -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 */
diff --git a/include/asm-x86/topology.h b/include/asm-x86/topology.h
index 8af05a9..d4c922c 100644
--- a/include/asm-x86/topology.h
+++ b/include/asm-x86/topology.h
@@ -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)
--
1.5.3.1.g1e61

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


achiang at hp

Apr 20, 2008, 10:24 PM

Post #9 of 11 (1062 views)
Permalink
Re: [PATCH, RFC] Create 'slot' sysfs attributein/sys/devices/system/cpu/cpuN/topology/ [In reply to]

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 <achiang [at] hp>:
> Hi Tony,
>
> * Luck, Tony <tony.luck [at] intel>:
> > + 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.
>
> 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 <achiang [at] hp>
>
> /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
>
> Signed-off-by: Alex Chiang <achiang [at] hp>
> ---
> 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(-)
>
> diff --git a/arch/ia64/kernel/setup.c b/arch/ia64/kernel/setup.c
> index 4aa9eae..228d960 100644
> --- a/arch/ia64/kernel/setup.c
> +++ b/arch/ia64/kernel/setup.c
> @@ -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);
>
> diff --git a/arch/ia64/kernel/topology.c b/arch/ia64/kernel/topology.c
> index a2484fc..512de14 100644
> --- a/arch/ia64/kernel/topology.c
> +++ b/arch/ia64/kernel/topology.c
> @@ -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)
> diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
> index 36a68fa..a3a740d 100644
> --- a/drivers/acpi/processor_core.c
> +++ b/drivers/acpi/processor_core.c
> @@ -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;
> }
>
> diff --git a/drivers/base/topology.c b/drivers/base/topology.c
> index e1d3ad4..4fcffca 100644
> --- a/drivers/base/topology.c
> +++ b/drivers/base/topology.c
> @@ -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_id
> define_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
> diff --git a/include/asm-ia64/cpu.h b/include/asm-ia64/cpu.h
> index e87fa32..393e545 100644
> --- a/include/asm-ia64/cpu.h
> +++ b/include/asm-ia64/cpu.h
> @@ -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);
> diff --git a/include/asm-ia64/processor.h b/include/asm-ia64/processor.h
> index 741f7ec..066553c 100644
> --- a/include/asm-ia64/processor.h
> +++ b/include/asm-ia64/processor.h
> @@ -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;
> diff --git a/include/asm-ia64/topology.h b/include/asm-ia64/topology.h
> index 2d67b72..f986693 100644
> --- a/include/asm-ia64/topology.h
> +++ b/include/asm-ia64/topology.h
> @@ -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 */
> diff --git a/include/asm-x86/topology.h b/include/asm-x86/topology.h
> index 8af05a9..d4c922c 100644
> --- a/include/asm-x86/topology.h
> +++ b/include/asm-x86/topology.h
> @@ -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)
> --
> 1.5.3.1.g1e61
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo [at] vger
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


tony.luck at intel

Apr 21, 2008, 3:00 PM

Post #10 of 11 (1070 views)
Permalink
RE: [PATCH, RFC] Create 'slot' sysfsattributein/sys/devices/system/cpu/cpuN/topology/ [In reply to]

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


> Subject: [PATCH] Create 'slot' sysfs attribute in /sys/devices/system/cpu/cpuN/topology/
> From: Alex Chiang <achiang [at] hp>
>
> /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
>
> Signed-off-by: Alex Chiang <achiang [at] hp>
> ---
> 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(-)
>
> diff --git a/arch/ia64/kernel/setup.c b/arch/ia64/kernel/setup.c
> index 4aa9eae..228d960 100644
> --- a/arch/ia64/kernel/setup.c
> +++ b/arch/ia64/kernel/setup.c
> @@ -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);
>
> diff --git a/arch/ia64/kernel/topology.c b/arch/ia64/kernel/topology.c
> index a2484fc..512de14 100644
> --- a/arch/ia64/kernel/topology.c
> +++ b/arch/ia64/kernel/topology.c
> @@ -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)
> diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
> index 36a68fa..a3a740d 100644
> --- a/drivers/acpi/processor_core.c
> +++ b/drivers/acpi/processor_core.c
> @@ -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;
> }
>
> diff --git a/drivers/base/topology.c b/drivers/base/topology.c
> index e1d3ad4..4fcffca 100644
> --- a/drivers/base/topology.c
> +++ b/drivers/base/topology.c
> @@ -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_id
> define_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
> diff --git a/include/asm-ia64/cpu.h b/include/asm-ia64/cpu.h
> index e87fa32..393e545 100644
> --- a/include/asm-ia64/cpu.h
> +++ b/include/asm-ia64/cpu.h
> @@ -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);
> diff --git a/include/asm-ia64/processor.h b/include/asm-ia64/processor.h
> index 741f7ec..066553c 100644
> --- a/include/asm-ia64/processor.h
> +++ b/include/asm-ia64/processor.h
> @@ -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;
> diff --git a/include/asm-ia64/topology.h b/include/asm-ia64/topology.h
> index 2d67b72..f986693 100644
> --- a/include/asm-ia64/topology.h
> +++ b/include/asm-ia64/topology.h
> @@ -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 */
> diff --git a/include/asm-x86/topology.h b/include/asm-x86/topology.h
> index 8af05a9..d4c922c 100644
> --- a/include/asm-x86/topology.h
> +++ b/include/asm-x86/topology.h
> @@ -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)
> --
> 1.5.3.1.g1e61
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo [at] vger
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


achiang at hp

Apr 24, 2008, 11:44 AM

Post #11 of 11 (1063 views)
Permalink
Re: [PATCH, RFC] Create 'slot' sysfsattributein/sys/devices/system/cpu/cpuN/topology/ [In reply to]

Hi Tony,

* Luck, Tony <tony.luck [at] intel>:
> > 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).

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 [at] ma ~]# cat /proc/cpuinfo | grep processor
processor : 0
processor : 1
processor : 2
processor : 3
[root [at] ma ~]# 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Linux kernel RSS feed   Index | Next | Previous | View Threaded
 
 


Interested in having your list archived? Contact Gossamer Threads
 
  Web Applications & Managed Hosting Powered by Gossamer Threads Inc.