* Re: [PATCH v2] stop_machine: Make stop_one_cpu_nowait() return void
2026-07-29 2:23 [PATCH v2] stop_machine: Make stop_one_cpu_nowait() return void Yury Norov
@ 2026-07-29 12:45 ` Bradley Morgan
2026-07-29 12:51 ` Peter Zijlstra
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Bradley Morgan @ 2026-07-29 12:45 UTC (permalink / raw)
To: Yury Norov, linux-kernel, Oleg Nesterov, Paul E . McKenney,
Peter Zijlstra, Phil Auld, Sebastian Andrzej Siewior,
Shrikanth Hegde, Tejun Heo, Thomas Weißschuh,
Valentin Schneider
Cc: Yury Norov
On 29 July 2026 03:23:54 BST, Yury Norov <ynorov@nvidia.com> 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@grrlz.net>
Reviewed-by: Bradley Morgan <include@grrlz.net>
>Signed-off-by: Yury Norov <ynorov@nvidia.com>
>---
>v1: https://lore.kernel.org/all/20260724232325.594212-1-ynorov@nvidia.com/
>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@kernel.org>
> */
>+#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!
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] stop_machine: Make stop_one_cpu_nowait() return void
2026-07-29 2:23 [PATCH v2] stop_machine: Make stop_one_cpu_nowait() return void Yury Norov
2026-07-29 12:45 ` Bradley Morgan
@ 2026-07-29 12:51 ` Peter Zijlstra
2026-07-29 13:40 ` Yury Norov
2026-07-29 15:30 ` Shrikanth Hegde
2026-08-03 7:48 ` [tip: sched/core] " tip-bot2 for Yury Norov
3 siblings, 1 reply; 6+ messages in thread
From: Peter Zijlstra @ 2026-07-29 12:51 UTC (permalink / raw)
To: Yury Norov
Cc: linux-kernel, Oleg Nesterov, Paul E . McKenney, Phil Auld,
Sebastian Andrzej Siewior, Shrikanth Hegde, Tejun Heo,
Thomas Weißschuh, Valentin Schneider, Yury Norov,
Bradley Morgan
On Tue, Jul 28, 2026 at 10:23:54PM -0400, Yury Norov 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@grrlz.net>
> Signed-off-by: Yury Norov <ynorov@nvidia.com>
I suppose; you want me to take this through the scheduler tree?
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] stop_machine: Make stop_one_cpu_nowait() return void
2026-07-29 12:51 ` Peter Zijlstra
@ 2026-07-29 13:40 ` Yury Norov
0 siblings, 0 replies; 6+ messages in thread
From: Yury Norov @ 2026-07-29 13:40 UTC (permalink / raw)
To: Peter Zijlstra
Cc: linux-kernel, Oleg Nesterov, Paul E . McKenney, Phil Auld,
Sebastian Andrzej Siewior, Shrikanth Hegde, Tejun Heo,
Thomas Weißschuh, Valentin Schneider, Yury Norov,
Bradley Morgan
On Wed, Jul 29, 2026 at 02:51:33PM +0200, Peter Zijlstra wrote:
> On Tue, Jul 28, 2026 at 10:23:54PM -0400, Yury Norov 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@grrlz.net>
> > Signed-off-by: Yury Norov <ynorov@nvidia.com>
>
> I suppose; you want me to take this through the scheduler tree?
Sure, thanks!
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] stop_machine: Make stop_one_cpu_nowait() return void
2026-07-29 2:23 [PATCH v2] stop_machine: Make stop_one_cpu_nowait() return void Yury Norov
2026-07-29 12:45 ` Bradley Morgan
2026-07-29 12:51 ` Peter Zijlstra
@ 2026-07-29 15:30 ` Shrikanth Hegde
2026-08-03 7:48 ` [tip: sched/core] " tip-bot2 for Yury Norov
3 siblings, 0 replies; 6+ messages in thread
From: Shrikanth Hegde @ 2026-07-29 15:30 UTC (permalink / raw)
To: Yury Norov, Yury Norov
Cc: Bradley Morgan, linux-kernel, Oleg Nesterov, Paul E . McKenney,
Peter Zijlstra, Phil Auld, Sebastian Andrzej Siewior, Tejun Heo,
Thomas Weißschuh, Valentin Schneider
Hi Yury.
On 7/29/26 7:53 AM, Yury Norov 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@grrlz.net>
> Signed-off-by: Yury Norov <ynorov@nvidia.com>
> ---
> v1: https://lore.kernel.org/all/20260724232325.594212-1-ynorov@nvidia.com/
> 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@kernel.org>
> */
> +#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().
nit:
* until the work is queued, i.e. ensuring preemption/irq disabled. ?
Because, if irq are disabled it is equally good enough.
#define preemptible() (preempt_count() == 0 && !irqs_disabled())
> */
> -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,
Other than that,
Reviewed-by: Shrikanth Hegde <sshegde@linux.ibm.com>
^ permalink raw reply [flat|nested] 6+ messages in thread* [tip: sched/core] stop_machine: Make stop_one_cpu_nowait() return void
2026-07-29 2:23 [PATCH v2] stop_machine: Make stop_one_cpu_nowait() return void Yury Norov
` (2 preceding siblings ...)
2026-07-29 15:30 ` Shrikanth Hegde
@ 2026-08-03 7:48 ` tip-bot2 for Yury Norov
3 siblings, 0 replies; 6+ messages in thread
From: tip-bot2 for Yury Norov @ 2026-08-03 7:48 UTC (permalink / raw)
To: linux-tip-commits
Cc: Yury Norov, Peter Zijlstra (Intel), Bradley Morgan,
Shrikanth Hegde, x86, linux-kernel
The following commit has been merged into the sched/core branch of tip:
Commit-ID: 5186ef36909c792591fda7a1ae2afc168f60bc90
Gitweb: https://git.kernel.org/tip/5186ef36909c792591fda7a1ae2afc168f60bc90
Author: Yury Norov <ynorov@nvidia.com>
AuthorDate: Tue, 28 Jul 2026 22:23:54 -04:00
Committer: Peter Zijlstra <peterz@infradead.org>
CommitterDate: Fri, 31 Jul 2026 12:35:26 +02:00
stop_machine: Make stop_one_cpu_nowait() return void
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.
Signed-off-by: Yury Norov <ynorov@nvidia.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Bradley Morgan <include@grrlz.net>
Reviewed-by: Shrikanth Hegde <sshegde@linux.ibm.com>
Link: https://patch.msgid.link/20260729022355.325058-1-ynorov@nvidia.com
---
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 0101111..84e7fb6 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 773d8e9..d085ba1 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@kernel.org>
*/
+#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,
^ permalink raw reply related [flat|nested] 6+ messages in thread