Messages in this thread | | | From | Rusty Russell <> | Subject | Re: [PATCH] stopmachine: add stopmachine_timeout | Date | Tue, 15 Jul 2008 17:50:11 +1000 |
| |
On Tuesday 15 July 2008 11:11:34 Hidetoshi Seto wrote: > Hi Rusty,
Hi Hidetoshi,
> However we need to be careful that the stuck CPU can restart unexpectedly.
OK, if you are worried about that race, I think we can still fix it...
> > + for_each_online_cpu(i) { > > + if (!cpu_isset(i, prepared_cpus) && i != smp_processor_id()) { > > + bool ignore; > > + > > Note that here is a window that a not-prepared frozen cpu can be thawed and > become be prepared. > > > + /* If we wanted to run on a particular CPU, and that's > > + * the one which is stuck, it's a real failure. */ > > + ignore = !cpus || !cpu_isset(i, *cpus); > > + printk(KERN_CRIT "stopmachine: cpu#%d seems to be " > > + "stuck, %s.\n", > > + i, ignore ? "ignoring" : "FAILING"); > > + /* Unbind thread: it will exit when it sees > > + * that prepared_cpus bit set. */ > > + set_cpus_allowed(threads[i], cpu_online_map); > > Unbinded threads still can wake up on a cpu where they originally targeted.
What if we use: if (i != smp_processor_id() && !cpu_test_and_set(i, prepared_cpus)) {
instead of cpu_isset? That means that if a CPU restarts during that window, either the thread will exit (because we set the bit here), or this will detect it.
Hmm, there's still the vague possibility that the thread doesn't schedule until we start a new stop_machine (and clear prepared_cpus). We could simply loop in the main thread if any threads are alive, before freeing them (inside the lock). A counter and notifier is the other way, but it seems like overkill for a very unlikely event.
Thanks for the analysis! Rusty.
| |