cputopology: Add default CPU topology information [3rd try]
Subsystems (3)
PATCH 1/1 cputopology: Add default CPU topology information [3rd try]
Define the macros topology_{physical_package,core}_id() and topology_{thread,core}_siblings() in <asm-generic/topology.h> if they are not already defined.
Move inclusion of <asm-generic/topology.h> after definitions of these macros in <asm-powerpc/topology.h> and <asm-x86/topology.h>.
Change drivers/base/topology.c to use the topology macros unconditionally and to cope with definitions that aren't lvalues.
Update documentation accordingly.
--- Documentation/cputopology.txt | 26 +++++++++----------------- drivers/base/topology.c | 38 ++++++++++---------------------------- include/asm-generic/topology.h | 15 +++++++++++++++ include/asm-powerpc/topology.h | 4 ++-- include/asm-x86/topology.h | 4 ++-- 5 files changed, 38 insertions(+), 49 deletions(-)
Documentation/cputopology.txt +9 -17
@@ -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: +If one architecture wants 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/asm-generic/topology.h +provides default definitions for any of the above macros that are +not already defined: +1) physical_package_id: -1 +2) core_id: 0 +3) thread_siblings: just the given CPU +4) core_siblings: just the given CPU
drivers/base/topology.c +10 -28
@@ -59,60 +59,42 @@ 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; \ - 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 inline 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); \ }#define define_siblings_show_func (name) \ define_siblings_show_map(name); define_siblings_show_list(name) -#ifdef topology_physical_package_iddefine_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_iddefine_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_siblingsdefine_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_siblingsdefine_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 };
include/asm-generic/topology.h +15 -0
@@ -27,6 +27,8 @@#ifndef _ASM_GENERIC_TOPOLOGY_H#define _ASM_GENERIC_TOPOLOGY_H +#include <linux/cpumask.h> +#ifndef CONFIG_NUMA /* Other architectures wishing to use this simple topology API should fill
@@ -66,4 +68,17 @@ _##v = node_to_cpumask(node)#endif +#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 /* _ASM_GENERIC_TOPOLOGY_H */
include/asm-powerpc/topology.h +2 -2
@@ -98,8 +98,6 @@ static inline void sysfs_remove_device_from_node(struct sys_device *dev,#endif /* CONFIG_NUMA */ -#include <asm-generic/topology.h> -#ifdef CONFIG_SMP #include <asm/cputable.h> #define smt_capable () (cpu_has_feature(CPU_FTR_SMT))
@@ -111,5 +109,7 @@ static inline void sysfs_remove_device_from_node(struct sys_device *dev,#endif #endif +#include <asm-generic/topology.h> +#endif /* __KERNEL__ */ #endif /* _ASM_POWERPC_TOPOLOGY_H */
include/asm-x86/topology.h +2 -2
@@ -184,8 +184,6 @@ extern int __node_distance(int, int);#endif -#include <asm-generic/topology.h> -extern cpumask_t cpu_coregroup_map (int cpu);#ifdef ENABLE_TOPO_DEFINES
@@ -220,4 +218,6 @@ static inline void set_mp_bus_to_node(int busnum, int node) }#endif +#include <asm-generic/topology.h> +#endif
Signature · 2 lines
Ben Hutchings, Senior Software Engineer, Solarflare Communications Not speaking for my employer; that's the marketing department's job.
Quoted 6 lines · Ben Hutchings
Define the macros topology_{physical_package,core}_id() and topology_{thread,core}_siblings() in <
asm-generic/topology.h> if they are not already defined.Move inclusion of <
asm-generic/topology.h> after definitions of these macros in <asm-powerpc/topology.h> and <asm-x86/topology.h>.
Hi again :)
As I said in my previous e-mail, having #includes in the middle of headers is nasty. This kind of dependency is really subtle and makes later modification much harder. (Actually, it hurts readability as well.)
The standard way to do this seems to be:
asm/topology.h should define ARCH_HAS_* macros if it wishes to override the defaultslinux/topology.h should #include asm/topology.h at the top of the file linux/topology.h should define the generic functions/macros only if the ARCH_HAS_* macros are undefined
Other files wishing to use these definitions should then include linux/topology.h.
Or is that unfeasible in this case?
Vegard
Signature · 4 lines
"The animistic metaphor of the bug that maliciously sneaked in while the programmer was not looking is intellectually dishonest as it disguises that the error is the programmer's own creation." -- E. W. Dijkstra, EWD1036
Quoted 9 lines · Vegard Nossum
Quoted 6 lines · Ben Hutchings
Define the macros topology_{physical_package,core}_id() and topology_{thread,core}_siblings() in <
asm-generic/topology.h> if they are not already defined.Move inclusion of <
asm-generic/topology.h> after definitions of these macros in <asm-powerpc/topology.h> and <asm-x86/topology.h>.
Quoted 6 lines
Hi again :)
As I said in my previous e-mail, having #includes in the middle of headers is nasty. This kind of dependency is really subtle and makes later modification much harder. (Actually, it hurts readability as well.)
It is a bit nasty, but it seems to be reasonably common to include <asm-generic/foo.h> toward the end of <asm-bar/foo.h>. So I think anyone working with an asm header should expect that.
Quoted 8 lines
The standard way to do this seems to be:
asm/topology.hshould define ARCH_HAS_* macros if it wishes to override the defaultslinux/topology.hshould #includeasm/topology.hat the top of the filelinux/topology.hshould define the generic functions/macros only if the ARCH_HAS_* macros are undefined
Quoted 4 lines
Other files wishing to use these definitions should then include
linux/topology.h.Or is that unfeasible in this case?
It seem feasible, though there is no need for ARCH_HAS_* macros given that the features being described are themselves macros. I would be happy to move the defaults to <linux/topology.h>. Actually, everything in <asm-generic/topology.h> could probably be moved to <linux/topology.h>.
Ben.
Signature · 2 lines
Ben Hutchings, Senior Software Engineer, Solarflare Communications Not speaking for my employer; that's the marketing department's job.
Quoted 9 lines · On Sat, 31 May 2008 22:44:30 +0100 Ben Hutchings <[email protected]>…
Define the macros topology_{physical_package,core}_id() and topology_{thread,core}_siblings() in <
asm-generic/topology.h> if they are not already defined.Move inclusion of <
asm-generic/topology.h> after definitions of these macros in <asm-powerpc/topology.h> and <asm-x86/topology.h>.Change
drivers/base/topology.cto use the topology macros unconditionally and to cope with definitions that aren't lvalues.
Can you please redo this against linux-next?
That tree contains a number of directly conflicting changes. In particular, linux-next introduces alternate implementations of show_##name and show_##name##_list in topology.c and from a quick peek, your change could make them go away again.
Please also enhance the changelog so we have some idea of why you're actually doing all this. What do we gain? Does it fix a bug? If so, what? etc.
Thanks.
Quoted 7 lines · Andrew Morton
Quoted 4 lines · On Sat, 31 May 2008 22:44:30 +0100 Ben Hutchings <[email protected]>…
Define the macros topology_{physical_package,core}_id() and topology_{thread,core}_siblings() in <
asm-generic/topology.h> if they are not already defined.
Quoted 7 lines
Quoted 5 lines
Move inclusion of <
asm-generic/topology.h> after definitions of these macros in <asm-powerpc/topology.h> and <asm-x86/topology.h>.Change
drivers/base/topology.cto use the topology macros unconditionally and to cope with definitions that aren't lvalues.Can you please redo this against linux-next?
Will do.
Quoted 4 lines
That tree contains a number of directly conflicting changes. In particular, linux-next introduces alternate implementations of show_##name and show_##name##_list in
topology.cand from a quick peek, your change could make them go away again.
I don't see what you're referring to. The only things I'm removing are the attribute list macros, which are unnecessary if the list of attributes is constant.
Quoted 3 lines
Please also enhance the changelog so we have some idea of why you're actually doing all this. What do we gain? Does it fix a bug? If so, what? etc.
I'll go back to the explanatory text from my first version.
Ben.
Signature · 2 lines
Ben Hutchings, Senior Software Engineer, Solarflare Communications Not speaking for my employer; that's the marketing department's job.