[PATCH] cputopology: Always define CPU topology information [4th try]

[PATCH] cputopology: Always define CPU topology information [4th try]

[Ben Hutchings]

Not all architectures and configurations define CPU topology information.
This can result in an empty topology directory in sysfs, and requires
in-kernel users to protect all uses with #ifdef - see
<http://marc.info/?l=linux-netdev&m=120639033904472&w=2>.

The documentation of CPU topology specifies what the defaults should be
if only partial information is available from the hardware.  So we can
provide these defaults as a fallback.

This patch:

- Adds default definitions of the 4 topology macros to <linux/topology.h>
- Changes drivers/base/topology.c to use the topology macros unconditionally
  and to cope with definitions that aren't lvalues
- Updates documentation accordingly

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
 Documentation/cputopology.txt |   26 +++++++++-----------------
 drivers/base/topology.c       |   38 ++++++++++----------------------------
 include/linux/topology.h      |   13 +++++++++++++
 3 files changed, 32 insertions(+), 45 deletions(-)

diff --git a/Documentation/cputopology.txt b/Documentation/cputopology.txt
index b61cb95..bd699da 100644
--- a/Documentation/cputopology.txt
+++ b/Documentation/cputopology.txt
@@ -14,9 +14,8 @@ represent the thread siblings to cpu X in the same physical package;
 To implement it in an architecture-neutral way, a new source file,
 drivers/base/topology.c, is to export the 4 attributes.
 
-If one architecture wants to support this feature, it just needs to
-implement 4 defines, typically in file include/asm-XXX/topology.h.
-The 4 defines are:
+For an architecture to support this feature, it must define some of
+these macros in include/asm-XXX/topology.h:
 #define topology_physical_package_id(cpu)
 #define topology_core_id(cpu)
 #define topology_thread_siblings(cpu)
@@ -25,17 +24,10 @@ The 4 defines are:
 The type of **_id is int.
 The type of siblings is cpumask_t.
 
