Re: [PATCH] cpu: Make atomic callbacks run on UP with disabled interrupts

[Peter Zijlstra]

On Mon, Nov 03, 2025 at 01:03:54PM +0100, Sebastian Andrzej Siewior wrote:
> On SMP callbacks in the "starting" range are invoked while the CPU is
> brought up and interrupts are still disabled. Callbacks which are added
> later ar invoked via the hotplug-thread on the target CPU and interrupts
> are explicitly disabled.
> In the UP case callbacks which are added later are invoked "directly"
> without the thread. This is okay since there is just one CPU but with
> enabled interrupts debug code, such as smp_processor_id(), will issue
> warnings.
> 
> Disable interrupts before invoking the calback on UP if the state is
> atomic and interrupts are expected to be disabled.
> The "save" part is required because this is also invoked early in the
> boot process while interrupts are disabled and must not be enabled.
> 
> Fixes: 06ddd17521bf1 ("sched/smp: Always define is_percpu_thread() and scheduler_ipi()")
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>
> ---
>  kernel/cpu.c | 26 +++++++++++++++++---------
>  1 file changed, 17 insertions(+), 9 deletions(-)
> 
> diff --git a/kernel/cpu.c b/kernel/cpu.c
> index db9f6c539b28c..a6902646b4933 100644
> --- a/kernel/cpu.c
> +++ b/kernel/cpu.c
> @@ -249,6 +249,14 @@ static int cpuhp_invoke_callback(unsigned int cpu, enum cpuhp_state state,
>  	return ret;
>  }
>  
> +/*
> + * The former STARTING/DYING states, ran with IRQs disabled and must not fail.
> + */
> +static bool cpuhp_is_atomic_state(enum cpuhp_state state)
> +{
> +	return CPUHP_AP_IDLE_DEAD <= state && state < CPUHP_AP_ONLINE;
> +}
> +
>  #ifdef CONFIG_SMP
>  static bool cpuhp_is_ap_state(enum cpuhp_state state)
>  {
> @@ -271,14 +279,6 @@ static inline void complete_ap_thread(struct cpuhp_cpu_state *st, bool bringup)
>  	complete(done);
>  }
>  
> -/*
> - * The former STARTING/DYING states, ran with IRQs disabled and must not fail.
> - */
> -static bool cpuhp_is_atomic_state(enum cpuhp_state state)
> -{
> -	return CPUHP_AP_IDLE_DEAD <= state && state < CPUHP_AP_ONLINE;
> -}
> -
>  /* Synchronization state management */
>  enum cpuhp_sync_state {
>  	SYNC_STATE_DEAD,
> @@ -2364,7 +2364,15 @@ static int cpuhp_issue_call(int cpu, enum cpuhp_state state, bool bringup,
>  	else
>  		ret = cpuhp_invoke_callback(cpu, state, bringup, node, NULL);
>  #else
> -	ret = cpuhp_invoke_callback(cpu, state, bringup, node, NULL);
> +	if (1) {
> +		unsigned long flags = 0;
> +
> +		if (cpuhp_is_atomic_state(state))
> +			local_irq_save(flags);
> +		ret = cpuhp_invoke_callback(cpu, state, bringup, node, NULL);
> +		if (cpuhp_is_atomic_state(state))
> +			local_irq_restore(flags);
> +	}

How about:

	if (cpuhp_is_atomic_state(state)) {
		guard(irqsave)();
		ret = cpuhp_invoke_callback(cpu, state, bringup, node, NULL);
		/*
		 * STARTING/DYING must not fail!
		 */
		WARN_ON_ONCE(ret);
	} else {
		ret = cpuhp_invoke_callback(cpu, state, bringup, node, NULL);
	}

which is a little more like cpuhp_thread_fun()


>  #endif
>  	BUG_ON(ret && !bringup);
>  	return ret;
> -- 
> 2.51.0
>