Re: [PATCH 18/19] arm64: smp: Use generic HOTPLUG_PARALLEL machinery for CPU onlining

[Will Deacon]

On Tue, Sep 08, 2026 at 09:05:18PM +0800, Jinjie Ruan wrote:
> 在 2026/9/8 0:40, Will Deacon 写道:
> > diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> > index 17868b497d7c..bec4bc1b12db 100644
> > --- a/arch/arm64/kernel/head.S
> > +++ b/arch/arm64/kernel/head.S
> > @@ -393,7 +393,6 @@ SYM_FUNC_START_LOCAL(__secondary_switched)
> >  	mov	x0, x20
> >  	bl	finalise_el2
> >  
> > -	str_l	xzr, __early_cpu_boot_status, x3
> >  	adr_l	x5, vectors
> >  	msr	vbar_el1, x5
> >  	isb
> > @@ -439,15 +438,15 @@ SYM_FUNC_END(set_cpu_boot_mode_flag)
> >   * with MMU turned off.
> >   *
> >   * update_early_cpu_boot_status tmp, status
> > - *  - Corrupts tmp1, tmp2
> > - *  - Writes 'status' to __early_cpu_boot_status and makes sure
> > + *  - Corrupts tmp1
> 
> Corrupts tmp1, tmp2 ?

Well spotted, thanks.

> > @@ -125,45 +135,42 @@ int arch_cpuhp_kick_ap_alive(unsigned int cpu, struct task_struct *idle)
> >  
> >  void arch_cpuhp_cleanup_kick_cpu(unsigned int cpu, bool is_alive)
> >  {
> > -	long status;
> > +	union secondary_status status;
> >  
> >  	if (is_alive)
> >  		return;
> >  
> > -	secondary_data.task = NULL;
> > -	status = READ_ONCE(secondary_data.status);
> > -	if (status == CPU_MMU_OFF)
> > -		status = READ_ONCE(__early_cpu_boot_status);
> > -
> >  	/* A CPU has failed to boot. Try to figure out what happened. */
> > -	switch (status & CPU_BOOT_STATUS_MASK) {
> > -	default:
> > -		pr_err("CPU%u: failed in unknown state : 0x%lx\n",
> > -		       cpu, status);
> > -		cpus_stuck_in_kernel++;
> > -		break;
> > -	case CPU_KILL_ME:
> > -		if (cpumask_test_cpu(cpu, &secondary_data.cpu_died_early_mask))
> > -			set_cpu_present(cpu, false);
> > +	if (smp_parallel_bringup)
> > +		pr_warn_once("Parallel CPU bringup failed; consider passing \"cpuhp.parallel=off\" for a more accurate diagnosis.\n");
> 
> For some production systems, restarting to reproduce the issue may be
> troublesome.

These errors _really_ shouldn't happen with production systems. They are
caused by critical, deterministic errors such as the secondary CPU not
supporting the system page size. If you can't reboot in that situation,
then you have no system!

The parallel bringup code will detect the issue, it just won't be able
to tell you which CPU caused which issue.

> > +	if (cpumask_test_cpu(cpu, &secondary_data.cpu_died_early_mask)) {
> > +		set_cpu_present(cpu, false);
> >  		if (!op_cpu_kill(cpu)) {
> >  			pr_crit("CPU%u: died during early boot\n", cpu);
> > -			break;
> > +			return;
> >  		}
> > -		pr_crit("CPU%u: may not have shut down cleanly\n", cpu);
> > -		fallthrough;
> > -	case CPU_STUCK_IN_KERNEL:
> > -		pr_crit("CPU%u: is stuck in kernel\n", cpu);
> > -		if (status & CPU_STUCK_REASON_52_BIT_VA)
> > -			pr_crit("CPU%u: does not support 52-bit VAs\n", cpu);
> > -		if (status & CPU_STUCK_REASON_NO_GRAN) {
> > -			pr_crit("CPU%u: does not support %luK granule\n",
> > -				cpu, PAGE_SIZE / SZ_1K);
> > -		}
> > -		cpus_stuck_in_kernel++;
> > -		break;
> > -	case CPU_PANIC_KERNEL:
> > -		panic("CPU%u detected unsupported configuration\n", cpu);
> >  	}
> > +
> > +	pr_crit_once("CPUs may be stuck in kernel\n");
> 
> Need we print the "cpu"

You should already get something like:

  CPUn: will not boot

so I don't think we need anything extra (as, as above, we don't know
exactly which CPUs are stuck).

> >  static void init_gic_priority_masking(void)
> > @@ -407,12 +414,8 @@ void __noreturn cpu_die_early(void)
> >  
> >  	cpumask_set_cpu(cpu, &secondary_data.cpu_died_early_mask);
> 
> It seems unsafe for multiple secondary CPUs to update
> cpu_died_early_mask concurrently.

Why? cpumask_set_cpu() is atomic and each CPU only sets their own bit.

Will