Expose _SUN in /proc/acpi/processor/<...>/info

RFC PATCH New Alex Chiang 2 participants 4 messages Oct 19, 2007 – Dec 13, 2007

Subsystems (2)

ACPI ACPI component architecture (acpica)

lore

Share & tools

lore

Alex Chiang

author

2007-10-19 19:39 UTC #0 lore raw

RFC PATCH 1/1 Expose _SUN in /proc/acpi/processor/<...>/info

Hello,

I'm looking for comments on the following patch that exposes _SUN on processor objects in /proc/acpi/processor/<...>/info.

I notice that acpiphp exposes _SUN for PCI slots in /sys/bus/pci/slots/N/, where N is the value of _SUN. I'm not so sure we want to go *exactly* in the same direction for CPUs, because:

- x86 systems might not implement _SUN for processors
- ia64 systems that implement ACPI 2.x do not have unique values for _SUN across the entire system

Nonetheless, even non-unique values of _SUN are still helpful for userspace to figure out which physical socket a CPU might be in, especially when combined with information from /proc/cpuinfo.

Thoughts?

Thanks.

/ac

From: Alex Chiang <[email protected]>

On platforms that provide _SUN for the processor object, higher level software can consume the data to learn physical topology information, so let's expose it.

Platforms that do not implement _SUN will see <not supported>.

Signed-off-by Alex Chiang <[email protected]>

---
 drivers/acpi/processor_core.c |   11 +++++++++++
 include/acpi/processor.h      |    1 +
 2 files changed, 12 insertions(+), 0 deletions(-)

drivers/acpi/processor_core.c +11 -0

index 9f11dc2..b44d733 100644
@@ -301,6 +301,11 @@ static int acpi_processor_info_seq_show(struct seq_file *seq, void *offset)
 		   pr->flags.throttling ? "yes" : "no",
 		   pr->flags.limit ? "yes" : "no");
 
+	if (pr->sun == -1)
+		seq_printf(seq, "slot user number:        <not supported>\n");
+	else
+		seq_printf(seq, "slot user number:        %d\n", pr->sun);
+
       end:
 	return 0;
 }

@@ -595,6 +600,12 @@ 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;
+
+	pr->sun = 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)

include/acpi/processor.h +1 -0

index 99934a9..326a3da 100644
@@ -206,6 +206,7 @@ struct acpi_processor {
 	u32 acpi_id;
 	u32 id;
 	u32 pblk;
+	u32 sun;
 	int performance_platform_limit;
 	int throttling_platform_limit;
 	/* 0 - states 0..n-th state available */

Signature · 1 line

1.5.3.1.1.g1e61

Alex Chiang

author

2007-10-30 23:51 UTC #1 lore raw

RFC PATCH 1/1 Expose _SUN in /proc/acpi/processor/<...>/info

Hello,

I'm looking for comments on the following patch that exposes _SUN on processor objects in /proc/acpi/processor/<...>/info.

This didn't get many comments on the linux-acpi list, so I'm sending to a wider distribution.

I notice that acpiphp exposes _SUN for (hotpluggable) PCI slots in /sys/bus/pci/slots/N/, where N is the value of _SUN. I'm not so sure we want to go *exactly* in the same direction for CPUs, because:

- x86 systems might not implement _SUN for processors
- ia64 systems that implement ACPI 2.x do not have unique values for _SUN across the entire system

Nonetheless, even non-unique values of _SUN are still helpful for userspace to figure out which physical socket a CPU might be in, especially when combined with information from /proc/cpuinfo.

Thoughts?

Thanks.

/ac

From: Alex Chiang <[email protected]>

On platforms that provide _SUN for the processor object, higher level software can consume the data to learn physical topology information, so let's expose it.

Platforms that do not implement _SUN will see <not supported>.

Signed-off-by Alex Chiang <[email protected]>

---
 drivers/acpi/processor_core.c |   11 +++++++++++
 include/acpi/processor.h      |    1 +
 2 files changed, 12 insertions(+), 0 deletions(-)

drivers/acpi/processor_core.c +11 -0

index 235a51e..86cb41a 100644
@@ -302,6 +302,11 @@ static int acpi_processor_info_seq_show(struct seq_file *seq, void *offset)
 		   pr->flags.throttling ? "yes" : "no",
 		   pr->flags.limit ? "yes" : "no");
 
+	if (pr->sun == -1)
+		seq_printf(seq, "slot user number:        <not supported>\n");
+	else
+		seq_printf(seq, "slot user number:        %d\n", (int)pr->sun);
+
       end:
 	return 0;
 }

@@ -590,6 +595,12 @@ 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;
+
+	pr->sun = 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)

include/acpi/processor.h +1 -0

