Re: [RFC][PATCH v2 02/11] stop_machine: Accumulate error code rather than overwrite

[Borislav Petkov]

On Tue, Mar 31, 2026 at 01:42:40AM +0000, Chang S. Bae wrote:
>  static int stop_cpus(const struct cpumask *cpumask, cpu_stop_fn_t fn, void *arg)
>  {
> @@ -512,7 +512,7 @@ static void cpu_stopper_thread(unsigned int cpu)
>  		ret = fn(arg);
>  		if (done) {
>  			if (ret)
> -				done->ret = ret;
> +				done->ret |= ret;

Yeah, can't really do that. Sashiko says:

| Looking at this update to done->ret, is it possible for multiple CPUs to
| write to this shared variable concurrently during a multi-CPU stop operation?
| 
| Since done->ret |= ret is a non-atomic read-modify-write operation, could
| concurrent updates from different cpu_stopper_thread() instances race and
| clobber each other, resulting in lost error bits?
| 
| Additionally, the stop_machine API is a generic mechanism where callbacks
| often return standard negative POSIX error codes.
| 
| If two different CPUs return different negative error codes, for example
| -ENOMEM (-12) and -EIO (-5), won't the bitwise OR result in -EPERM (-1)?
| Does this regression corrupt the final return value for generic callers
| that rely on standard negative error codes?

and I can certainly see it:

__stop_cpus() declares done on its stack and passes it to
queue_stop_cpus_work() which copies the pointer into the cpu_stop_work struct
of every CPU and by the time we end up in the stopper thread, it has been
woken up and we don't take any locks when we run ->fn - we're just
preemption-disabled... So, IINM, we should drop this patch.

Making this work is perhaps too much of an effort with diminishing returns...

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette