[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20251103124254.GA3245006@noisy.programming.kicks-ass.net>
Date: Mon, 3 Nov 2025 13:42:54 +0100
From: Peter Zijlstra <peterz@...radead.org>
To: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
Cc: linux-kernel@...r.kernel.org, Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...nel.org>
Subject: Re: [PATCH] cpu: Make atomic callbacks run on UP with disabled
interrupts
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@...utronix.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
>
Powered by blists - more mailing lists