2019-06-17 19:01:30

by Atish Patra

[permalink] [raw]
Subject: [PATCH v7 0/7] Unify CPU topology across ARM & RISC-V

The cpu-map DT entry in ARM can describe the CPU topology in much better
way compared to other existing approaches. RISC-V can easily adopt this
binding to represent its own CPU topology. Thus, both cpu-map DT
binding and topology parsing code can be moved to a common location so
that RISC-V or any other architecture can leverage that.

The relevant discussion regarding unifying cpu topology can be found in
[1].

arch_topology seems to be a perfect place to move the common code. I
have not introduced any significant functional changes in the moved code.
The only downside in this approach is that the capacity code will be
executed for RISC-V as well. But, it will exit immediately after not
able to find the appropriate DT node. If the overhead is considered too
much, we can always compile out capacity related functions under a
different config for the architectures that do not support them.

There was an opportunity to unify topology data structure for ARM32 done
by patch 3/4. But, I refrained from making any other changes as I am not
very well versed with original intention for some functions that
are present in arch_topology.c. I hope this patch series can be served
as a baseline for such changes in the future.

The patches have been tested for RISC-V and compile tested for ARM64,
ARM32 & x86.

From Jeremy,

"I applied these to 5.2rc2, along with my PPTT/MT change and verified the
system & scheduler topology/etc on DAWN and ThunderX2 using ACPI on arm64.
They appear to be working correctly.

so for the series,
Tested-by: Jeremy Linton <[email protected]>"

The socket change[2] is also now part of this series.

[1] https://lkml.org/lkml/2018/11/6/19
[2] https://lkml.org/lkml/2018/11/7/918

QEMU changes for RISC-V topology are available at

https://github.com/atishp04/qemu/tree/riscv_topology_dt

HiFive Unleashed DT with topology node is available here.
https://github.com/atishp04/opensbi/tree/HiFive_unleashed_topology

It can be verified with OpenSBI with following additional compile time
option.

FW_PAYLOAD_FDT="unleashed_topology.dtb"

Changes from v6->v7
1. Added socket to HiFive Unleashed topology example.
2. Added Acked-by & Reviewed-by.

Changes from v5->v6
1. Added two more patches from Sudeep about maintainership of arch_topology.c
and Kconfig update.
2. Added Tested-by & Reviewed-by
3. Fixed a nit (reordering of variables)

Changes from v4-v5
1. Removed the arch_topology.h header inclusion from topology.c and arch_topology.c
file. Added it in linux/topology.h.
2. core_id is set to -1 upon reset. Otherwise, ARM topology store function does not
work.

Changes from v3->v4
1. Get rid of ARM32 specific information in topology structure.
2. Remove redundant functions from ARM32 and use common code instead.

Changes from v2->v3
1. Cover letter update with experiment DT for topology changes.
2. Added the patch for [2].

Changes from v1->v2
1. ARM32 can now use the common code as well.

Atish Patra (4):
dt-binding: cpu-topology: Move cpu-map to a common binding.
cpu-topology: Move cpu topology code to common code.
arm: Use common cpu_topology structure and functions.
RISC-V: Parse cpu topology during boot.

Sudeep Holla (3):
Documentation: DT: arm: add support for sockets defining package
boundaries
base: arch_topology: update Kconfig help description
MAINTAINERS: Add an entry for generic architecture topology

.../topology.txt => cpu/cpu-topology.txt} | 136 ++++++--
MAINTAINERS | 7 +
arch/arm/include/asm/topology.h | 20 --
arch/arm/kernel/topology.c | 60 +---
arch/arm64/include/asm/topology.h | 23 --
arch/arm64/kernel/topology.c | 303 +-----------------
arch/riscv/Kconfig | 1 +
arch/riscv/kernel/smpboot.c | 3 +
drivers/base/Kconfig | 2 +-
drivers/base/arch_topology.c | 298 +++++++++++++++++
include/linux/arch_topology.h | 26 ++
include/linux/topology.h | 1 +
12 files changed, 454 insertions(+), 426 deletions(-)
rename Documentation/devicetree/bindings/{arm/topology.txt => cpu/cpu-topology.txt} (66%)

--
2.21.0


2019-06-17 19:01:34

by Atish Patra

[permalink] [raw]
Subject: [PATCH v7 1/7] Documentation: DT: arm: add support for sockets defining package boundaries

From: Sudeep Holla <[email protected]>

The current ARM DT topology description provides the operating system
with a topological view of the system that is based on leaf nodes
representing either cores or threads (in an SMT system) and a
hierarchical set of cluster nodes that creates a hierarchical topology
view of how those cores and threads are grouped.

