[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20220513090330.z25fwthekn4rjkwq@bogus>
Date: Fri, 13 May 2022 10:03:30 +0100
From: Sudeep Holla <sudeep.holla@....com>
To: Dietmar Eggemann <dietmar.eggemann@....com>
Cc: Qing Wang <wangqing@...o.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Sudeep Holla <sudeep.holla@....com>,
"Rafael J . Wysocki" <rafael@...nel.org>,
Vincent Guittot <vincent.guittot@...aro.org>,
Morten Rasmussen <morten.rasmussen@....com>,
linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH] arch_topology: Use llc_id instead of package_id
On Fri, May 13, 2022 at 10:34:00AM +0200, Dietmar Eggemann wrote:
> package_id should represent socket in DT. Free it for possible socket
> dts parsing and use the so far unused llc_id / llc_sibling cpumask to
> decode 1. level clusters used in pre-DynamIQ big/little systems.
>
> cpu_coregroup_mask() will return llc_sibling isntead of core_mask in
> this case.
>
> This will let the cluster_id / cluster_sibling cpumask be available for
> 2. level clusters (e.g. to map L2 sharing in Armv9 A510 complexes).
>
> The corresponding sched domain CLS is similarly used in ACPI (servers)
> to map e.g. Kunpeng920 L3-tags or Ampere Altra's SCU bounderies.
>
> Lighlty tested on qemu-system-aarch64 with 1x8 (cluster-x-core) and
> 2x4 cpu-map.
>
> Signed-off-by: Dietmar Eggemann <dietmar.eggemann@....com>
> ---
>
> Related to: https://lkml.kernel.org/r/1652361692-13196-1-git-send-email-wangqing@vivo.com
>
> drivers/base/arch_topology.c | 24 ++++++++++++------------
> 1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
> index f73b836047cf..ac1488990cc8 100644
> --- a/drivers/base/arch_topology.c
> +++ b/drivers/base/arch_topology.c
> @@ -490,7 +490,7 @@ static int __init get_cpu_for_node(struct device_node *node)
> return cpu;
> }
>
> -static int __init parse_core(struct device_node *core, int package_id,
> +static int __init parse_core(struct device_node *core, int llc_id,
> int core_id)
> {
> char name[20];
> @@ -506,7 +506,8 @@ static int __init parse_core(struct device_node *core, int package_id,
> leaf = false;
> cpu = get_cpu_for_node(t);
> if (cpu >= 0) {
> - cpu_topology[cpu].package_id = package_id;
> + cpu_topology[cpu].package_id = 0;
> + cpu_topology[cpu].llc_id = llc_id;
> cpu_topology[cpu].core_id = core_id;
> cpu_topology[cpu].thread_id = i;
> } else if (cpu != -ENODEV) {
> @@ -527,7 +528,8 @@ static int __init parse_core(struct device_node *core, int package_id,
> return -EINVAL;
> }
>
> - cpu_topology[cpu].package_id = package_id;
> + cpu_topology[cpu].package_id = 0;
While the above looks good and matches with what I am attempting to do
as well ...
> + cpu_topology[cpu].llc_id = llc_id;
This looks wrong for simple reason that this is derived incorrectly from
the cpu-map while there is no guarantee that it matches the last level
cache ID on the system as we didn't parse the cache topology for this.
So I disagree with this change as it might conflict with the actual and
correct llc_id.
--
Regards,
Sudeep
Powered by blists - more mailing lists