Re: [PATCH v2] stop_machine: Make stop_one_cpu_nowait() return void

[Bradley Morgan]

On 29 July 2026 03:23:54 BST, Yury Norov <ynorov@xxxxxxxxxx> wrote:
>No caller checks the return value from stop_one_cpu_nowait(). All
>callers require the callback to run and arrange for the target CPU's
>stopper to remain enabled while queuing the work. In particular, commit
>f0498d2a54e7 ("sched: Fix stop_one_cpu_nowait() vs hotplug") added
>preemption protection to the scheduler callers so that queuing must
>succeed once the target CPU has been observed online.
>
>Therefore, a failure is an unrecoverable violation rather than a condition
>individual callers can recover from. Diagnose it with WARN_ON_ONCE() in
>stop_one_cpu_nowait(). A check in the common helper covers current and
>future callers consistently, while individual checks would duplicate
>the same non-recoverable handling at every call site.
>
>Make the function return void because there is no longer a meaningful
>result for callers to consume.
>
>On UP, warn if the supplied CPU is not the current CPU because the work
>cannot be scheduled in that case.
>
>CC: Bradley Morgan <include@xxxxxxxxx>
Reviewed-by: Bradley Morgan <include@xxxxxxxxx>
>Signed-off-by: Yury Norov <ynorov@xxxxxxxxxx>
>---
>v1: https://lore.kernel.org/all/20260724232325.594212-1-ynorov@xxxxxxxxxx/
>v2:
> - carefully include linux/bug.h (Bradley);
> - update the CONTEXT section (Bradley).
>
> include/linux/stop_machine.h | 21 ++++++++++-----------
> kernel/stop_machine.c        | 14 ++++++--------
> 2 files changed, 16 insertions(+), 19 deletions(-)
>
>diff --git a/include/linux/stop_machine.h b/include/linux/stop_machine.h
>index 01011113d226..84e7fb627ba4 100644
>--- a/include/linux/stop_machine.h
>+++ b/include/linux/stop_machine.h
>@@ -2,6 +2,7 @@
> #ifndef _LINUX_STOP_MACHINE
> #define _LINUX_STOP_MACHINE
> 
>+#include <linux/bug.h>
> #include <linux/cpu.h>
> #include <linux/cpumask_types.h>
> #include <linux/smp.h>
>@@ -31,7 +32,7 @@ struct cpu_stop_work {
> 
> int stop_one_cpu(unsigned int cpu, cpu_stop_fn_t fn, void *arg);
> int stop_two_cpus(unsigned int cpu1, unsigned int cpu2, cpu_stop_fn_t fn,
> void *arg);
>-bool stop_one_cpu_nowait(unsigned int cpu, cpu_stop_fn_t fn, void *arg,
>+void stop_one_cpu_nowait(unsigned int cpu, cpu_stop_fn_t fn, void *arg,
> 			 struct cpu_stop_work *work_buf);
> void stop_machine_park(int cpu);
> void stop_machine_unpark(int cpu);
>@@ -68,19 +69,17 @@ static void stop_one_cpu_nowait_workfn(struct work_struct *work)
> 	preempt_enable();
> }
> 
>-static inline bool stop_one_cpu_nowait(unsigned int cpu,
>+static inline void stop_one_cpu_nowait(unsigned int cpu,
> 				       cpu_stop_fn_t fn, void *arg,
> 				       struct cpu_stop_work *work_buf)
> {
>-	if (cpu == smp_processor_id()) {
>-		INIT_WORK(&work_buf->work, stop_one_cpu_nowait_workfn);
>-		work_buf->fn = fn;
>-		work_buf->arg = arg;
>-		schedule_work(&work_buf->work);
>-		return true;
>-	}
>-
>-	return false;
>+	if (WARN_ON_ONCE(cpu != smp_processor_id()))
>+		return;
>+
>+	INIT_WORK(&work_buf->work, stop_one_cpu_nowait_workfn);
>+	work_buf->fn = fn;
>+	work_buf->arg = arg;
>+	schedule_work(&work_buf->work);
> }
> 
> static inline void print_stop_info(const char *log_lvl, struct
> task_struct *task) { }
>diff --git a/kernel/stop_machine.c b/kernel/stop_machine.c
>index 773d8e9ae30c..d085ba1f4b44 100644
>--- a/kernel/stop_machine.c
>+++ b/kernel/stop_machine.c
>@@ -7,6 +7,7 @@
>  * Copyright (C) 2010		SUSE Linux Products GmbH
>  * Copyright (C) 2010		Tejun Heo <tj@xxxxxxxxxx>
>  */
>+#include <linux/bug.h>
> #include <linux/compiler.h>
> #include <linux/completion.h>
> #include <linux/cpu.h>
>@@ -376,17 +377,14 @@ int stop_two_cpus(unsigned int cpu1, unsigned int cpu2, cpu_stop_fn_t fn, void *
>  * and will remain untouched until stopper starts executing @fn.
>  *
>  * CONTEXT:
>- * Don't care.
>- *
>- * RETURNS:
>- * true if cpu_stop_work was queued successfully and @fn will be called,
>- * false otherwise.
>+ * Don't care, but the caller must ensure @cpu's stopper stays enabled
>+ * until the work is queued, e.g. by preempt_disable().
>  */
>-bool stop_one_cpu_nowait(unsigned int cpu, cpu_stop_fn_t fn, void *arg,
>-			struct cpu_stop_work *work_buf)
>+void stop_one_cpu_nowait(unsigned int cpu, cpu_stop_fn_t fn, void *arg,
>+			 struct cpu_stop_work *work_buf)
> {
> 	*work_buf = (struct cpu_stop_work){ .fn = fn, .arg = arg, .caller = _RET_IP_, };
>-	return cpu_stop_queue_work(cpu, work_buf);
>+	WARN_ON_ONCE(!cpu_stop_queue_work(cpu, work_buf));
> }
> 
> static bool queue_stop_cpus_work(const struct cpumask *cpumask,
>


Thanks!