However this hierarchical representation of clusters does not allow to
describe what topology level actually represents the physical package or
the socket boundary, which is a key piece of information to be used by
an operating system to optimize resource allocation and scheduling.

Lets add a new "socket" node type in the cpu-map node to describe the
same.

Signed-off-by: Sudeep Holla <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
---
.../devicetree/bindings/arm/topology.txt | 52 ++++++++++++++-----
1 file changed, 39 insertions(+), 13 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/topology.txt b/Documentation/devicetree/bindings/arm/topology.txt
index b0d80c0fb265..3b8febb46dad 100644
--- a/Documentation/devicetree/bindings/arm/topology.txt
+++ b/Documentation/devicetree/bindings/arm/topology.txt
@@ -9,6 +9,7 @@ ARM topology binding description
In an ARM system, the hierarchy of CPUs is defined through three entities that
are used to describe the layout of physical CPUs in the system:

+- socket
- cluster
- core
- thread
@@ -63,21 +64,23 @@ nodes are listed.

The cpu-map node's child nodes can be:

- - one or more cluster nodes
+ - one or more cluster nodes or
+ - one or more socket nodes in a multi-socket system

Any other configuration is considered invalid.

-The cpu-map node can only contain three types of child nodes:
+The cpu-map node can only contain 4 types of child nodes:

+- socket node
- cluster node
- core node
- thread node

whose bindings are described in paragraph 3.

-The nodes describing the CPU topology (cluster/core/thread) can only
-be defined within the cpu-map node and every core/thread in the system
-must be defined within the topology. Any other configuration is
+The nodes describing the CPU topology (socket/cluster/core/thread) can
+only be defined within the cpu-map node and every core/thread in the
+system must be defined within the topology. Any other configuration is
invalid and therefore must be ignored.

===========================================
@@ -85,26 +88,44 @@ invalid and therefore must be ignored.
===========================================