-To be consistent on all architectures, the 4 attributes should have
-default values if their values are unavailable. Below is the rule.
-1) physical_package_id: If cpu has no physical package id, -1 is the
-default value.
-2) core_id: If cpu doesn't support multi-core, its core id is 0.
-3) thread_siblings: Just include itself, if the cpu doesn't support
-HT/multi-thread.
-4) core_siblings: Just include itself, if the cpu doesn't support
-multi-core and HT/Multi-thread.
-
-So be careful when declaring the 4 defines in include/asm-XXX/topology.h.
-
-If an attribute isn't defined on an architecture, it won't be exported.
-
+To be consistent on all architectures, include/linux/topology.h
+provides default definitions for any of the above macros that are
+not defined by include/asm-XXX/topology.h:
+1) physical_package_id: -1
+2) core_id: 0
+3) thread_siblings: just the given CPU
+4) core_siblings: just the given CPU
diff --git a/drivers/base/topology.c b/drivers/base/topology.c
index 1efe162..8999828 100644
--- a/drivers/base/topology.c
+++ b/drivers/base/topology.c
@@ -62,14 +62,16 @@ static ssize_t show_cpumap(int type, cpumask_t *mask, char *buf)
 static ssize_t show_##name(struct sys_device *dev, char *buf)	\
 {									\
 	unsigned int cpu = dev->id;					\
-	return show_cpumap(0, &(topology_##name(cpu)), buf);		\
+	cpumask_t siblings = topology_##name(cpu);			\
+	return show_cpumap(0, &siblings, buf);				\
 }
 
 #define define_siblings_show_list(name)					\
 static ssize_t show_##name##_list(struct sys_device *dev, char *buf) \
 {									\
 	unsigned int cpu = dev->id;					\
-	return show_cpumap(1, &(topology_##name(cpu)), buf);		\
+	cpumask_t siblings = topology_##name(cpu);			\
+	return show_cpumap(1, &siblings, buf);				\
 }
 
 #else
@@ -93,47 +95,27 @@ static ssize_t show_##name##_list(struct sys_device *dev, char *buf) \
 #define define_siblings_show_func(name)		\
 	define_siblings_show_map(name); define_siblings_show_list(name)
 
-#ifdef	topology_physical_package_id
 define_id_show_func(physical_package_id);
 define_one_ro(physical_package_id);
-#define ref_physical_package_id_attr	&attr_physical_package_id.attr,
-#else
-#define ref_physical_package_id_attr
-#endif
 
-#ifdef topology_core_id
 define_id_show_func(core_id);
 define_one_ro(core_id);
-#define ref_core_id_attr		&attr_core_id.attr,
-#else
-#define ref_core_id_attr
-#endif
 
-#ifdef topology_thread_siblings
 define_siblings_show_func(thread_siblings);
 define_one_ro(thread_siblings);
 define_one_ro(thread_siblings_list);
-#define ref_thread_siblings_attr	\
-		&attr_thread_siblings.attr, &attr_thread_siblings_list.attr,
-#else
-#define ref_thread_siblings_attr
-#endif
 
-#ifdef topology_core_siblings
 define_siblings_show_func(core_siblings);
 define_one_ro(core_siblings);
 define_one_ro(core_siblings_list);
-#define ref_core_siblings_attr		\
-		&attr_core_siblings.attr, &attr_core_siblings_list.attr,
-#else
-#define ref_core_siblings_attr
-#endif
 
 static struct attribute *default_attrs[] = {
-	ref_physical_package_id_attr
-	ref_core_id_attr
-	ref_thread_siblings_attr
-	ref_core_siblings_attr
+	&attr_physical_package_id.attr,
+	&attr_core_id.attr,
+	&attr_thread_siblings.attr,
+	&attr_thread_siblings_list.attr,
+	&attr_core_siblings.attr,
+	&attr_core_siblings_list.attr,
 	NULL
 };
 
diff --git a/include/linux/topology.h b/include/linux/topology.h
index 24f3d22..2158fc0 100644
--- a/include/linux/topology.h
+++ b/include/linux/topology.h
@@ -179,4 +179,17 @@ void arch_update_cpu_topology(void);
 #endif
 #endif /* CONFIG_NUMA */
 
+#ifndef topology_physical_package_id
+#define topology_physical_package_id(cpu)	((void)(cpu), -1)
+#endif
+#ifndef topology_core_id
+#define topology_core_id(cpu)			((void)(cpu), 0)
+#endif
+#ifndef topology_thread_siblings
+#define topology_thread_siblings(cpu)		cpumask_of_cpu(cpu)
+#endif
+#ifndef topology_core_siblings
+#define topology_core_siblings(cpu)		cpumask_of_cpu(cpu)
+#endif
+
 #endif /* _LINUX_TOPOLOGY_H */

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.

Re: [PATCH] cputopology: Always define CPU topology information [4th try]

[Andrew Morton]

On Wed, 4 Jun 2008 16:44:56 +0100 Ben Hutchings <bhutchings@solarflare.com> wrote:

> Not all architectures and configurations define CPU topology information.
> This can result in an empty topology directory in sysfs, and requires
> in-kernel users to protect all uses with #ifdef - see
> <http://marc.info/?l=linux-netdev&m=120639033904472&w=2>.
> 
> The documentation of CPU topology specifies what the defaults should be
> if only partial information is available from the hardware.  So we can
> provide these defaults as a fallback.
> 
> This patch:
> 
> - Adds default definitions of the 4 topology macros to <linux/topology.h>
> - Changes drivers/base/topology.c to use the topology macros unconditionally
>   and to cope with definitions that aren't lvalues
> - Updates documentation accordingly

See, this is what I meant.  After your patch we have:

#ifdef arch_provides_topology_pointers
#define define_siblings_show_map(name)					\
static ssize_t show_##name(struct sys_device *dev, char *buf)	\
{									\
	unsigned int cpu = dev->id;					\
	cpumask_t siblings = topology_##name(cpu);			\
	return show_cpumap(0, &siblings, buf);				\
}

#define define_siblings_show_list(name)					\
static ssize_t show_##name##_list(struct sys_device *dev, char *buf) \
{									\
	unsigned int cpu = dev->id;					\
	cpumask_t siblings = topology_##name(cpu);			\
	return show_cpumap(1, &siblings, buf);				\
}

#else
#define define_siblings_show_map(name)					\
static ssize_t show_##name(struct sys_device *dev, char *buf)	\
{									\
	unsigned int cpu = dev->id;					\
	cpumask_t mask = topology_##name(cpu);				\
	return show_cpumap(0, &mask, buf);				\
}

#define define_siblings_show_list(name)					\
static ssize_t show_##name##_list(struct sys_device *dev, char *buf) \
{									\
	unsigned int cpu = dev->id;					\
	cpumask_t mask = topology_##name(cpu);				\
	return show_cpumap(1, &mask, buf);				\
}
#endif

they're the same!

Re: [PATCH] cputopology: Always define CPU topology information [4th try]

[Ben Hutchings]

Andrew Morton wrote:
> On Wed, 4 Jun 2008 16:44:56 +0100 Ben Hutchings <bhutchings@solarflare.com> wrote:
> 
> > Not all architectures and configurations define CPU topology information.
> > This can result in an empty topology directory in sysfs, and requires
> > in-kernel users to protect all uses with #ifdef - see
> > <http://marc.info/?l=linux-netdev&m=120639033904472&w=2>.
> > 
> > The documentation of CPU topology specifies what the defaults should be
> > if only partial information is available from the hardware.  So we can
> > provide these defaults as a fallback.
> > 
> > This patch:
> > 
> > - Adds default definitions of the 4 topology macros to <linux/topology.h>
> > - Changes drivers/base/topology.c to use the topology macros unconditionally
> >   and to cope with definitions that aren't lvalues
> > - Updates documentation accordingly
> 
> See, this is what I meant.  After your patch we have:
[...]

Sorry, I don't know how that escaped me.  My changes to the show functions
should be unnecessary - though I think that the two different implementations
for lvalues and rvalues are a premature optimisation.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.

Re: [PATCH] cputopology: Always define CPU topology information [4th try]

[Andrew Morton]

On Thu, 5 Jun 2008 13:08:30 +0100 Ben Hutchings <bhutchings@solarflare.com> wrote:

> Andrew Morton wrote:
> > On Wed, 4 Jun 2008 16:44:56 +0100 Ben Hutchings <bhutchings@solarflare.com> wrote:
> > 
> > > Not all architectures and configurations define CPU topology information.
> > > This can result in an empty topology directory in sysfs, and requires
> > > in-kernel users to protect all uses with #ifdef - see
> > > <http://marc.info/?l=linux-netdev&m=120639033904472&w=2>.
> > > 
> > > The documentation of CPU topology specifies what the defaults should be
> > > if only partial information is available from the hardware.  So we can
> > > provide these defaults as a fallback.
> > > 
> > > This patch:
> > > 
> > > - Adds default definitions of the 4 topology macros to <linux/topology.h>
> > > - Changes drivers/base/topology.c to use the topology macros unconditionally
> > >   and to cope with definitions that aren't lvalues
> > > - Updates documentation accordingly
> > 
> > See, this is what I meant.  After your patch we have:
> [...]
> 
> Sorry, I don't know how that escaped me.  My changes to the show functions
> should be unnecessary - though I think that the two different implementations
> for lvalues and rvalues are a premature optimisation.

um, what does this mean.  Will you be sending an updated patch or should
I drop those two or...?

[PATCH] cputopology: Always define CPU topology information [5th try]

[Ben Hutchings]

Not all architectures and configurations define CPU topology information.
This can result in an empty topology directory in sysfs, and requires
in-kernel users to protect all uses with #ifdef - see
<http://marc.info/?l=linux-netdev&m=120639033904472&w=2>.

The documentation of CPU topology specifies what the defaults should be
if only partial information is available from the hardware.  So we can
provide these defaults as a fallback.

This patch:

- Adds default definitions of the 4 topology macros to <linux/topology.h>
- Changes drivers/base/topology.c to use the topology macros unconditionally
- Updates documentation accordingly

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
 Documentation/cputopology.txt |   26 +++++++++-----------------
 drivers/base/topology.c       |   32 ++++++--------------------------
 include/linux/topology.h      |   13 +++++++++++++
 3 files changed, 28 insertions(+), 43 deletions(-)

diff --git a/Documentation/cputopology.txt b/Documentation/cputopology.txt
index b61cb95..bd699da 100644
--- a/Documentation/cputopology.txt
+++ b/Documentation/cputopology.txt
@@ -14,9 +14,8 @@ represent the thread siblings to cpu X in the same physical package;
 To implement it in an architecture-neutral way, a new source file,
 drivers/base/topology.c, is to export the 4 attributes.
 
-If one architecture wants to support this feature, it just needs to
-implement 4 defines, typically in file include/asm-XXX/topology.h.
-The 4 defines are:
+For an architecture to support this feature, it must define some of
+these macros in include/asm-XXX/topology.h:
 #define topology_physical_package_id(cpu)
 #define topology_core_id(cpu)
 #define topology_thread_siblings(cpu)
@@ -25,17 +24,10 @@ The 4 defines are:
 The type of **_id is int.
 The type of siblings is cpumask_t.
 
-To be consistent on all architectures, the 4 attributes should have
-default values if their values are unavailable. Below is the rule.
-1) physical_package_id: If cpu has no physical package id, -1 is the
-default value.
-2) core_id: If cpu doesn't support multi-core, its core id is 0.
-3) thread_siblings: Just include itself, if the cpu doesn't support
-HT/multi-thread.
-4) core_siblings: Just include itself, if the cpu doesn't support
-multi-core and HT/Multi-thread.
-
-So be careful when declaring the 4 defines in include/asm-XXX/topology.h.
-
-If an attribute isn't defined on an architecture, it won't be exported.
-
+To be consistent on all architectures, include/linux/topology.h
+provides default definitions for any of the above macros that are
+not defined by include/asm-XXX/topology.h:
+1) physical_package_id: -1
+2) core_id: 0
+3) thread_siblings: just the given CPU
+4) core_siblings: just the given CPU
diff --git a/drivers/base/topology.c b/drivers/base/topology.c
index 1efe162..3f6d9b0 100644
--- a/drivers/base/topology.c
+++ b/drivers/base/topology.c
@@ -93,47 +93,27 @@ static ssize_t show_##name##_list(struct sys_device *dev, char *buf) \
 #define define_siblings_show_func(name)		\
 	define_siblings_show_map(name); define_siblings_show_list(name)
 
