> On Jul 25, 2019, at 5:36 AM, Thomas Gleixner <t...@linutronix.de> wrote: > > On Mon, 22 Jul 2019, Nadav Amit wrote: >>> On Jul 22, 2019, at 11:51 AM, Thomas Gleixner <t...@linutronix.de> wrote: >>> void on_each_cpu(void (*func) (void *info), void *info, int wait) >>> { >>> unsigned long flags; >>> >>> preempt_disable(); >>> smp_call_function(func, info, wait); >>> >>> smp_call_function() has another preempt_disable as it can be called >>> separately and it does: >>> >>> preempt_disable(); >>> smp_call_function_many(cpu_online_mask, func, info, wait); >>> >>> Your new on_each_cpu() implementation does not. So there is a >>> difference. Whether it matters or not is a different question, but that >>> needs to be explained and documented. >> >> Thanks for explaining - so your concern is for CPUs being offlined. >> >> But unless I am missing something: on_each_cpu() calls __on_each_cpu_mask(), >> which disables preemption and calls __smp_call_function_many(). >> >> Then __smp_call_function_many() runs: >> >> cpumask_and(cfd->cpumask, mask, cpu_online_mask); >> >> … before choosing which remote CPUs should run the function. So the only >> case that I was missing is if the current CPU goes away and the function is >> called locally. >> >> Can it happen? I can add documentation and a debug assertion for this case. > > I don't think it can happen: > > on_each_cpu() > on_each_cpu_mask(....) > preempt_disable() > __smp_call_function_many() > > So if a CPU goes offline between on_each_cpu() and preempt_disable() then > there is no damage. After the preempt_disable() it can't go away anymore > and the task executing this cannot be migrated either. > > So yes, it's safe, but please add a big fat comment so future readers won't > be puzzled.
I will do. I will need some more time to respin the next version. I see that what I build on top of it might require some changes, and I want to minimize them.