2022-08-23 09:00:11

by Ionela Voinescu

[permalink] [raw]
Subject: Re: [PATCH] arch_topology: Make cluster topology span at least SMT CPUs

Hi Yicong,

On Tuesday 23 Aug 2022 at 15:30:44 (+0800), Yicong Yang wrote:
> From: Yicong Yang <[email protected]>
>
> Currently cpu_clustergroup_mask() will return CPU mask if cluster span
> more or the same CPUs as cpu_coregroup_mask(). This will result topology
> borken on non-Cluster SMT machines.

Might be worth adding here:.. "when building with CONFIG_SCHED_CLUSTER=y"

>

I thought I had covered this case, but I think I had considered LLC
spanning more than one core (tested on TX2 as well).

So you'd only hit this if LLC and cluster level span the same cores (a
single core in this case, for non-cluster), in the presence of SMT.

Reviewed-by: Ionela Voinescu <[email protected]>

Many thanks,
Ionela.

> Test with:
> qemu-system-aarch64 -enable-kvm -machine virt \
> -net none \
> -cpu host \
> -bios ./QEMU_EFI.fd \
> -m 2G \
> -smp 48,sockets=2,cores=12,threads=2 \
> -kernel $Image \
> -initrd $Rootfs \
> -nographic
> -append "rdinit=init console=ttyAMA0 sched_verbose loglevel=8"
>
> We'll get below error:
> [ 3.084568] BUG: arch topology borken
> [ 3.084570] the SMT domain not a subset of the CLS domain
>
> Since cluster is a level higher than SMT, fix this by making cluster
> spans at least SMT CPUs.
>
> Cc: Sudeep Holla <[email protected]>
> Cc: Vincent Guittot <[email protected]>
> Cc: Ionela Voinescu <[email protected]>
> Fixes: bfcc4397435d ("arch_topology: Limit span of cpu_clustergroup_mask()")
> Signed-off-by: Yicong Yang <[email protected]>
> ---
> drivers/base/arch_topology.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
> index 0424b59b695e..0056a1273275 100644
> --- a/drivers/base/arch_topology.c
> +++ b/drivers/base/arch_topology.c
> @@ -724,7 +724,7 @@ const struct cpumask *cpu_clustergroup_mask(int cpu)
> */
> if (cpumask_subset(cpu_coregroup_mask(cpu),
> &cpu_topology[cpu].cluster_sibling))
> - return get_cpu_mask(cpu);
> + return cpu_smt_mask(cpu);
>
> return &cpu_topology[cpu].cluster_sibling;
> }
> --
> 2.24.0
>