-#ifdef	topology_physical_package_id
 define_id_show_func(physical_package_id);
 define_one_ro(physical_package_id);
-#define ref_physical_package_id_attr	&attr_physical_package_id.attr,
-#else
-#define ref_physical_package_id_attr
-#endif
 
-#ifdef topology_core_id
 define_id_show_func(core_id);
 define_one_ro(core_id);
-#define ref_core_id_attr		&attr_core_id.attr,
-#else
-#define ref_core_id_attr
-#endif
 
-#ifdef topology_thread_siblings
 define_siblings_show_func(thread_siblings);
 define_one_ro(thread_siblings);
 define_one_ro(thread_siblings_list);
-#define ref_thread_siblings_attr	\
-		&attr_thread_siblings.attr, &attr_thread_siblings_list.attr,
-#else
-#define ref_thread_siblings_attr
-#endif
 
-#ifdef topology_core_siblings
 define_siblings_show_func(core_siblings);
 define_one_ro(core_siblings);
 define_one_ro(core_siblings_list);
-#define ref_core_siblings_attr		\
-		&attr_core_siblings.attr, &attr_core_siblings_list.attr,
-#else
-#define ref_core_siblings_attr
-#endif
 
 static struct attribute *default_attrs[] = {
-	ref_physical_package_id_attr
-	ref_core_id_attr
-	ref_thread_siblings_attr
-	ref_core_siblings_attr
+	&attr_physical_package_id.attr,
+	&attr_core_id.attr,
+	&attr_thread_siblings.attr,
+	&attr_thread_siblings_list.attr,
+	&attr_core_siblings.attr,
+	&attr_core_siblings_list.attr,
 	NULL
 };
 
