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

[Bradley Morgan]

Hi Yury,

Nice. The bool was a lie anyway, nobody reads it.

[...]
> -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 (WARN_ON_ONCE(cpu != smp_processor_id()))
> +		return;

Fine, the silent false was worse. Nit: stop_machine.h never includes
linux/bug.h itself, so this WARN_ON_ONCE only builds through whatever
cpu.h happens to drag in. maybe add the include while youre in here.

[...]
>   * CONTEXT:
>   * Don't care.
> - *
> - * RETURNS:
> - * true if cpu_stop_work was queued successfully and @fn will be called,
> - * false otherwise.
>   */

"Don't care" is stale now. The old contract was: call this from wherever,
worst case you get false back and deal with it. The new contract is that
the caller must keep the target CPU's stopper from getting parked while
it queues, or it eats a WARN. Thats the whole preempt_disable() after
observing the CPU online dance from f0498d2a54e7. The changelog says it,
the comment doesnt, and future callers will read the comment. please put
something like:

 * CONTEXT:
 * Don't care, but the caller must ensure @cpu's stopper stays enabled
 * until the work is queued, e.g. by disabling preemption after having
 * observed @cpu online. See f0498d2a54e7 ("sched: Fix
 * stop_one_cpu_nowait() vs hotplug").

With the CONTEXT bit sorted this looks good to me. The bug.h include
is optional, skip it if you dont care.

Thanks!