Skip to content

Commit 0c740d0

Browse files
oleg-nesterovtorvalds
authored andcommitted
introduce for_each_thread() to replace the buggy while_each_thread()
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 <[email protected]> Reviewed-by: Sergey Dyasly <[email protected]> Tested-by: Sergey Dyasly <[email protected]> Reviewed-by: Sameer Nanda <[email protected]> Acked-by: David Rientjes <[email protected]> Cc: "Eric W. Biederman" <[email protected]> Cc: Frederic Weisbecker <[email protected]> Cc: Mandeep Singh Baines <[email protected]> Cc: "Ma, Xindong" <[email protected]> Cc: Michal Hocko <[email protected]> Cc: "Tu, Xiaobing" <[email protected]> Signed-off-by: Andrew Morton <[email protected]> Signed-off-by: Linus Torvalds <[email protected]>
1 parent 9853a40 commit 0c740d0

File tree

4 files changed

+22
-0
lines changed

4 files changed

+22
-0
lines changed

include/linux/init_task.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ extern struct fs_struct init_fs;
4141

4242
#define INIT_SIGNALS(sig) { \
4343
.nr_threads = 1, \
44+
.thread_head = LIST_HEAD_INIT(init_task.thread_node), \
4445
.wait_chldexit = __WAIT_QUEUE_HEAD_INITIALIZER(sig.wait_chldexit),\
4546
.shared_pending = { \
4647
.list = LIST_HEAD_INIT(sig.shared_pending.list), \
@@ -222,6 +223,7 @@ extern struct task_group root_task_group;
222223
[PIDTYPE_SID] = INIT_PID_LINK(PIDTYPE_SID), \
223224
}, \
224225
.thread_group = LIST_HEAD_INIT(tsk.thread_group), \
226+
.thread_node = LIST_HEAD_INIT(init_signals.thread_head), \
225227
INIT_IDS \
226228
INIT_PERF_EVENTS(tsk) \
227229
INIT_TRACE_IRQFLAGS \

include/linux/sched.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -549,6 +549,7 @@ struct signal_struct {
549549
atomic_t sigcnt;
550550
atomic_t live;
551551
int nr_threads;
552+
struct list_head thread_head;
552553

553554
wait_queue_head_t wait_chldexit; /* for wait4() */
554555

@@ -1271,6 +1272,7 @@ struct task_struct {
12711272
/* PID/PID hash table linkage. */
12721273
struct pid_link pids[PIDTYPE_MAX];
12731274
struct list_head thread_group;
1275+
struct list_head thread_node;
12741276

12751277
struct completion *vfork_done; /* for vfork() */
12761278
int __user *set_child_tid; /* CLONE_CHILD_SETTID */
@@ -2341,6 +2343,16 @@ extern bool current_is_single_threaded(void);
23412343
#define while_each_thread(g, t) \
23422344
while ((t = next_thread(t)) != g)
23432345

2346+
#define __for_each_thread(signal, t) \
2347+
list_for_each_entry_rcu(t, &(signal)->thread_head, thread_node)
2348+
2349+
#define for_each_thread(p, t) \
2350+
__for_each_thread((p)->signal, t)
2351+
2352+
/* Careful: this is a double loop, 'break' won't work as expected. */
2353+
#define for_each_process_thread(p, t) \
2354+
for_each_process(p) for_each_thread(p, t)
2355+
23442356
static inline int get_nr_threads(struct task_struct *tsk)
23452357
{
23462358
return tsk->signal->nr_threads;

kernel/exit.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ static void __unhash_process(struct task_struct *p, bool group_dead)
7474
__this_cpu_dec(process_counts);
7575
}
7676
list_del_rcu(&p->thread_group);
77+
list_del_rcu(&p->thread_node);
7778
}
7879

7980
/*

kernel/fork.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1035,6 +1035,11 @@ static int copy_signal(unsigned long clone_flags, struct task_struct *tsk)
10351035
sig->nr_threads = 1;
10361036
atomic_set(&sig->live, 1);
10371037
atomic_set(&sig->sigcnt, 1);
1038+
1039+
/* list_add(thread_node, thread_head) without INIT_LIST_HEAD() */
1040+
sig->thread_head = (struct list_head)LIST_HEAD_INIT(tsk->thread_node);
1041+
tsk->thread_node = (struct list_head)LIST_HEAD_INIT(sig->thread_head);
1042+
10381043
init_waitqueue_head(&sig->wait_chldexit);
10391044
sig->curr_target = tsk;
10401045
init_sigpending(&sig->shared_pending);
@@ -1474,6 +1479,8 @@ static struct task_struct *copy_process(unsigned long clone_flags,
14741479
atomic_inc(&current->signal->sigcnt);
14751480
list_add_tail_rcu(&p->thread_group,
14761481
&p->group_leader->thread_group);
1482+
list_add_tail_rcu(&p->thread_node,
1483+
&p->signal->thread_head);
14771484
}
14781485
attach_pid(p, PIDTYPE_PID);
14791486
nr_threads++;

0 commit comments

Comments
 (0)