diff --git a/include/linux/topology.h b/include/linux/topology.h
index 24f3d22..2158fc0 100644
--- a/include/linux/topology.h
+++ b/include/linux/topology.h
@@ -179,4 +179,17 @@ void arch_update_cpu_topology(void);
 #endif
 #endif /* CONFIG_NUMA */
 
+#ifndef topology_physical_package_id
+#define topology_physical_package_id(cpu)	((void)(cpu), -1)
+#endif
+#ifndef topology_core_id
+#define topology_core_id(cpu)			((void)(cpu), 0)
+#endif
+#ifndef topology_thread_siblings
+#define topology_thread_siblings(cpu)		cpumask_of_cpu(cpu)
+#endif
+#ifndef topology_core_siblings
+#define topology_core_siblings(cpu)		cpumask_of_cpu(cpu)
+#endif
+
 #endif /* _LINUX_TOPOLOGY_H */

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.

Re: [PATCH] cputopology: Always define CPU topology information [5th try]

[Ingo Molnar]

* Ben Hutchings <bhutchings@solarflare.com> wrote:

> Not all architectures and configurations define CPU topology 
> information. This can result in an empty topology directory in sysfs, 
> and requires in-kernel users to protect all uses with #ifdef - see 
> <http://marc.info/?l=linux-netdev&m=120639033904472&w=2>.
> 
> The documentation of CPU topology specifies what the defaults should 
> be if only partial information is available from the hardware.  So we 
> can provide these defaults as a fallback.
> 
> This patch:
> 
> - Adds default definitions of the 4 topology macros to <linux/topology.h>
> - Changes drivers/base/topology.c to use the topology macros unconditionally
> - Updates documentation accordingly

