From: Dietmar Eggemann <dietmar.eggemann@arm.com>
To: Darren Hart <darren@os.amperecomputing.com>
Cc: Vincent Guittot <vincent.guittot@linaro.org>,
Will Deacon <will@kernel.org>,
LKML <linux-kernel@vger.kernel.org>,
Linux Arm <linux-arm-kernel@lists.infradead.org>,
Sudeep Holla <sudeep.holla@arm.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
"Rafael J. Wysocki" <rafael@kernel.org>,
Catalin Marinas <Catalin.Marinas@arm.com>,
Peter Zijlstra <peterz@infradead.org>,
Barry Song <song.bao.hua@hisilicon.com>,
Valentin Schneider <Valentin.Schneider@arm.com>,
"D . Scott Phillips" <scott@os.amperecomputing.com>,
Ilkka Koskinen <ilkka@os.amperecomputing.com>,
stable@vger.kernel.org
Subject: Re: [PATCH v3] topology: make core_mask include at least cluster_siblings
Date: Mon, 14 Mar 2022 17:35:05 +0100 [thread overview]
Message-ID: <9398d7ad-30e7-890a-3e18-c3011c383585@arm.com> (raw)
In-Reply-To: <YijxUAuufpBKLtwy@fedora>
On 09/03/2022 19:26, Darren Hart wrote:
> On Wed, Mar 09, 2022 at 01:50:07PM +0100, Dietmar Eggemann wrote:
>> On 08/03/2022 18:49, Darren Hart wrote:
>>> On Tue, Mar 08, 2022 at 05:03:07PM +0100, Dietmar Eggemann wrote:
>>>> On 08/03/2022 12:04, Vincent Guittot wrote:
>>>>> On Tue, 8 Mar 2022 at 11:30, Will Deacon <will@kernel.org> wrote:
[...]
>>>> I do not have any better idea than this tweak here either in case the
>>>> platform can't provide a cleaner setup.
>>>
>>> I'd argue The platform is describing itself accurately in ACPI PPTT
>>> terms. The topology doesn't fit nicely within the kernel abstractions
>>> today. This is an area where I hope to continue to improve things going
>>> forward.
>>
>> I see. And I assume lying about SCU/LLC boundaries in ACPI is not an
>> option since it messes up /sys/devices/system/cpu/cpu0/cache/index*/.
>>
>> [...]
>
> I'm not aware of a way to accurately describe the SCU topology in the PPTT, and
> the risk we run with lying about LLC topology is that lie has to be comprehended
> by all OSes and not conflict with other lies people may ask for. In general, I
> think it is preferable and more maintainable to describe the topology as
> accurately and honestly as we can within the existing platform mechanisms (PPTT,
> HMAT, etc) and work on the higher level abstractions to accommodate a broader
> set of topologies as they emerge (as well as working to more fully describe the
> topology with new platform level mechanisms as needed).
>
> As I mentioned, I intend to continue looking in to how to improve the current
> abstractions. For now, it sounds like we have agreement that this patch can be
> merged to address the BUG?
What about swapping the CLS and MC cpumasks for such a machine? This
would avoid that the task scheduler has to deal with a system which has
CLS but no MC. We essentially promote the CLS cpumask up to MC in this
case.
cat /sys/kernel/debug/sched/domains/cpu0/domain*/name
MC
^^
DIE
NUMA
cat /sys/kernel/debug/sched/domains/cpu0# cat domain*/flags
SD_BALANCE_NEWIDLE SD_BALANCE_EXEC SD_BALANCE_FORK SD_WAKE_AFFINE SD_SHARE_PKG_RESOURCES SD_PREFER_SIBLING
^^^^^^^^^^^^^^^^^^^^^^
SD_BALANCE_NEWIDLE SD_BALANCE_EXEC SD_BALANCE_FORK SD_WAKE_AFFINE SD_PREFER_SIBLING
SD_BALANCE_NEWIDLE SD_BALANCE_EXEC SD_BALANCE_FORK SD_WAKE_AFFINE SD_SERIALIZE SD_OVERLAP SD_NUMA
Only very lightly tested on Altra and Juno-r0 (DT).
--->8---
From 54bef59e7f50fa41b7ae39190fd71af57209c27d Mon Sep 17 00:00:00 2001
From: Dietmar Eggemann <dietmar.eggemann@arm.com>
Date: Mon, 14 Mar 2022 15:08:23 +0000
Subject: [PATCH] arch_topology: Swap MC & CLS SD mask if MC weight==1 &
subset(MC,CLS)
This avoids the issue of having a system with a CLS SD but no MC SD.
CLS should be sub-SD of MC.
The cpumask under /sys/devices/system/cpu/cpu*/cache/index* and
/sys/devices/system/cpu/cpu*/topology are not changed by this.
Signed-off-by: Dietmar Eggemann <dietmar.eggemann@arm.com>
---
drivers/base/arch_topology.c | 30 ++++++++++++++++++++++++++++--
1 file changed, 28 insertions(+), 2 deletions(-)
diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
index 976154140f0b..9af90a5625c7 100644
--- a/drivers/base/arch_topology.c
+++ b/drivers/base/arch_topology.c
@@ -614,7 +614,7 @@ static int __init parse_dt_topology(void)
struct cpu_topology cpu_topology[NR_CPUS];
EXPORT_SYMBOL_GPL(cpu_topology);
-const struct cpumask *cpu_coregroup_mask(int cpu)
+const struct cpumask *_cpu_coregroup_mask(int cpu)
{
const cpumask_t *core_mask = cpumask_of_node(cpu_to_node(cpu));
@@ -631,11 +631,37 @@ const struct cpumask *cpu_coregroup_mask(int cpu)
return core_mask;
}
-const struct cpumask *cpu_clustergroup_mask(int cpu)
+const struct cpumask *_cpu_clustergroup_mask(int cpu)
{
return &cpu_topology[cpu].cluster_sibling;
}
+static int
+swap_masks(const cpumask_t *core_mask, const cpumask_t *cluster_mask)
+{
+ if (cpumask_weight(core_mask) == 1 &&
+ cpumask_subset(core_mask, cluster_mask))
+ return 1;
+
+ return 0;
+}
+
+const struct cpumask *cpu_coregroup_mask(int cpu)
+{
+ const cpumask_t *cluster_mask = _cpu_clustergroup_mask(cpu);
+ const cpumask_t *core_mask = _cpu_coregroup_mask(cpu);
+
+ return swap_masks(core_mask, cluster_mask) ? cluster_mask : core_mask;
+}
+
+const struct cpumask *cpu_clustergroup_mask(int cpu)
+{
+ const cpumask_t *cluster_mask = _cpu_clustergroup_mask(cpu);
+ const cpumask_t *core_mask = _cpu_coregroup_mask(cpu);
+
+ return swap_masks(core_mask, cluster_mask) ? core_mask : cluster_mask;
+}
+
void update_siblings_masks(unsigned int cpuid)
{
struct cpu_topology *cpu_topo, *cpuid_topo = &cpu_topology[cpuid];
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2022-03-14 17:13 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-03-04 17:01 [PATCH v3] topology: make core_mask include at least cluster_siblings Darren Hart
2022-03-08 10:30 ` Will Deacon
2022-03-08 10:45 ` Sudeep Holla
2022-03-08 11:04 ` Vincent Guittot
2022-03-08 16:03 ` Dietmar Eggemann
2022-03-08 17:49 ` Darren Hart
2022-03-09 12:50 ` Dietmar Eggemann
2022-03-09 18:26 ` Darren Hart
2022-03-14 9:37 ` Dietmar Eggemann
2022-03-14 16:56 ` Darren Hart
2022-03-16 14:42 ` Dietmar Eggemann
2022-03-14 16:35 ` Dietmar Eggemann [this message]
2022-03-14 16:54 ` Darren Hart
2022-03-16 14:48 ` Dietmar Eggemann
2022-03-16 15:20 ` Darren Hart
2022-03-16 15:55 ` Sudeep Holla
2022-03-21 14:30 ` Will Deacon
2022-03-21 15:56 ` Greg Kroah-Hartman
2022-03-14 21:29 ` [PATCH] arch_topology: Swap MC & CLS SD mask if MC weight==1 & kernel test robot
2022-03-14 23:02 ` kernel test robot
2022-03-17 6:10 ` [PATCH v3] topology: make core_mask include at least cluster_siblings Barry Song
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=9398d7ad-30e7-890a-3e18-c3011c383585@arm.com \
--to=dietmar.eggemann@arm.com \
--cc=Catalin.Marinas@arm.com \
--cc=Valentin.Schneider@arm.com \
--cc=darren@os.amperecomputing.com \
--cc=gregkh@linuxfoundation.org \
--cc=ilkka@os.amperecomputing.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=peterz@infradead.org \
--cc=rafael@kernel.org \
--cc=scott@os.amperecomputing.com \
--cc=song.bao.hua@hisilicon.com \
--cc=stable@vger.kernel.org \
--cc=sudeep.holla@arm.com \
--cc=vincent.guittot@linaro.org \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).