Messages in this thread | | | Date | Mon, 21 Jun 2010 20:00:50 +0200 | From | Oleg Nesterov <> | Subject | Re: while_each_thread() under rcu_read_lock() is broken? |
| |
On 06/21, Oleg Nesterov wrote: > > On 06/21, Oleg Nesterov wrote: > > > > So, I am thinking about the first attempt > > > > #define while_each_thread(g, t) \ > > while ((t = next_thread(t)) != g && pid_alive(g)) > > > > again. But this means while_each_thread() can miss more threads > > than it currently can under the same conditions. Correct, but > > not good. > > Not good, but correct ;) Probably it makes sense to fix the problem > anyway, then think about the more optimal fix. > > static inline struct task_struct * > next_thread_careful(const struct task_struct *g, const struct task_struct *t) > { > t = next_thread(t); > /* > * this pairs with the implicit barrier between detach_pid() > * and list_del_rcu(g->thread_group) in __unhash_process(g). > */ > smp_rmb(); > if (likely(pid_alive(g))) > return t; > else > return g; > } > > #define while_each_thread(g, t) \ > while ((t = next_thread_careful(t)) != g) > > I think this should work. detach_pid() does unlock + lock at least > once and thus we have the barrier (this worth a comment or we > can add the explicit wmb() in __unhash_process). > > Paul, Roland, do you see any problems from the correctness pov, > or a better fix for now? > > Perhaps it also makes sense to keep the old variant renamed to > while_each_thread_locked(), I dunno.
Well. but current_is_single_threaded() and zap_threads() have to use next_thread() or while_each_thread_locked() in this case...
Oleg.
| |