cpu-map child nodes must follow a naming convention where the node name
-must be "clusterN", "coreN", "threadN" depending on the node type (ie
-cluster/core/thread) (where N = {0, 1, ...} is the node number; nodes which
-are siblings within a single common parent node must be given a unique and
+must be "socketN", "clusterN", "coreN", "threadN" depending on the node type
+(ie socket/cluster/core/thread) (where N = {0, 1, ...} is the node number; nodes
+which are siblings within a single common parent node must be given a unique and
sequential N value, starting from 0).
cpu-map child nodes which do not share a common parent node can have the same
name (ie same number N as other cpu-map child nodes at different device tree
levels) since name uniqueness will be guaranteed by the device tree hierarchy.

===========================================
-3 - cluster/core/thread node bindings
+3 - socket/cluster/core/thread node bindings
===========================================

-Bindings for cluster/cpu/thread nodes are defined as follows:
+Bindings for socket/cluster/cpu/thread nodes are defined as follows:
+
+- socket node
+
+ Description: must be declared within a cpu-map node, one node
+ per physical socket in the system. A system can
+ contain single or multiple physical socket.
+ The association of sockets and NUMA nodes is beyond
+ the scope of this bindings, please refer [2] for
+ NUMA bindings.
+
+ This node is optional for a single socket system.
+
+ The socket node name must be "socketN" as described in 2.1 above.
+ A socket node can not be a leaf node.
+
+ A socket node's child nodes must be one or more cluster nodes.
+
+ Any other configuration is considered invalid.

- cluster node

Description: must be declared within a cpu-map node, one node
per cluster. A system can contain several layers of
- clustering and cluster nodes can be contained in parent
- cluster nodes.
+ clustering within a single physical socket and cluster
+ nodes can be contained in parent cluster nodes.

The cluster node name must be "clusterN" as described in 2.1 above.
A cluster node can not be a leaf node.
@@ -164,13 +185,15 @@ Bindings for cluster/cpu/thread nodes are defined as follows:
4 - Example dts
===========================================

-Example 1 (ARM 64-bit, 16-cpu system, two clusters of clusters):
+Example 1 (ARM 64-bit, 16-cpu system, two clusters of clusters in a single
+physical socket):

cpus {
#size-cells = <0>;
#address-cells = <2>;

cpu-map {
+ socket0 {
cluster0 {
cluster0 {
core0 {
@@ -253,6 +276,7 @@ cpus {
};
};
};
+ };

CPU0: cpu@0 {
device_type = "cpu";
@@ -473,3 +497,5 @@ cpus {
===============================================================================
[1] ARM Linux kernel documentation
Documentation/devicetree/bindings/arm/cpus.yaml
+[2] Devicetree NUMA binding description
+ Documentation/devicetree/bindings/numa.txt
--
2.21.0

2019-06-17 19:01:42

by Atish Patra

[permalink] [raw]
Subject: [PATCH v7 5/7] RISC-V: Parse cpu topology during boot.

Currently, there are no topology defined for RISC-V.
Parse the cpu-map node from device tree and setup the
cpu topology.

CPU topology after applying the patch.
$cat /sys/devices/system/cpu/cpu2/topology/core_siblings_list
0-3
$cat /sys/devices/system/cpu/cpu3/topology/core_siblings_list
0-3
$cat /sys/devices/system/cpu/cpu3/topology/physical_package_id
0
$cat /sys/devices/system/cpu/cpu3/topology/core_id
3

Signed-off-by: Atish Patra <[email protected]>
Acked-by: Sudeep Holla <[email protected]>
Acked-by: Paul Walmsley <[email protected]>
---
arch/riscv/Kconfig | 1 +
arch/riscv/kernel/smpboot.c | 3 +++
2 files changed, 4 insertions(+)

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 0c4b12205632..2d8a16299a85 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -47,6 +47,7 @@ config RISCV
select PCI_MSI if PCI
select RISCV_TIMER
select GENERIC_IRQ_MULTI_HANDLER
+ select GENERIC_ARCH_TOPOLOGY if SMP
select ARCH_HAS_PTE_SPECIAL
select ARCH_HAS_MMIOWB
select HAVE_EBPF_JIT if 64BIT
diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
index 7462a44304fe..18ae6da5115e 100644
--- a/arch/riscv/kernel/smpboot.c
+++ b/arch/riscv/kernel/smpboot.c
@@ -8,6 +8,7 @@
* Copyright (C) 2017 SiFive
*/

+#include <linux/arch_topology.h>
#include <linux/module.h>
#include <linux/init.h>
#include <linux/kernel.h>
@@ -35,6 +36,7 @@ static DECLARE_COMPLETION(cpu_running);

void __init smp_prepare_boot_cpu(void)
{
+ init_cpu_topology();
}

void __init smp_prepare_cpus(unsigned int max_cpus)
@@ -138,6 +140,7 @@ asmlinkage void __init smp_callin(void)

trap_init();
notify_cpu_starting(smp_processor_id());
+ update_siblings_masks(smp_processor_id());
set_cpu_online(smp_processor_id(), 1);
/*
* Remote TLB flushes are ignored while the CPU is offline, so emit
--
2.21.0

2019-06-17 19:02:35

by Atish Patra

[permalink] [raw]
Subject: [PATCH v7 4/7] arm: Use common cpu_topology structure and functions.

Currently, ARM32 and ARM64 uses different data structures to represent
their cpu topologies. Since, we are moving the ARM64 topology to common
code to be used by other architectures, we can reuse that for ARM32 as
well.

Take this opprtunity to remove the redundant functions from ARM32 and
reuse the common code instead.

To: Russell King <[email protected]>
Signed-off-by: Atish Patra <[email protected]>
Tested-by: Sudeep Holla <[email protected]> (on TC2)
Reviewed-by : Sudeep Holla <[email protected]>

---
Hi Russell,
Can we get a ACK for this patch ? We are hoping that the entire
series can be merged at one go.
---
arch/arm/include/asm/topology.h | 20 -----------
arch/arm/kernel/topology.c | 60 ++++-----------------------------
drivers/base/arch_topology.c | 4 ++-
include/linux/arch_topology.h | 6 ++--
4 files changed, 11 insertions(+), 79 deletions(-)

diff --git a/arch/arm/include/asm/topology.h b/arch/arm/include/asm/topology.h
index 2a786f54d8b8..8a0fae94d45e 100644
--- a/arch/arm/include/asm/topology.h
+++ b/arch/arm/include/asm/topology.h
@@ -5,26 +5,6 @@
#ifdef CONFIG_ARM_CPU_TOPOLOGY

#include <linux/cpumask.h>
-
-struct cputopo_arm {
- int thread_id;
- int core_id;
- int socket_id;
- cpumask_t thread_sibling;
- cpumask_t core_sibling;
-};
-
-extern struct cputopo_arm cpu_topology[NR_CPUS];
-
-#define topology_physical_package_id(cpu) (cpu_topology[cpu].socket_id)
-#define topology_core_id(cpu) (cpu_topology[cpu].core_id)
-#define topology_core_cpumask(cpu) (&cpu_topology[cpu].core_sibling)
-#define topology_sibling_cpumask(cpu) (&cpu_topology[cpu].thread_sibling)
-
-void init_cpu_topology(void);
-void store_cpu_topology(unsigned int cpuid);
-const struct cpumask *cpu_coregroup_mask(int cpu);
-
#include <linux/arch_topology.h>

/* Replace task scheduler's default frequency-invariant accounting */
diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c
index 60e375ce1ab2..238f1da0219c 100644
--- a/arch/arm/kernel/topology.c
+++ b/arch/arm/kernel/topology.c
@@ -177,17 +177,6 @@ static inline void parse_dt_topology(void) {}
static inline void update_cpu_capacity(unsigned int cpuid) {}
#endif

- /*
- * cpu topology table
- */
-struct cputopo_arm cpu_topology[NR_CPUS];
-EXPORT_SYMBOL_GPL(cpu_topology);
-
-const struct cpumask *cpu_coregroup_mask(int cpu)
-{
- return &cpu_topology[cpu].core_sibling;
-}
-
/*
* The current assumption is that we can power gate each core independently.
* This will be superseded by DT binding once available.
@@ -197,32 +186,6 @@ const struct cpumask *cpu_corepower_mask(int cpu)
return &cpu_topology[cpu].thread_sibling;
}

-static void update_siblings_masks(unsigned int cpuid)
-{
- struct cputopo_arm *cpu_topo, *cpuid_topo = &cpu_topology[cpuid];
- int cpu;
-
- /* update core and thread sibling masks */
- for_each_possible_cpu(cpu) {
- cpu_topo = &cpu_topology[cpu];
-
- if (cpuid_topo->socket_id != cpu_topo->socket_id)
- continue;
-
- cpumask_set_cpu(cpuid, &cpu_topo->core_sibling);
- if (cpu != cpuid)
- cpumask_set_cpu(cpu, &cpuid_topo->core_sibling);
-
- if (cpuid_topo->core_id != cpu_topo->core_id)
- continue;
-
- cpumask_set_cpu(cpuid, &cpu_topo->thread_sibling);
- if (cpu != cpuid)
- cpumask_set_cpu(cpu, &cpuid_topo->thread_sibling);
- }
- smp_wmb();
-}
-
/*
* store_cpu_topology is called at boot when only one cpu is running
* and with the mutex cpu_hotplug.lock locked, when several cpus have booted,
@@ -230,7 +193,7 @@ static void update_siblings_masks(unsigned int cpuid)
*/
void store_cpu_topology(unsigned int cpuid)
{
- struct cputopo_arm *cpuid_topo = &cpu_topology[cpuid];
+ struct cpu_topology *cpuid_topo = &cpu_topology[cpuid];
unsigned int mpidr;

/* If the cpu topology has been already set, just return */
@@ -250,12 +213,12 @@ void store_cpu_topology(unsigned int cpuid)
/* core performance interdependency */
cpuid_topo->thread_id = MPIDR_AFFINITY_LEVEL(mpidr, 0);
cpuid_topo->core_id = MPIDR_AFFINITY_LEVEL(mpidr, 1);
- cpuid_topo->socket_id = MPIDR_AFFINITY_LEVEL(mpidr, 2);
+ cpuid_topo->package_id = MPIDR_AFFINITY_LEVEL(mpidr, 2);
} else {
/* largely independent cores */
cpuid_topo->thread_id = -1;
cpuid_topo->core_id = MPIDR_AFFINITY_LEVEL(mpidr, 0);
- cpuid_topo->socket_id = MPIDR_AFFINITY_LEVEL(mpidr, 1);
+ cpuid_topo->package_id = MPIDR_AFFINITY_LEVEL(mpidr, 1);
}
} else {
/*
@@ -265,7 +228,7 @@ void store_cpu_topology(unsigned int cpuid)
*/
cpuid_topo->thread_id = -1;
cpuid_topo->core_id = 0;
- cpuid_topo->socket_id = -1;
+ cpuid_topo->package_id = -1;
}

update_siblings_masks(cpuid);
@@ -275,7 +238,7 @@ void store_cpu_topology(unsigned int cpuid)
pr_info("CPU%u: thread %d, cpu %d, socket %d, mpidr %x\n",
cpuid, cpu_topology[cpuid].thread_id,
cpu_topology[cpuid].core_id,
- cpu_topology[cpuid].socket_id, mpidr);
+ cpu_topology[cpuid].package_id, mpidr);
}

static inline int cpu_corepower_flags(void)
@@ -298,18 +261,7 @@ static struct sched_domain_topology_level arm_topology[] = {
*/
void __init init_cpu_topology(void)
{
- unsigned int cpu;
-
- /* init core mask and capacity */
- for_each_possible_cpu(cpu) {
- struct cputopo_arm *cpu_topo = &(cpu_topology[cpu]);
-
- cpu_topo->thread_id = -1;
- cpu_topo->core_id = -1;
- cpu_topo->socket_id = -1;
- cpumask_clear(&cpu_topo->core_sibling);
- cpumask_clear(&cpu_topo->thread_sibling);
- }
+ reset_cpu_topology();
smp_wmb();

parse_dt_topology();
diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
index 5781bb4c457c..797e3cd71bea 100644
--- a/drivers/base/arch_topology.c
+++ b/drivers/base/arch_topology.c
@@ -426,6 +426,7 @@ static int __init parse_dt_topology(void)
of_node_put(cn);
return ret;
}
+#endif

/*
* cpu topology table
@@ -491,7 +492,7 @@ static void clear_cpu_topology(int cpu)
cpumask_set_cpu(cpu, &cpu_topo->thread_sibling);
}

-static void __init reset_cpu_topology(void)
+void __init reset_cpu_topology(void)
{
unsigned int cpu;

@@ -526,6 +527,7 @@ __weak int __init parse_acpi_topology(void)
return 0;
}

+#if defined(CONFIG_ARM64) || defined(CONFIG_RISCV)
void __init init_cpu_topology(void)
{
reset_cpu_topology();
diff --git a/include/linux/arch_topology.h b/include/linux/arch_topology.h
index d4e76e0a283f..d4311127970d 100644
--- a/include/linux/arch_topology.h
+++ b/include/linux/arch_topology.h
@@ -54,11 +54,9 @@ extern struct cpu_topology cpu_topology[NR_CPUS];
void init_cpu_topology(void);
void store_cpu_topology(unsigned int cpuid);
const struct cpumask *cpu_coregroup_mask(int cpu);
-#endif
-
-#if defined(CONFIG_ARM64) || defined(CONFIG_RISCV)
void update_siblings_masks(unsigned int cpu);
-#endif
void remove_cpu_topology(unsigned int cpuid);
+void reset_cpu_topology(void);
+#endif

#endif /* _LINUX_ARCH_TOPOLOGY_H_ */
--
2.21.0

2019-06-17 19:02:56

by Atish Patra

[permalink] [raw]
Subject: [PATCH v7 7/7] MAINTAINERS: Add an entry for generic architecture topology

From: Sudeep Holla <[email protected]>

arm and arm64 shared lot of CPU topology related code. This was
consolidated under driver/base/arch_topology.c by Juri. Now RISC-V
is also started sharing the same code pulling more code from arm64
into arch_topology.c

Since I was involved in the review from the beginning, I would like
to assume maintenance for the same.

Cc: Will Deacon <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Acked-by: Juri Lelli <[email protected]>
Signed-off-by: Sudeep Holla <[email protected]>
---
MAINTAINERS | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 57f496cff999..c6f7d7152f01 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6595,6 +6595,13 @@ W: https://linuxtv.org
S: Maintained
F: drivers/media/radio/radio-gemtek*

+GENERIC ARCHITECTURE TOPOLOGY
+M: Sudeep Holla <[email protected]>
+L: [email protected]
+S: Maintained
+F: drivers/base/arch_topology.c
+F: include/linux/arch_topology.h
+
GENERIC GPIO I2C DRIVER
M: Wolfram Sang <[email protected]>
S: Supported
--
2.21.0

2019-06-17 19:02:59

by Atish Patra

[permalink] [raw]
Subject: [PATCH v7 6/7] base: arch_topology: update Kconfig help description

From: Sudeep Holla <[email protected]>

Commit 5d777b185f6d ("arch_topology: Make cpu_capacity sysfs node as read-only")
made cpu_capacity sysfs node read-only. Update the GENERIC_ARCH_TOPOLOGY
Kconfig help section to reflect the same.

Cc: Greg Kroah-Hartman <[email protected]>
Signed-off-by: Sudeep Holla <[email protected]>
---
drivers/base/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
index dc404492381d..28b92e3cc570 100644
--- a/drivers/base/Kconfig
+++ b/drivers/base/Kconfig
@@ -202,7 +202,7 @@ config GENERIC_ARCH_TOPOLOGY
help
Enable support for architectures common topology code: e.g., parsing
CPU capacity information from DT, usage of such information for
- appropriate scaling, sysfs interface for changing capacity values at
+ appropriate scaling, sysfs interface for reading capacity values at
runtime.

endmenu
--
2.21.0

2019-06-19 12:13:36

by Sudeep Holla

[permalink] [raw]
Subject: Re: [PATCH v7 4/7] arm: Use common cpu_topology structure and functions.

Hi Russell,

On Mon, Jun 17, 2019 at 11:59:17AM -0700, Atish Patra wrote:
> Currently, ARM32 and ARM64 uses different data structures to represent
> their cpu topologies. Since, we are moving the ARM64 topology to common
> code to be used by other architectures, we can reuse that for ARM32 as
> well.
>
> Take this opprtunity to remove the redundant functions from ARM32 and
> reuse the common code instead.
>
> To: Russell King <[email protected]>
> Signed-off-by: Atish Patra <[email protected]>
> Tested-by: Sudeep Holla <[email protected]> (on TC2)
> Reviewed-by : Sudeep Holla <[email protected]>
>
> ---
> Hi Russell,
> Can we get a ACK for this patch ? We are hoping that the entire
> series can be merged at one go.

It would be nice to get this in for v5.3 as it's almost there.
Are you fine with these changes ?

--
Regards,
Sudeep

2019-06-19 17:38:21

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v7 7/7] MAINTAINERS: Add an entry for generic architecture topology

On Mon, Jun 17, 2019 at 11:59:20AM -0700, Atish Patra wrote:
> From: Sudeep Holla <[email protected]>
>
> arm and arm64 shared lot of CPU topology related code. This was
> consolidated under driver/base/arch_topology.c by Juri. Now RISC-V
> is also started sharing the same code pulling more code from arm64
> into arch_topology.c
>
> Since I was involved in the review from the beginning, I would like
> to assume maintenance for the same.
>
> Cc: Will Deacon <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Acked-by: Juri Lelli <[email protected]>
> Signed-off-by: Sudeep Holla <[email protected]>

Acked-by: Greg Kroah-Hartman <[email protected]>

2019-06-24 15:08:51

by Sudeep Holla

[permalink] [raw]
Subject: Re: [PATCH v7 4/7] arm: Use common cpu_topology structure and functions.

On Wed, Jun 19, 2019 at 01:10:57PM +0100, Sudeep Holla wrote:
> Hi Russell,
>
> On Mon, Jun 17, 2019 at 11:59:17AM -0700, Atish Patra wrote:
> > Currently, ARM32 and ARM64 uses different data structures to represent
> > their cpu topologies. Since, we are moving the ARM64 topology to common
> > code to be used by other architectures, we can reuse that for ARM32 as
> > well.
> >
> > Take this opprtunity to remove the redundant functions from ARM32 and
> > reuse the common code instead.
> >
> > To: Russell King <[email protected]>
> > Signed-off-by: Atish Patra <[email protected]>
> > Tested-by: Sudeep Holla <[email protected]> (on TC2)
> > Reviewed-by : Sudeep Holla <[email protected]>
> >
> > ---
> > Hi Russell,
> > Can we get a ACK for this patch ? We are hoping that the entire
> > series can be merged at one go.
>
> It would be nice to get this in for v5.3 as it's almost there.
> Are you fine with these changes ?
>

Do you have any objections with this patch ? We plan to merge through
RISC-V tree, please let us know. It has been acked-by all the other
maintainers.

--
Regards,
Sudeep

2019-06-24 19:41:52

by Russell King (Oracle)

[permalink] [raw]
Subject: Re: [PATCH v7 4/7] arm: Use common cpu_topology structure and functions.

On Mon, Jun 24, 2019 at 04:06:58PM +0100, Sudeep Holla wrote:
> On Wed, Jun 19, 2019 at 01:10:57PM +0100, Sudeep Holla wrote:
> > Hi Russell,
> >
> > On Mon, Jun 17, 2019 at 11:59:17AM -0700, Atish Patra wrote:
> > > Currently, ARM32 and ARM64 uses different data structures to represent
> > > their cpu topologies. Since, we are moving the ARM64 topology to common
> > > code to be used by other architectures, we can reuse that for ARM32 as
> > > well.
> > >
> > > Take this opprtunity to remove the redundant functions from ARM32 and
> > > reuse the common code instead.
> > >
> > > To: Russell King <[email protected]>
> > > Signed-off-by: Atish Patra <[email protected]>
> > > Tested-by: Sudeep Holla <[email protected]> (on TC2)
> > > Reviewed-by : Sudeep Holla <[email protected]>
> > >
> > > ---
> > > Hi Russell,
> > > Can we get a ACK for this patch ? We are hoping that the entire
> > > series can be merged at one go.
> >
> > It would be nice to get this in for v5.3 as it's almost there.
> > Are you fine with these changes ?
> >
>
> Do you have any objections with this patch ? We plan to merge through
> RISC-V tree, please let us know. It has been acked-by all the other
> maintainers.

I have no interest in the CPU topology code; as far as I know I have
no systems that are able to exercise this code in any way. Therefore,
I don't know this code, I have no way to test it, and so it is not
appropriate for me to ack patches for it.

--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

2019-06-24 19:41:59

by Sudeep Holla

[permalink] [raw]
Subject: Re: [PATCH v7 4/7] arm: Use common cpu_topology structure and functions.

On Mon, Jun 24, 2019 at 04:30:33PM +0100, Russell King - ARM Linux admin wrote:
> On Mon, Jun 24, 2019 at 04:06:58PM +0100, Sudeep Holla wrote:
> > On Wed, Jun 19, 2019 at 01:10:57PM +0100, Sudeep Holla wrote:
> > > Hi Russell,
> > >
> > > On Mon, Jun 17, 2019 at 11:59:17AM -0700, Atish Patra wrote:
> > > > Currently, ARM32 and ARM64 uses different data structures to represent
> > > > their cpu topologies. Since, we are moving the ARM64 topology to common
> > > > code to be used by other architectures, we can reuse that for ARM32 as
> > > > well.
> > > >
> > > > Take this opprtunity to remove the redundant functions from ARM32 and
> > > > reuse the common code instead.
> > > >
> > > > To: Russell King <[email protected]>
> > > > Signed-off-by: Atish Patra <[email protected]>
> > > > Tested-by: Sudeep Holla <[email protected]> (on TC2)
> > > > Reviewed-by : Sudeep Holla <[email protected]>
> > > >
> > > > ---
> > > > Hi Russell,
> > > > Can we get a ACK for this patch ? We are hoping that the entire
> > > > series can be merged at one go.
> > >
> > > It would be nice to get this in for v5.3 as it's almost there.
> > > Are you fine with these changes ?
> > >
> >
> > Do you have any objections with this patch ? We plan to merge through
> > RISC-V tree, please let us know. It has been acked-by all the other
> > maintainers.
>
> I have no interest in the CPU topology code; as far as I know I have
> no systems that are able to exercise this code in any way. Therefore,
> I don't know this code, I have no way to test it, and so it is not
> appropriate for me to ack patches for it.
>

I completely understand that and we can take care of testing. As along
as you don't have objections to this, that should be fine I believe.

--
Regards,
Sudeep

2019-06-27 00:55:06

by Paul Walmsley

[permalink] [raw]
Subject: Re: [PATCH v7 1/7] Documentation: DT: arm: add support for sockets defining package boundaries

Hi Sudeep, Atish,

On Mon, 17 Jun 2019, Atish Patra wrote:

> From: Sudeep Holla <[email protected]>
>
> The current ARM DT topology description provides the operating system
> with a topological view of the system that is based on leaf nodes
> representing either cores or threads (in an SMT system) and a
> hierarchical set of cluster nodes that creates a hierarchical topology
> view of how those cores and threads are grouped.
>
> However this hierarchical representation of clusters does not allow to
> describe what topology level actually represents the physical package or
> the socket boundary, which is a key piece of information to be used by
> an operating system to optimize resource allocation and scheduling.
>
> Lets add a new "socket" node type in the cpu-map node to describe the
> same.
>
> Signed-off-by: Sudeep Holla <[email protected]>
> Reviewed-by: Rob Herring <[email protected]>

This one doesn't apply cleanly here on top of v5.2-rc2, Linus's master
branch, and next-20190626. The reject file is below. Am I missing
a patch?


- Paul

--- Documentation/devicetree/bindings/arm/topology.txt
+++ Documentation/devicetree/bindings/arm/topology.txt
@@ -185,13 +206,15 @@ Bindings for cluster/cpu/thread nodes are defined as follows:
4 - Example dts
===========================================

-Example 1 (ARM 64-bit, 16-cpu system, two clusters of clusters):
+Example 1 (ARM 64-bit, 16-cpu system, two clusters of clusters in a single
+physical socket):

cpus {
#size-cells = <0>;
#address-cells = <2>;

cpu-map {
+ socket0 {
cluster0 {
cluster0 {
core0 {

2019-06-27 02:19:18

by Atish Patra

[permalink] [raw]
Subject: Re: [PATCH v7 1/7] Documentation: DT: arm: add support for sockets defining package boundaries

On 6/26/19 5:31 PM, Paul Walmsley wrote:
> Hi Sudeep, Atish,
>
> On Mon, 17 Jun 2019, Atish Patra wrote:
>
>> From: Sudeep Holla <[email protected]>
>>
>> The current ARM DT topology description provides the operating system
>> with a topological view of the system that is based on leaf nodes
>> representing either cores or threads (in an SMT system) and a
>> hierarchical set of cluster nodes that creates a hierarchical topology
>> view of how those cores and threads are grouped.
>>
>> However this hierarchical representation of clusters does not allow to
>> describe what topology level actually represents the physical package or
>> the socket boundary, which is a key piece of information to be used by
>> an operating system to optimize resource allocation and scheduling.
>>
>> Lets add a new "socket" node type in the cpu-map node to describe the
>> same.
>>
>> Signed-off-by: Sudeep Holla <[email protected]>
>> Reviewed-by: Rob Herring <[email protected]>
>
> This one doesn't apply cleanly here on top of v5.2-rc2, Linus's master
> branch, and next-20190626. The reject file is below. Am I missing
> a patch?
>

That's weird. I could apply the patch from any git tree (github or
git.kernel.org) but not from mail or patchworks.

git log doesn't show any recent modifications of that file. I am trying
to figure out what's wrong.
>
> - Paul
>
> --- Documentation/devicetree/bindings/arm/topology.txt
> +++ Documentation/devicetree/bindings/arm/topology.txt
> @@ -185,13 +206,15 @@ Bindings for cluster/cpu/thread nodes are defined as follows:
> 4 - Example dts
> ===========================================
>
> -Example 1 (ARM 64-bit, 16-cpu system, two clusters of clusters):
> +Example 1 (ARM 64-bit, 16-cpu system, two clusters of clusters in a single
> +physical socket):
>
> cpus {
> #size-cells = <0>;
> #address-cells = <2>;
>
> cpu-map {
> + socket0 {
> cluster0 {
> cluster0 {
> core0 {
>


--
Regards,
Atish

2019-06-27 19:59:22

by Atish Patra

[permalink] [raw]
Subject: Re: [PATCH v7 1/7] Documentation: DT: arm: add support for sockets defining package boundaries

On Wed, 2019-06-26 at 19:18 -0700, Atish Patra wrote:
> On 6/26/19 5:31 PM, Paul Walmsley wrote:
> > Hi Sudeep, Atish,
> >
> > On Mon, 17 Jun 2019, Atish Patra wrote:
> >
> > > From: Sudeep Holla <[email protected]>
> > >
> > > The current ARM DT topology description provides the operating
> > > system
> > > with a topological view of the system that is based on leaf nodes
> > > representing either cores or threads (in an SMT system) and a
> > > hierarchical set of cluster nodes that creates a hierarchical
> > > topology
> > > view of how those cores and threads are grouped.
> > >
> > > However this hierarchical representation of clusters does not
> > > allow to
> > > describe what topology level actually represents the physical
> > > package or
> > > the socket boundary, which is a key piece of information to be
> > > used by
> > > an operating system to optimize resource allocation and
> > > scheduling.
> > >
> > > Lets add a new "socket" node type in the cpu-map node to describe
> > > the
> > > same.
> > >
> > > Signed-off-by: Sudeep Holla <[email protected]>
> > > Reviewed-by: Rob Herring <[email protected]>
> >
> > This one doesn't apply cleanly here on top of v5.2-rc2, Linus's
> > master
> > branch, and next-20190626. The reject file is below. Am I missing
> > a patch?
> >
>
> That's weird. I could apply the patch from any git tree (github or
> git.kernel.org) but not from mail or patchworks.
>
> git log doesn't show any recent modifications of that file. I am
> trying
> to figure out what's wrong.

The space changes in this patch caused the conflict. The patch was
generated with -b which was suggested during the initial review.

I should removed it before sending v7. My bad.
I have fixed that and sent a v8 that should be cleanly applied on
latest master. The patch series is also available at

https://github.com/atishp04/linux/tree/5.2-rc6_topology

Regards,
Atish
> > - Paul
> >
> > --- Documentation/devicetree/bindings/arm/topology.txt
> > +++ Documentation/devicetree/bindings/arm/topology.txt
> > @@ -185,13 +206,15 @@ Bindings for cluster/cpu/thread nodes are
> > defined as follows:
> > 4 - Example dts
> > ===========================================
> >
> > -Example 1 (ARM 64-bit, 16-cpu system, two clusters of clusters):
> > +Example 1 (ARM 64-bit, 16-cpu system, two clusters of clusters in
> > a single
> > +physical socket):
> >
> > cpus {
> > #size-cells = <0>;
> > #address-cells = <2>;
> >
> > cpu-map {
> > + socket0 {
> > cluster0 {
> > cluster0 {
> > core0 {
> >
>
>