From: Qing Wang <hidden> Date: 2022-04-22 11:52:06
From: Wang Qing <redacted>
The DSU cluster supports blocks that are called complexes
which contain up to two cores of the same type and some shared logic,
which sharing some logic between the cores can make a complex area efficient.
Complex also can be considered as a shared cache group smaller
than cluster.
This patch adds complex level for complexs by parsing cache topology
form DT. It will directly benefit a lot of workload which loves more
resources such as memory bandwidth, caches.
Note this patch only handle the DT case.
V2:
fix commit log and loop more
wangqing (2):
arch_topology: support for describing cache topology from DT
arm64: Add complex scheduler level for arm64
arch/arm64/Kconfig | 13 ++++++++++
arch/arm64/kernel/smp.c | 48 ++++++++++++++++++++++++++++++++++-
drivers/base/arch_topology.c | 47 +++++++++++++++++++++++++++++++++-
include/linux/arch_topology.h | 3 +++
4 files changed, 109 insertions(+), 2 deletions(-)
--
2.7.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Qing Wang <hidden> Date: 2022-04-22 11:52:11
From: Wang Qing <redacted>
When ACPI is not enabled, we can get cache topolopy from DT like:
* cpu0: cpu@000 {
* next-level-cache = <&L2_1>;
* L2_1: l2-cache {
* compatible = "cache";
* next-level-cache = <&L3_1>;
* };
* L3_1: l3-cache {
* compatible = "cache";
* };
* };
*
* cpu1: cpu@001 {
* next-level-cache = <&L2_1>;
* };
* ...
* };
cache_topology[] hold the pointer describing by "next-level-cache",
which can describe the cache topology of every level.
MAX_CACHE_LEVEL is strictly corresponding to the cache level from L2.
V2:
make function name more sense
Signed-off-by: Wang Qing <redacted>
---
drivers/base/arch_topology.c | 47 ++++++++++++++++++++++++++++++++++-
include/linux/arch_topology.h | 3 +++
2 files changed, 49 insertions(+), 1 deletion(-)
@@ -480,8 +480,10 @@ static int __init get_cpu_for_node(struct device_node *node)return-1;cpu=of_cpu_node_to_id(cpu_node);-if(cpu>=0)+if(cpu>=0){topology_parse_cpu_capacity(cpu_node,cpu);+topology_parse_cpu_caches(cpu_node,cpu);+}elsepr_info("CPU node for %pOF exist but the possible cpu range is :%*pbl\n",cpu_node,cpumask_pr_args(cpu_possible_mask));
@@ -647,6 +649,49 @@ static int __init parse_dt_topology(void)}#endif+/*+*cpucachetopologytable+*/+#define MAX_CACHE_LEVEL 7+staticstructdevice_node*cache_topology[NR_CPUS][MAX_CACHE_LEVEL];++voidtopology_parse_cpu_caches(structdevice_node*cpu_node,intcpu)+{+structdevice_node*node_cache=cpu_node;+intlevel=0;++while(level<MAX_CACHE_LEVEL){+node_cache=of_parse_phandle(node_cache,"next-level-cache",0);+if(!node_cache)+break;++cache_topology[cpu][level++]=node_cache;+}+}++/*+*findthelargestsubsetofthesharedcacheintherangeofcpu_mask+*/+voidfind_subset_of_share_cache(conststructcpumask*cpu_mask,intcpu,+structcpumask*sc_mask)+{+intcache_level,cpu_id;++for(cache_level=MAX_CACHE_LEVEL-1;cache_level>=0;cache_level--){+if(!cache_topology[cpu][cache_level])+continue;++cpumask_clear(sc_mask);+for(cpu_id=0;cpu_id<NR_CPUS;cpu_id++){+if(cache_topology[cpu][cache_level]==cache_topology[cpu_id][cache_level])+cpumask_set_cpu(cpu_id,sc_mask);+}++if(cpumask_subset(sc_mask,cpu_mask))+break;+}+}+/**cputopologytable*/
From: Qing Wang <hidden> Date: 2022-04-22 11:52:15
From: Wang Qing <redacted>
The DSU-110 DynamIQ™ cluster supports blocks that are called complexes
which contain up to two cores of the same type and some shared logic.
Sharing some logic between the cores can make a complex area efficient.
This patch adds complex level for complexs and automatically enables
the load balance among complexs. It will directly benefit a lot of
workload which loves more resources such as memory bandwidth, caches.
Testing has been done with Stream benchmark:
8threads stream (2 little cores * 2(complex) + 3 medium cores + 1 big core)
stream stream
w/o patch w/ patch
MB/sec copy 37579.2 ( 0.00%) 39127.3 ( 4.12%)
MB/sec scale 38261.1 ( 0.00%) 39195.4 ( 2.44%)
MB/sec add 39497.0 ( 0.00%) 41101.5 ( 4.06%)
MB/sec triad 39885.6 ( 0.00%) 40772.7 ( 2.22%)
And in order to support this features, we defined arm64_topology.
V2:
fix commit log and loop more
Signed-off-by: Wang Qing <redacted>
---
arch/arm64/Kconfig | 13 +++++++++++
arch/arm64/kernel/smp.c | 48 ++++++++++++++++++++++++++++++++++++++++-
2 files changed, 60 insertions(+), 1 deletion(-)
On Fri, Apr 22, 2022 at 04:51:25AM -0700, Qing Wang wrote:
From: Wang Qing <redacted>
When ACPI is not enabled, we can get cache topolopy from DT like:
* cpu0: cpu@000 {
* next-level-cache = <&L2_1>;
* L2_1: l2-cache {
* compatible = "cache";
* next-level-cache = <&L3_1>;
* };
* L3_1: l3-cache {
* compatible = "cache";
* };
* };
*
* cpu1: cpu@001 {
* next-level-cache = <&L2_1>;
* };
* ...
* };
cache_topology[] hold the pointer describing by "next-level-cache",
which can describe the cache topology of every level.
MAX_CACHE_LEVEL is strictly corresponding to the cache level from L2.
I have no idea what this changelog means at all.
What are you trying to do? What problem are you solving? Why are you
doing any of this?
V2:
make function name more sense
As per the documentation this goes below the --- line, right?
@@ -480,8 +480,10 @@ static int __init get_cpu_for_node(struct device_node *node)return-1;cpu=of_cpu_node_to_id(cpu_node);-if(cpu>=0)+if(cpu>=0){topology_parse_cpu_capacity(cpu_node,cpu);+topology_parse_cpu_caches(cpu_node,cpu);+}elsepr_info("CPU node for %pOF exist but the possible cpu range is :%*pbl\n",cpu_node,cpumask_pr_args(cpu_possible_mask));
@@ -647,6 +649,49 @@ static int __init parse_dt_topology(void)}#endif+/*+*cpucachetopologytable+*/+#define MAX_CACHE_LEVEL 7+staticstructdevice_node*cache_topology[NR_CPUS][MAX_CACHE_LEVEL];
So for a normal big system of 4k cpus * 7 levels, that's a lot of
memory? are you sure?
How big of a box did you test this on?
quoted hunk
++void topology_parse_cpu_caches(struct device_node *cpu_node, int cpu)+{+ struct device_node *node_cache = cpu_node;+ int level = 0;++ while (level < MAX_CACHE_LEVEL) {+ node_cache = of_parse_phandle(node_cache, "next-level-cache", 0);+ if (!node_cache)+ break;++ cache_topology[cpu][level++] = node_cache;+ }
No locking anywhere? What could go wrong :(
quoted hunk
+}++/*+ * find the largest subset of the shared cache in the range of cpu_mask+ */+void find_subset_of_share_cache(const struct cpumask *cpu_mask, int cpu,+ struct cpumask *sc_mask)
Again, horrid global function name.
And no kernel documentation for how this works?
quoted hunk
+{+ int cache_level, cpu_id;++ for (cache_level = MAX_CACHE_LEVEL - 1; cache_level >= 0; cache_level--) {+ if (!cache_topology[cpu][cache_level])+ continue;
No locking???
quoted hunk
++ cpumask_clear(sc_mask);+ for (cpu_id = 0; cpu_id < NR_CPUS; cpu_id++) {+ if (cache_topology[cpu][cache_level] == cache_topology[cpu_id][cache_level])+ cpumask_set_cpu(cpu_id, sc_mask);+ }++ if (cpumask_subset(sc_mask, cpu_mask))+ break;+ }+}+ /* * cpu topology table */
@@ -93,6 +93,9 @@ void update_siblings_masks(unsigned int cpu);voidremove_cpu_topology(unsignedintcpuid);voidreset_cpu_topology(void);intparse_acpi_topology(void);+voidtopology_parse_cpu_caches(structdevice_node*cpu_node,intcpu);+voidfind_subset_of_share_cache(conststructcpumask*cpu_mask,intcpu,+structcpumask*sc_mask);
I still have no idea what this last function is supposed to do.
And very odd indentation, did you run checkpatch on this?
totally confused,
greg k-h
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Fri, Apr 22, 2022 at 04:51:26AM -0700, Qing Wang wrote:
quoted hunk
From: Wang Qing <redacted>
The DSU-110 DynamIQ™ cluster supports blocks that are called complexes
which contain up to two cores of the same type and some shared logic.
Sharing some logic between the cores can make a complex area efficient.
This patch adds complex level for complexs and automatically enables
the load balance among complexs. It will directly benefit a lot of
workload which loves more resources such as memory bandwidth, caches.
Testing has been done with Stream benchmark:
8threads stream (2 little cores * 2(complex) + 3 medium cores + 1 big core)
stream stream
w/o patch w/ patch
MB/sec copy 37579.2 ( 0.00%) 39127.3 ( 4.12%)
MB/sec scale 38261.1 ( 0.00%) 39195.4 ( 2.44%)
MB/sec add 39497.0 ( 0.00%) 41101.5 ( 4.06%)
MB/sec triad 39885.6 ( 0.00%) 40772.7 ( 2.22%)
And in order to support this features, we defined arm64_topology.
V2:
fix commit log and loop more
Signed-off-by: Wang Qing <redacted>
---
arch/arm64/Kconfig | 13 +++++++++++
arch/arm64/kernel/smp.c | 48 ++++++++++++++++++++++++++++++++++++++++-
2 files changed, 60 insertions(+), 1 deletion(-)
+static int arm64_complex_flags(void)+{+ return SD_SHARE_PKG_RESOURCES;+}++const struct cpumask *arm64_complex_mask(int cpu)+{+ const struct cpumask *core_mask = cpu_cpu_mask(cpu);++ /* Find the smaller shared cache level than clustergroup and coregroup*/+#ifdef CONFIG_SCHED_MC+ core_mask = cpu_coregroup_mask(cpu);+#endif+#ifdef CONFIG_SCHED_CLUSTER+ core_mask = cpu_clustergroup_mask(cpu);+#endif
See, same here. This is a mess and unmaintainable.
thanks,
greg k-h
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Wang Qing <redacted>
The DSU-110 DynamIQ™ cluster supports blocks that are called complexes
which contain up to two cores of the same type and some shared logic.
Sharing some logic between the cores can make a complex area efficient.
This patch adds complex level for complexs and automatically enables
the load balance among complexs. It will directly benefit a lot of
workload which loves more resources such as memory bandwidth, caches.
Testing has been done with Stream benchmark:
8threads stream (2 little cores * 2(complex) + 3 medium cores + 1 big core)
stream stream
w/o patch w/ patch
MB/sec copy 37579.2 ( 0.00%) 39127.3 ( 4.12%)
MB/sec scale 38261.1 ( 0.00%) 39195.4 ( 2.44%)
MB/sec add 39497.0 ( 0.00%) 41101.5 ( 4.06%)
MB/sec triad 39885.6 ( 0.00%) 40772.7 ( 2.22%)
And in order to support this features, we defined arm64_topology.
V2:
fix commit log and loop more
Signed-off-by: Wang Qing <redacted>
---
arch/arm64/Kconfig | 13 +++++++++++
arch/arm64/kernel/smp.c | 48 ++++++++++++++++++++++++++++++++++++++++-
2 files changed, 60 insertions(+), 1 deletion(-)
by sharing mid-level caches, last-level cache tags or internal
busses.
+config SCHED_COMPLEX
+ bool "Complex scheduler support"
+ help
+ DSU supports blocks that are called complexes which contain up to
+ two cores of the same type and some shared logic. Sharing some logic
+ between the cores can make a complex area efficient.
+
+ Complex also can be considered as a shared cache group smaller
+ than cluster.
+
+ Complex scheduler support improves the CPU scheduler's decision
+ making when dealing with machines that have complexs of CPUs.
+
config SCHED_SMT
bool "SMT scheduler support"
help
From: Wang Qing <redacted>
When ACPI is not enabled, we can get cache topolopy from DT like:
* cpu0: cpu@000 {
* next-level-cache = <&L2_1>;
* L2_1: l2-cache {
* compatible = "cache";
* next-level-cache = <&L3_1>;
* };
* L3_1: l3-cache {
* compatible = "cache";
* };
* };
*
* cpu1: cpu@001 {
* next-level-cache = <&L2_1>;
* };
* ...
* };
cache_topology[] hold the pointer describing by "next-level-cache",
which can describe the cache topology of every level.
MAX_CACHE_LEVEL is strictly corresponding to the cache level from L2.
I have no idea what this changelog means at all.
What are you trying to do? What problem are you solving? Why are you
doing any of this?
This patch just supports parsing cache topology from DT in the early
(before sched domain initialization). Then we can use the information
about shared cache level to build the sched domain level.
quoted
V2:
make function name more sense
As per the documentation this goes below the --- line, right?
@@ -480,8 +480,10 @@ static int __init get_cpu_for_node(struct device_node *node)
return -1;
cpu = of_cpu_node_to_id(cpu_node);
- if (cpu >= 0)
+ if (cpu >= 0) {
topology_parse_cpu_capacity(cpu_node, cpu);
+ topology_parse_cpu_caches(cpu_node, cpu);
+ }
else
pr_info("CPU node for %pOF exist but the possible cpu range is :%*pbl\n",
cpu_node, cpumask_pr_args(cpu_possible_mask));
@@ -647,6 +649,49 @@ static int __init parse_dt_topology(void)
So for a normal big system of 4k cpus * 7 levels, that's a lot of
memory? are you sure?
Yes, if CONFIG_NR_CPUS is configured as 4k, it will take up 224KB memory
in 4k cpus system.
How big of a box did you test this on?
I tested on android phone.
quoted
++void topology_parse_cpu_caches(struct device_node *cpu_node, int cpu)+{+ struct device_node *node_cache = cpu_node;+ int level = 0;++ while (level < MAX_CACHE_LEVEL) {+ node_cache = of_parse_phandle(node_cache, "next-level-cache", 0);+ if (!node_cache)+ break;++ cache_topology[cpu][level++] = node_cache;+ }
No locking anywhere? What could go wrong :(
The calling process of the function has guaranteed no data racing condition.
cache_topology[] will only be modified by parse_dt_topology().
cache_topology[] behaves like cpu_topology[].
quoted
+}++/*+ * find the largest subset of the shared cache in the range of cpu_mask+ */+void find_subset_of_share_cache(const struct cpumask *cpu_mask, int cpu,+ struct cpumask *sc_mask)
Again, horrid global function name.
And no kernel documentation for how this works?
Maybe I should post an FRC patch first.
quoted
+{+ int cache_level, cpu_id;++ for (cache_level = MAX_CACHE_LEVEL - 1; cache_level >= 0; cache_level--) {+ if (!cache_topology[cpu][cache_level])+ continue;
No locking???
quoted
+
+ cpumask_clear(sc_mask);
+ for (cpu_id = 0; cpu_id < NR_CPUS; cpu_id++) {
+ if (cache_topology[cpu][cache_level] == cache_topology[cpu_id][cache_level])
+ cpumask_set_cpu(cpu_id, sc_mask);
+ }
+
+ if (cpumask_subset(sc_mask, cpu_mask))
+ break;
+ }
+}
+
/*
* cpu topology table
*/
@@ -93,6 +93,9 @@ void update_siblings_masks(unsigned int cpu);
void remove_cpu_topology(unsigned int cpuid);
void reset_cpu_topology(void);
int parse_acpi_topology(void);
+void topology_parse_cpu_caches(struct device_node *cpu_node, int cpu);
+void find_subset_of_share_cache(const struct cpumask *cpu_mask, int cpu,
+ struct cpumask *sc_mask);
I still have no idea what this last function is supposed to do.
This function is to find the largest shared cache subset from the
specified cpumask, so we can make them a new level sched domain,
which will directly benefit a lot of workload.
And very odd indentation, did you run checkpatch on this?
Yes, I modify it manually after running checkpatch, this is my problem.
From: Jonathan Cameron <hidden> Date: 2022-04-25 13:20:04
On Fri, 22 Apr 2022 04:51:24 -0700
Qing Wang [off-list ref] wrote:
From: Wang Qing <redacted>
The DSU cluster supports blocks that are called complexes
which contain up to two cores of the same type and some shared logic,
which sharing some logic between the cores can make a complex area efficient.
Given the complex shares things like the SVE units (cortex a510)...
Why not handle this as SMT?
Seems like a blurred boundary between separate cores and SMT threads.
I think we need to express and potentially take advantage of knowledge
about what logic is being shared.
Jonathan
Complex also can be considered as a shared cache group smaller
than cluster.
This patch adds complex level for complexs by parsing cache topology
form DT. It will directly benefit a lot of workload which loves more
resources such as memory bandwidth, caches.
Note this patch only handle the DT case.
V2:
fix commit log and loop more
wangqing (2):
arch_topology: support for describing cache topology from DT
arm64: Add complex scheduler level for arm64
arch/arm64/Kconfig | 13 ++++++++++
arch/arm64/kernel/smp.c | 48 ++++++++++++++++++++++++++++++++++-
drivers/base/arch_topology.c | 47 +++++++++++++++++++++++++++++++++-
include/linux/arch_topology.h | 3 +++
4 files changed, 109 insertions(+), 2 deletions(-)
From: Wang Qing <redacted>
The DSU cluster supports blocks that are called complexes
which contain up to two cores of the same type and some shared logic,
which sharing some logic between the cores can make a complex area efficient.
Given the complex shares things like the SVE units (cortex a510)...
Why not handle this as SMT?
SMT should share all cache levels. but complexs only share L2(and L3)
cache here.
Seems like a blurred boundary between separate cores and SMT threads.
I think we need to express and potentially take advantage of knowledge
about what logic is being shared.
Logic such as a Vector Processing Unit, L2 Translation Lookaside Buffer
(TLB) ... are shared, I believe this will improve efficiency even if
only L2 cache is shared.
Thanks,
Qing
Jonathan
quoted
Complex also can be considered as a shared cache group smaller
than cluster.
This patch adds complex level for complexs by parsing cache topology
form DT. It will directly benefit a lot of workload which loves more
resources such as memory bandwidth, caches.
Note this patch only handle the DT case.
V2:
fix commit log and loop more
wangqing (2):
arch_topology: support for describing cache topology from DT
arm64: Add complex scheduler level for arm64
arch/arm64/Kconfig | 13 ++++++++++
arch/arm64/kernel/smp.c | 48 ++++++++++++++++++++++++++++++++++-
drivers/base/arch_topology.c | 47 +++++++++++++++++++++++++++++++++-
include/linux/arch_topology.h | 3 +++
4 files changed, 109 insertions(+), 2 deletions(-)
Hi Sudeep:
I am thinking if it is possible to add a complex level cpu topology
between cluster and SMT?
We can describe it directly in “cpu-map”, instead of parsing it through
the cache info.
Thanks,
Qing
quoted
quoted
From: Wang Qing <redacted>
The DSU cluster supports blocks that are called complexes
which contain up to two cores of the same type and some shared logic,
which sharing some logic between the cores can make a complex area efficient.
Given the complex shares things like the SVE units (cortex a510)...
Why not handle this as SMT?
SMT should share all cache levels. but complexs only share L2(and L3)
cache here.
quoted
Seems like a blurred boundary between separate cores and SMT threads.
I think we need to express and potentially take advantage of knowledge
about what logic is being shared.
Logic such as a Vector Processing Unit, L2 Translation Lookaside Buffer
(TLB) ... are shared, I believe this will improve efficiency even if
only L2 cache is shared.
Thanks,
Qing
quoted
Jonathan
quoted
Complex also can be considered as a shared cache group smaller
than cluster.
This patch adds complex level for complexs by parsing cache topology
form DT. It will directly benefit a lot of workload which loves more
resources such as memory bandwidth, caches.
Note this patch only handle the DT case.
V2:
fix commit log and loop more
wangqing (2):
arch_topology: support for describing cache topology from DT
arm64: Add complex scheduler level for arm64
arch/arm64/Kconfig | 13 ++++++++++
arch/arm64/kernel/smp.c | 48 ++++++++++++++++++++++++++++++++++-
drivers/base/arch_topology.c | 47 +++++++++++++++++++++++++++++++++-
include/linux/arch_topology.h | 3 +++
4 files changed, 109 insertions(+), 2 deletions(-)
On Tue, Apr 26, 2022 at 07:05:15AM +0000, 王擎 wrote:
Hi Sudeep:
I am thinking if it is possible to add a complex level cpu topology
between cluster and SMT?
We can describe it directly in “cpu-map”, instead of parsing it through
the cache info.
I don't know or understand what you mean by that. Do you have a proposal
for DT bindings ? Please post the patch with details on the motivation for
the change to help us understand your proposal. I can't comment much without
seeing or understanding your proposal at this moment.
--
Regards,
Sudeep
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel