Thread (32 messages) 32 messages, 6 authors, 2022-07-19

Re: [PATCH v6 17/21] arch_topology: Limit span of cpu_clustergroup_mask()

From: Darren Hart <hidden>
Date: 2022-07-08 16:27:46
Also in: linux-riscv, lkml

On Fri, Jul 08, 2022 at 09:04:24AM +0100, Sudeep Holla wrote:
Hi Darren,

I will let Ionela or Dietmar cover some of the scheduler aspects as
I don't have much knowledge in that area.

On Thu, Jul 07, 2022 at 05:10:19PM -0700, Darren Hart wrote:
quoted
On Mon, Jul 04, 2022 at 11:16:01AM +0100, Sudeep Holla wrote:
quoted
From: Ionela Voinescu <redacted>
Hi Sudeep and Ionela,
quoted
Currently the cluster identifier is not set on DT based platforms.
The reset or default value is -1 for all the CPUs. Once we assign the
cluster identifier values correctly, the cluster_sibling mask will be
populated and returned by cpu_clustergroup_mask() to contribute in the
creation of the CLS scheduling domain level, if SCHED_CLUSTER is
enabled.

To avoid topologies that will result in questionable or incorrect
scheduling domains, impose restrictions regarding the span of clusters,
Can you provide a specific example of a valid topology that results in
the wrong thing currently?
As a simple example, Juno with 2 clusters and L2 for each cluster. IIUC
MC is preferred instead of CLS and both MC and CLS domains are exact
match.
quoted
quoted
While previously the scheduling domain builder code would have removed MC
as redundant and kept CLS if SCHED_CLUSTER was enabled and the
cpu_coregroup_mask() and cpu_clustergroup_mask() spanned the same CPUs,
now CLS will be removed and MC kept.
This is not desireable for all systems, particular those which don't
have an L3 but do share other resources - such as the snoop filter in
the case of the Ampere Altra.
I was wrong here. This match also modifies the coregroup, the MC after
this patch is equivalent to the CLS before the patch. The Altra is not
negatively impacted here.
quoted
While not universally supported, we agreed in the discussion on the
above patch to allow systems to define clusters independently from the
L3 as an LLC since this is also independently defined in PPTT.

Going back to my first comment - does this fix an existing system with a
valid topology? 
Yes as mentioned above Juno.
quoted
It's not clear to me what that would look like. The Ampere Altra presents
a cluster level in PPTT because that is the desireable topology for the
system.
Absolutely wrong reason. It should present because the hardware is so,
not because some OSPM desires something in someway. Sorry that's not how
DT/ACPI is designed for. If 2 different OSPM desires different things, then
one ACPI will not be sufficient.
Agree. I worded that badly. I should have said the Altra presents a PPTT
topology that accurately reflects the hardwere. There is no shared
cpu-side LLC, and there is an affinity between the DSU pairs which share
a snoop filter.

I do think the general assumption that MC shares a cpu-side LLC will
continue to present challenges to the Altra topology in terms of ongoing
to changes to the code. I don't have a good solution to that at the
moment, something I'll continue to think on.
quoted
If it's not desirable for another system to have the cluster topology -
shouldn't it not present that layer to the kernel in the first place?
Absolutely 100% yes, it must present it if the hardware is designed so.
No if or but.

-- 
Regards,
Sudeep
Thanks Sudeep,

-- 
Darren Hart
Ampere Computing / OS and Kernel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help