applied to tip/core/topology - thanks Ben.

	Ingo

Re: [PATCH] cputopology: Always define CPU topology information [5th try]

[Ben Hutchings]

Ingo Molnar wrote:
> 
> * Ben Hutchings <bhutchings@solarflare.com> wrote:
> 
> > Not all architectures and configurations define CPU topology 
> > information. This can result in an empty topology directory in sysfs, 
> > and requires in-kernel users to protect all uses with #ifdef - see 
> > <http://marc.info/?l=linux-netdev&m=120639033904472&w=2>.
> > 
> > The documentation of CPU topology specifies what the defaults should 
> > be if only partial information is available from the hardware.  So we 
> > can provide these defaults as a fallback.
> > 
> > This patch:
> > 
> > - Adds default definitions of the 4 topology macros to <linux/topology.h>
> > - Changes drivers/base/topology.c to use the topology macros unconditionally
> > - Updates documentation accordingly
> 
> applied to tip/core/topology - thanks Ben.

Unfortunately you lost the first paragraph of the commit message, then
created a second commit (difference between 4th and 5th versions) with
your own commit message and me as the author.  Maybe it's nitpicking but I
would prefer to have strictly accurate metadata in the commit log.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.

Re: [PATCH] cputopology: Always define CPU topology information [5th try]

[Ingo Molnar]

* Ben Hutchings <bhutchings@solarflare.com> wrote:

> > > This patch:
> > > 
> > > - Adds default definitions of the 4 topology macros to <linux/topology.h>
> > > - Changes drivers/base/topology.c to use the topology macros unconditionally
> > > - Updates documentation accordingly
> > 
> > applied to tip/core/topology - thanks Ben.
> 
> Unfortunately you lost the first paragraph of the commit message, then 
> created a second commit (difference between 4th and 5th versions) with 
> your own commit message and me as the author.  Maybe it's nitpicking 
> but I would prefer to have strictly accurate metadata in the commit 
> log.

that's OK, see the delta patch below, it's rather trivial. It's better 
to do these small delta patches where each change stands on its own than 
to review a more complex combo patch. (Suggest me any other commit 
metadata for this delta if you prefer, we can still change it.)

	Ingo

-------------->
commit 131b943ae643b1ad6febd67cdbb31d955706ecf4
Author: Ben Hutchings <bhutchings@solarflare.com>
Date:   Thu Jun 5 17:37:15 2008 +0100

    cputopology: always define CPU topology information, clean up
    
    simplify drivers/base/topology.c a bit.
    
    Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
    Signed-off-by: Ingo Molnar <mingo@elte.hu>

