[lkml]   [2013]   [Dec]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [PATCH v2 1/4] introduce for_each_thread() to replace the buggy while_each_thread()
On Wed, 4 Dec 2013, Oleg Nesterov wrote:

> while_each_thread() and next_thread() should die, almost every
> lockless usage is wrong.
> 1. Unless g == current, the lockless while_each_thread() is not safe.
> while_each_thread(g, t) can loop forever if g exits, next_thread()
> can't reach the unhashed thread in this case. Note that this can
> happen even if g is the group leader, it can exec.
> 2. Even if while_each_thread() itself was correct, people often use
> it wrongly.
> It was never safe to just take rcu_read_lock() and loop unless
> you verify that pid_alive(g) == T, even the first next_thread()
> can point to the already freed/reused memory.
> This patch adds signal_struct->thread_head and task->thread_node
> to create the normal rcu-safe list with the stable head. The new
> for_each_thread(g, t) helper is always safe under rcu_read_lock()
> as long as this task_struct can't go away.
> Note: of course it is ugly to have both task_struct->thread_node
> and the old task_struct->thread_group, we will kill it later, after
> we change the users of while_each_thread() to use for_each_thread().
> Perhaps we can kill it even before we convert all users, we can
> reimplement next_thread(t) using the new thread_head/thread_node.
> But we can't do this right now because this will lead to subtle
> behavioural changes. For example, do/while_each_thread() always
> sees at least one task, while for_each_thread() can do nothing if
> the whole thread group has died. Or thread_group_empty(), currently
> its semantics is not clear unless thread_group_leader(p) and we
> need to audit the callers before we can change it.
> So this patch adds the new interface which has to coexist with the
> old one for some time, hopefully the next changes will be more or
> less straightforward and the old one will go away soon.
> Signed-off-by: Oleg Nesterov <>
> Reviewed-and-Tested-by: Sergey Dyasly <>
> Reviewed-by: Sameer Nanda <>

Acked-by: David Rientjes <>

Minor: should the definitions of thread_{head,node} be annotated with
__rcu for users of CONFIG_SPARSE_RCU_POINTER?

 \ /
  Last update: 2013-12-05 02:41    [W:0.120 / U:5.740 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site