Messages in this thread |  | | Date | Wed, 4 Dec 2013 16:27:19 +0100 | From | Oleg Nesterov <> | Subject | Re: [PATCH v2 1/4] introduce for_each_thread() to replace the buggy while_each_thread() |
| |
On 12/04, Frederic Weisbecker wrote: > > On Wed, Dec 04, 2013 at 02:49:17PM +0100, Oleg Nesterov wrote: > > > > Yes, perhaps we will need for_each_thread_continue(). I am not sure > > yet. And note that, say, check_hung_uninterruptible_tasks() already > > does _continue if fact, although it is still not clear to me if we > > actually need this helper. > > So that's one of the possible users. _continue() can make sense if the > reader can easily cope with missing a few threads from time to time, which > is the case of the hung task detector.
Yes, but again it is not clear if we need the new helper.
For example. Note that you can simply do something like:
// p can't go away
rcu_read_lock(); for_each_thread(p, t) { do_something(t);
if (need_to_sleep()) { get_task_struct(t); rcu_read_unlock();
schedule_timeout_interruptible(...);
rcu_read_lock(); put_task_struct(); if (!pid_alive(t)) break; } } rcu_read_unlock();
If you rewrite this code using for_each_thread_continue (which is just list_for_each_entry_continue_rcu) the code will look more complex.
> > Note also that _continue() can't be safely used lockless, unless > > you verify pid_alive() or something similar. > > Hmm, due to concurrent list_del()? > > Right, tsk->thread_list.next could point to junk after a list_del(), say if the next > entry has been freed.
Yes. The same problem which while_each_thread() currently has (I mean, even ignoring the fact it is itself buggy).
Oleg.
|  |