2021-11-28 21:24:44

by Heiko Carstens

[permalink] [raw]
Subject: [PATCH 2/3] topology/sysfs: export cluster attributes only if an architectures has support

The cluster_id and cluster_cpus topology sysfs attributes have been
added with c5e22feffdd7 ("topology: Represent clusters of CPUs within
a die").

They are currently only used for x86, arm64, and riscv (via generic
arch topology), however they are still present with bogus default
values for all other architectures. Instead of enforcing such new
sysfs attributes to all architectures, make them only optional visible
if an architecture opts in by defining both the topology_cluster_id
and topology_cluster_cpumask attributes.

This is similar to what was done when the book and drawer topology
levels were introduced: avoid useless and therefore confusing sysfs
attributes for architectures which cannot make use of them.

This should not break any existing applications, since this is a
new interface introduced with the v5.16 merge window.

Signed-off-by: Heiko Carstens <[email protected]>
---
Documentation/admin-guide/cputopology.rst | 4 ++--
drivers/base/topology.c | 8 ++++++++
include/linux/topology.h | 2 ++
3 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/Documentation/admin-guide/cputopology.rst b/Documentation/admin-guide/cputopology.rst
index c68d07533c45..ad2238b41439 100644
--- a/Documentation/admin-guide/cputopology.rst
+++ b/Documentation/admin-guide/cputopology.rst
@@ -11,8 +11,8 @@ Architecture-neutral, drivers/base/topology.c, exports these attributes.
However, the book and drawer related sysfs files will only be created if
CONFIG_SCHED_BOOK and CONFIG_SCHED_DRAWER are selected, respectively.

-The die hierarchy related sysfs files will only be created if an architecture
-provides the related macros as described below.
+The die and cluster hierarchy related sysfs files will only be created if an
+architecture provides the related macros as described below.

CONFIG_SCHED_BOOK and CONFIG_SCHED_DRAWER are currently only used on s390,
where they reflect the cpu and cache hierarchy.
diff --git a/drivers/base/topology.c b/drivers/base/topology.c
index 7fa5a7fbef25..1a288eac2c44 100644
--- a/drivers/base/topology.c
+++ b/drivers/base/topology.c
@@ -50,8 +50,10 @@ define_id_show_func(die_id);
static DEVICE_ATTR_RO(die_id);
#endif

+#ifndef topology_cluster_no_sysfs
define_id_show_func(cluster_id);
static DEVICE_ATTR_RO(cluster_id);
+#endif

define_id_show_func(core_id);
static DEVICE_ATTR_RO(core_id);
@@ -68,9 +70,11 @@ define_siblings_read_func(core_siblings, core_cpumask);
static BIN_ATTR_RO(core_siblings, 0);
static BIN_ATTR_RO(core_siblings_list, 0);

+#ifndef topology_cluster_no_sysfs
define_siblings_read_func(cluster_cpus, cluster_cpumask);
static BIN_ATTR_RO(cluster_cpus, 0);
static BIN_ATTR_RO(cluster_cpus_list, 0);
+#endif

#ifndef topology_die_no_sysfs
define_siblings_read_func(die_cpus, die_cpumask);
@@ -105,8 +109,10 @@ static struct bin_attribute *bin_attrs[] = {
&bin_attr_thread_siblings_list,
&bin_attr_core_siblings,
&bin_attr_core_siblings_list,
+#ifndef topology_cluster_no_sysfs
&bin_attr_cluster_cpus,
&bin_attr_cluster_cpus_list,
+#endif
#ifndef topology_die_no_sysfs
&bin_attr_die_cpus,
&bin_attr_die_cpus_list,
@@ -129,7 +135,9 @@ static struct attribute *default_attrs[] = {
#ifndef topology_die_no_sysfs
&dev_attr_die_id.attr,
#endif
+#ifndef topology_cluster_no_sysfs
&dev_attr_cluster_id.attr,
+#endif
&dev_attr_core_id.attr,
#ifdef CONFIG_SCHED_BOOK
&dev_attr_book_id.attr,
diff --git a/include/linux/topology.h b/include/linux/topology.h
index 1541d210be24..cf57413f7c28 100644
--- a/include/linux/topology.h
+++ b/include/linux/topology.h
@@ -189,6 +189,7 @@ static inline int cpu_to_mem(int cpu)
#endif
#ifndef topology_cluster_id
#define topology_cluster_id(cpu) ((void)(cpu), -1)
+#define topology_cluster_no_sysfs 1
#endif
#ifndef topology_core_id
#define topology_core_id(cpu) ((void)(cpu), 0)
@@ -201,6 +202,7 @@ static inline int cpu_to_mem(int cpu)
#endif
#ifndef topology_cluster_cpumask
#define topology_cluster_cpumask(cpu) cpumask_of(cpu)
+#define topology_cluster_no_sysfs 1
#endif
#ifndef topology_die_cpumask
#define topology_die_cpumask(cpu) cpumask_of(cpu)
--
2.32.0



2021-11-29 06:56:32

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 2/3] topology/sysfs: export cluster attributes only if an architectures has support

On Sun, Nov 28, 2021 at 10:22:20PM +0100, Heiko Carstens wrote:
> The cluster_id and cluster_cpus topology sysfs attributes have been
> added with c5e22feffdd7 ("topology: Represent clusters of CPUs within
> a die").
>
> They are currently only used for x86, arm64, and riscv (via generic
> arch topology), however they are still present with bogus default
> values for all other architectures. Instead of enforcing such new
> sysfs attributes to all architectures, make them only optional visible
> if an architecture opts in by defining both the topology_cluster_id
> and topology_cluster_cpumask attributes.
>
> This is similar to what was done when the book and drawer topology
> levels were introduced: avoid useless and therefore confusing sysfs
> attributes for architectures which cannot make use of them.
>
> This should not break any existing applications, since this is a
> new interface introduced with the v5.16 merge window.
>
> Signed-off-by: Heiko Carstens <[email protected]>
> ---
> Documentation/admin-guide/cputopology.rst | 4 ++--
> drivers/base/topology.c | 8 ++++++++
> include/linux/topology.h | 2 ++
> 3 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/admin-guide/cputopology.rst b/Documentation/admin-guide/cputopology.rst
> index c68d07533c45..ad2238b41439 100644
> --- a/Documentation/admin-guide/cputopology.rst
> +++ b/Documentation/admin-guide/cputopology.rst
> @@ -11,8 +11,8 @@ Architecture-neutral, drivers/base/topology.c, exports these attributes.
> However, the book and drawer related sysfs files will only be created if
> CONFIG_SCHED_BOOK and CONFIG_SCHED_DRAWER are selected, respectively.
>
> -The die hierarchy related sysfs files will only be created if an architecture
> -provides the related macros as described below.
> +The die and cluster hierarchy related sysfs files will only be created if an
> +architecture provides the related macros as described below.
>
> CONFIG_SCHED_BOOK and CONFIG_SCHED_DRAWER are currently only used on s390,
> where they reflect the cpu and cache hierarchy.
> diff --git a/drivers/base/topology.c b/drivers/base/topology.c
> index 7fa5a7fbef25..1a288eac2c44 100644
> --- a/drivers/base/topology.c
> +++ b/drivers/base/topology.c
> @@ -50,8 +50,10 @@ define_id_show_func(die_id);
> static DEVICE_ATTR_RO(die_id);
> #endif
>
> +#ifndef topology_cluster_no_sysfs

Same comment here, do not enable an option that disables something :)

thanks,

greg k-h