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

Peter Zijlstra
2025-11-03T12:42:57Z

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@linutronix.de>
> ---
> 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
>