diff --git a/drivers/base/topology.c b/drivers/base/topology.c
index 24d29a9..f0cb270 100644
--- a/drivers/base/topology.c
+++ b/drivers/base/topology.c
@@ -59,16 +59,14 @@ static ssize_t show_cpumap(int type, cpumask_t *mask, char *buf)
 static inline ssize_t show_##name(struct sys_device *dev, char *buf)	\
 {									\
 	unsigned int cpu = dev->id;					\
-	cpumask_t siblings = topology_##name(cpu);			\
-	return show_cpumap(0, &siblings, buf);				\
+	return show_cpumap(0, &(topology_##name(cpu)), buf);		\
 }
 
 #define define_siblings_show_list(name)					\
 static inline ssize_t show_##name##_list(struct sys_device *dev, char *buf) \
 {									\
 	unsigned int cpu = dev->id;					\
-	cpumask_t siblings = topology_##name(cpu);			\
-	return show_cpumap(1, &siblings, buf);				\
+	return show_cpumap(1, &(topology_##name(cpu)), buf);		\
 }
 
 #define define_siblings_show_func(name)		\

Re: [PATCH] cputopology: Always define CPU topology information [5th try]

[Ben Hutchings]

Ingo Molnar wrote:
> 
> * Ben Hutchings <bhutchings@solarflare.com> wrote:
> 
> > > > This patch:
> > > > 
> > > > - Adds default definitions of the 4 topology macros to <linux/topology.h>
> > > > - Changes drivers/base/topology.c to use the topology macros unconditionally
> > > > - Updates documentation accordingly
> > > 
> > > applied to tip/core/topology - thanks Ben.
> > 
> > Unfortunately you lost the first paragraph of the commit message, then 
> > created a second commit (difference between 4th and 5th versions) with 
> > your own commit message and me as the author.  Maybe it's nitpicking 
> > but I would prefer to have strictly accurate metadata in the commit 
> > log.
> 
> that's OK, see the delta patch below, it's rather trivial. It's better 
> to do these small delta patches where each change stands on its own than 
> to review a more complex combo patch. (Suggest me any other commit 
> metadata for this delta if you prefer, we can still change it.)
[...]
 
The message should be:

Revert first two hunks of "cputopology: Always define CPU topology information"
which were redundant with "x86: cleanup early per cpu variables/accesses v4".

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.

Re: [PATCH] cputopology: Always define CPU topology information [4th try]

[Nathan Lynch]

Hi Ben-

Ben Hutchings wrote:
> Not all architectures and configurations define CPU topology information.
> This can result in an empty topology directory in sysfs, and requires
> in-kernel users to protect all uses with #ifdef - see
> <http://marc.info/?l=linux-netdev&m=120639033904472&w=2>.
> 
> The documentation of CPU topology specifies what the defaults should be
> if only partial information is available from the hardware.  So we can
> provide these defaults as a fallback.

I've been looking at adding topology information to powerpc and I came
across this.

I understand the need for fallback definitions of the topology APIs
within the kernel, but I'm not sure I agree with exposing these things
in sysfs unconditionally -- the default values for physical_package_id
and core_id don't really make sense on powerpc (and other non-x86
architectures, I suspect).

Would you object to a patch which exposes in sysfs only the topology
information which the architecture provides?

Re: [PATCH] cputopology: Always define CPU topology information [4th try]

[Ben Hutchings]

Nathan Lynch wrote:
> Hi Ben-
> 
> Ben Hutchings wrote:
> > Not all architectures and configurations define CPU topology information.
> > This can result in an empty topology directory in sysfs, and requires
> > in-kernel users to protect all uses with #ifdef - see
> > <http://marc.info/?l=linux-netdev&m=120639033904472&w=2>.
> > 
> > The documentation of CPU topology specifies what the defaults should be
> > if only partial information is available from the hardware.  So we can
> > provide these defaults as a fallback.
> 
> I've been looking at adding topology information to powerpc and I came
> across this.
> 
> I understand the need for fallback definitions of the topology APIs
> within the kernel, but I'm not sure I agree with exposing these things
> in sysfs unconditionally -- the default values for physical_package_id
> and core_id don't really make sense on powerpc (and other non-x86
> architectures, I suspect).

In what way are they wrong?

> Would you object to a patch which exposes in sysfs only the topology
> information which the architecture provides?

I was primarily concerned with having the fallbacks available in-kernel.
However, I don't think you will be doing user-space any favours by
requiring checks for missing attributes for ever.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.