index 26d79f6..81792c6 100644
@@ -210,6 +210,7 @@ struct acpi_processor {
 	u32 acpi_id;
 	u32 id;
 	u32 pblk;
+	acpi_native_int sun;
 	int performance_platform_limit;
 	int throttling_platform_limit;
 	/* 0 - states 0..n-th state available */

Signature · 1 line

1.5.3.1.1.g1e61

Len Brown

reviewer

2007-11-19 17:54 UTC #2 lore raw

Quoted 17 lines · Alex Chiang

Hello,
I'm looking for comments on the following patch that exposes _SUN on processor objects in /proc/acpi/processor/<...>/info.
This didn't get many comments on the linux-acpi list, so I'm sending to a wider distribution.
I notice that acpiphp exposes _SUN for (hotpluggable) PCI slots in /sys/bus/pci/slots/N/, where N is the value of _SUN. I'm not so sure we want to go *exactly* in the same direction for CPUs, because:
- x86 systems might not implement _SUN for processors
- ia64 systems that implement ACPI 2.x do not have unique values for _SUN across the entire system

Quoted 5 lines

Nonetheless, even non-unique values of _SUN are still helpful for userspace to figure out which physical socket a CPU might be in, especially when combined with information from /proc/cpuinfo.
Thoughts?

We've not allowed new features in /proc/acpi since we started removing /proc/acpi.
ie. we don't want to update the API, we want to delete it.

If this is a useful thing for user-space to know, then you need to figure out a generic way to present it -- likely under /sys/devices/system/cpu/

Andrew, please remove acpi-expose-_sun-in-proc-acpi-processor-info.patch from the mm series.

thanks,
-Len

Quoted 56 lines

From: Alex Chiang <[email protected]>

On platforms that provide _SUN for the processor object, higher level software can consume the data to learn physical topology information, so let's expose it.

Platforms that do not implement _SUN will see <not supported>.

Signed-off-by Alex Chiang <[email protected]>

---
 drivers/acpi/processor_core.c |   11 +++++++++++
 include/acpi/processor.h      |    1 +
 2 files changed, 12 insertions(+), 0 deletions(-)

drivers/acpi/processor_core.c +11 -0

index 235a51e..86cb41a 100644
@@ -302,6 +302,11 @@ static int acpi_processor_info_seq_show(struct seq_file *seq, void *offset)
 		   pr->flags.throttling ? "yes" : "no",
 		   pr->flags.limit ? "yes" : "no");
 
+	if (pr->sun == -1)
+		seq_printf(seq, "slot user number:        <not supported>\n");
+	else
+		seq_printf(seq, "slot user number:        %d\n", (int)pr->sun);
+
       end:
 	return 0;
 }

@@ -590,6 +595,12 @@ 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;
+
+	pr->sun = 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)

include/acpi/processor.h +1 -0

index 26d79f6..81792c6 100644
@@ -210,6 +210,7 @@ struct acpi_processor {
 	u32 acpi_id;
 	u32 id;
 	u32 pblk;
+	acpi_native_int sun;
 	int performance_platform_limit;
 	int throttling_platform_limit;
 	/* 0 - states 0..n-th state available */

Alex Chiang

author

2007-12-13 19:08 UTC #3 lore raw

Hi Len,

* Len Brown <[email protected]>:

Quoted 21 lines · Alex Chiang

Quoted 20 lines
I'm looking for comments on the following patch that exposes _SUN on processor objects in /proc/acpi/processor/<...>/info.
This didn't get many comments on the linux-acpi list, so I'm sending to a wider distribution.
I notice that acpiphp exposes _SUN for (hotpluggable) PCI slots in /sys/bus/pci/slots/N/, where N is the value of _SUN. I'm not so sure we want to go *exactly* in the same direction for CPUs, because:
- x86 systems might not implement _SUN for processors
- ia64 systems that implement ACPI 2.x do not have unique values for _SUN across the entire system
Nonetheless, even non-unique values of _SUN are still helpful for userspace to figure out which physical socket a CPU might be in, especially when combined with information from /proc/cpuinfo.

Quoted 5 lines

Quoted 1 line
Thoughts?
We've not allowed new features in /proc/acpi since we started removing /proc/acpi.
ie. we don't want to update the API, we want to delete it.

Ok.

Quoted 3 lines

If this is a useful thing for user-space to know, then you need to figure out a generic way to present it -- likely under /sys/devices/system/cpu/

There are lots of ways to go with this, and I'd like to get an idea of what you think makes most sense before going off and spending tons of time here.

I see possible choices as:

1) create something like:

/sys/devices/system/cpu/cpuN/topology/socket_id

And leave it up to the arch to figure if/how they want to implement it. On ia64 systems, we can get this information from the _SUN method.

2) create something like:

/sys/devices/system/cpu/cpuN/acpi/

And port over the stuff from /proc/acpi, adding in a new field for _SUN.

Thoughts?

Thanks.

/ac