RE: [PATCH] cpu-topology: Skip the exist but not possible cpu nodes

From: Zengtao (B)
Date: Mon Jan 06 2020 - 20:36:11 EST


> -----Original Message-----
> From: Dietmar Eggemann [mailto:dietmar.eggemann@xxxxxxx]
> Sent: Tuesday, January 07, 2020 2:42 AM
> To: Zengtao (B); sudeep.holla@xxxxxxx
> Cc: Linuxarm; Greg Kroah-Hartman; Rafael J. Wysocki;
> linux-kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH] cpu-topology: Skip the exist but not possible cpu
> nodes
>
> On 02/01/2020 04:24, Zeng Tao wrote:
> > When CONFIG_NR_CPUS is smaller than the cpu nodes defined in the
> device
> > tree, the cpu node parsing will fail. And this is not reasonable for a
> > legal device tree configs.
> > In this patch, skip such cpu nodes rather than return an error.
>
> Is this extra code really necessary?
>
> Currently you get warnings indicating that CONFIG_NR_CPUS is too small
> so you could correct the setup issue easily.
>

Not only about warning messages, the problem is :
What we are expected to do if the CONFIG_NR_CPUS is too small? I think there
are two choices:
1. Keep the dts parsing result, but skip the the CPU nodes whose id exceeds the
the CONFIG_NR_CPUS, and this is what this patch do.
2. Just abort all the CPU nodes parsing, and using MPIDR to guess the topology,
and this is what the current code do.
And i think choice 1 is better because:
1. It's a legal dts, we should keep the same result whether CONFIG_NR_CPUS is
too small or not.
2. In the function of_parse_and_init_cpus, we just do the same way as choice 1.

But i am open for the issue, any suggestions are welcomed.

Thanks
Zengtao

> Example: Arm64 Juno board
>
> $ grep "cpu@" ./arch/arm64/boot/dts/arm/juno.dts
> A57_0: cpu@0 {
> A57_1: cpu@1 {
> A53_0: cpu@100 {
> A53_1: cpu@101 {
> A53_2: cpu@102 {
> A53_3: cpu@103 {
>
> root@juno:~# uname -r
> 5.5.0-rc5
>
> root@juno:~# zcat /proc/config.gz | grep CONFIG_NR_CPUS
> CONFIG_NR_CPUS=4
>
> root@juno:~# cat /proc/cpuinfo | grep ^proc
> processor : 0
> processor : 1
> processor : 2
> processor : 3
>
> root@juno:~# dmesg | grep "Unable\|Can't"
> [ 0.085089] Unable to find CPU node for /cpus/cpu@102
> [ 0.090179] /cpus/cpu-map/cluster1/core2: Can't get CPU for leaf core
>
> [...]