Messages in this thread | | | Date | Mon, 1 Mar 2010 16:41:44 +0100 | From | Oleg Nesterov <> | Subject | Re: [PATCH 10/43] stop_machine: reimplement without using workqueue |
| |
On 03/02, Tejun Heo wrote: > > Hello, again. > > On 02/28/2010 11:34 PM, Oleg Nesterov wrote: > > On 02/26, Tejun Heo wrote: > >> > >> @@ -164,19 +259,18 @@ int __stop_machine(int (*fn)(void *), void *data, const struct cpumask *cpus) > >> idle.fn = chill; > >> idle.data = NULL; > >> > >> + smp_wmb(); /* -> stop_cpu()::set_current_state() */ > >> ... > >> + for_each_online_cpu(i) > >> + wake_up_process(*per_cpu_ptr(stop_machine_threads, i)); > > > > Afaics, this smp_wmb() is not needed, wake_up_process() (try_to_wake_up) > > should ensure we can't race with set_current_state() + check_condition. > > It does, note the wmb() in try_to_wake_up(). > > Yeap, the initial version was like that and it was awkward to explain > in the comment in stop_cpu() so I basically put it there as a > documentation anchor.
OK,
> Do you think removing it would be better?
No, I just wanted to understand what I have missed. This applies to all my questions in this thread ;)
Thanks,
Oleg.
| |