Re: [RFC PATCH 5/5] powerpc/smp: Remap boot CPU onto core 0 if >= nr_cpu_ids

[Jiri Bohac <jbohac@suse.cz>]

On Tue, Jan 02, 2024 at 10:16:04AM +0530, Aneesh Kumar K.V wrote:
> Michael Ellerman <mpe@ellerman.id.au> writes:
> 
> ....
> 
> >  #ifdef CONFIG_PPC64
> >  int boot_cpu_hwid = -1;
> > @@ -492,12 +493,26 @@ void __init smp_setup_cpu_maps(void)
> >  			avail = !of_property_match_string(dn,
> >  					"enable-method", "spin-table");
> >  
> > -		cpu = assign_threads(cpu, nthreads, avail, intserv);
> > +		if (boot_core_hwid >= 0) {
> > +			if (cpu == 0) {
> > +				pr_info("Skipping CPU node %pOF to allow for boot core.\n", dn);
> > +				cpu = nthreads;
> > +				continue;
> > +			}
> >  
> > -		if (cpu >= nr_cpu_ids) {
> > +			if (be32_to_cpu(intserv[0]) == boot_core_hwid) {
> > +				pr_info("Renumbered boot core %pOF to logical 0\n", dn);
> > +				assign_threads(0, nthreads, avail, intserv);
> > +				of_node_put(dn);
> > +				break;
> >
> 
> I was expecting a 'continue' here. Why 'break' the loop? The condition that
> should break the loop should be cpu >= nr_cpu_ids 

No, the patch seems correct:

We're in the "if (boot_core_hwid >= 0)" branch, meaning that it
was determined by early_init_dt_scan_cpus() that boot_cpuid >=
nr_cpu_ids. So we loop until we get to the boot CPU, so it can be
renumbered to 0. Once we do that we break, because we
know we are already past nr_cpu_ids - otherwise boot_core_hwid
would not be >= 0. 


> > +			}
> > +		} else if (cpu >= nr_cpu_ids) {
> >  			of_node_put(dn);
> >  			break;
> >  		}

Here is what you expected - in case the boot CPU was < nr_cpu_ids
we break as soon as nr_cpu_ids is reached.

> > +
> > +		if (cpu < nr_cpu_ids)

this ensures that CPUs between nr_cpu_ids and the boot CPU are
correctly ignored in case we're already past nr_cpu_ids and only
scanning further to find the boot CPU to be renumbered to 0

-- 
Jiri Bohac <jbohac@suse.cz>
SUSE Labs, Prague, Czechia