Messages in this thread | | | From | Nadav Amit <> | Subject | Re: [PATCH v3 1/9] smp: Run functions concurrently in smp_call_function_many() | Date | Mon, 22 Jul 2019 18:41:44 +0000 |
| |
> On Jul 22, 2019, at 11:16 AM, Peter Zijlstra <peterz@infradead.org> wrote: > > On Fri, Jul 19, 2019 at 11:23:06AM -0700, Dave Hansen wrote: >> On 7/18/19 5:58 PM, Nadav Amit wrote: >>> @@ -624,16 +622,11 @@ EXPORT_SYMBOL(on_each_cpu); >>> void on_each_cpu_mask(const struct cpumask *mask, smp_call_func_t func, >>> void *info, bool wait) >>> { >>> - int cpu = get_cpu(); >>> + preempt_disable(); >>> >>> - smp_call_function_many(mask, func, info, wait); >>> - if (cpumask_test_cpu(cpu, mask)) { >>> - unsigned long flags; >>> - local_irq_save(flags); >>> - func(info); >>> - local_irq_restore(flags); >>> - } >>> - put_cpu(); >>> + __smp_call_function_many(mask, func, func, info, wait); >>> + >>> + preempt_enable(); >>> } >> >> The get_cpu() was missing it too, but it would be nice to add some >> comments about why preempt needs to be off. I was also thinking it >> might make sense to do: >> >> cfd = get_cpu_var(cfd_data); >> __smp_call_function_many(cfd, ...); >> put_cpu_var(cfd_data); >> >> instead of the explicit preempt_enable/disable(), but I don't feel too >> strongly about it. > > It is also required for cpu hotplug.
But then smpcfd_dead_cpu() will not respect the “cpu” argument. Do you still prefer it this way (instead of the current preempt_enable() / preempt_disable())? | |