By reading group_leader after taking the threadgroup_lock, we can avoid the double-check locking. This removes the return of -EAGAIN so we can cleanup cgroup_procs_write at the same time. The suggestion was made here: https://lkml.org/lkml/2011/12/22/371 Suggested-by: Tejun Heo <tj@xxxxxxxxxx> Signed-off-by: Mandeep Singh Baines <msb@xxxxxxxxxxxx> Cc: Li Zefan <lizf@xxxxxxxxxxxxxx> Cc: containers@xxxxxxxxxxxxxxxxxxxxxxxxxx Cc: cgroups@xxxxxxxxxxxxxxx Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@xxxxxxxxxxxxxx> Cc: Frederic Weisbecker <fweisbec@xxxxxxxxx> Cc: Oleg Nesterov <oleg@xxxxxxxxxx> Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> Cc: Paul Menage <paul@xxxxxxxxxxxxxx> --- kernel/cgroup.c | 40 ++++++---------------------------------- 1 files changed, 6 insertions(+), 34 deletions(-) diff --git a/kernel/cgroup.c b/kernel/cgroup.c index 032139d..a5f7d1b 100644 --- a/kernel/cgroup.c +++ b/kernel/cgroup.c @@ -2234,9 +2234,6 @@ static int attach_task_by_pid(struct cgroup *cgrp, u64 pid, bool threadgroup) ret= -ESRCH; goto out_unlock_cgroup; } - /* we check later for a group_leader race with de_thread */ - if (threadgroup) - tsk = tsk->group_leader; /* * even if we're attaching all tasks in the thread group, we * only need to check permissions on one of them. @@ -2252,33 +2249,17 @@ static int attach_task_by_pid(struct cgroup *cgrp, u64 pid, bool threadgroup) get_task_struct(tsk); rcu_read_unlock(); } else { - if (threadgroup) - tsk = current->group_leader; - else - tsk = current; + tsk = current; get_task_struct(tsk); } threadgroup_lock(tsk); - - if (threadgroup) { - if (!thread_group_leader(tsk)) { - /* - * a race with de_thread from another thread's exec() - * may strip us of our leadership, if this happens, - * there is no choice but to throw this task away and - * try again (from cgroup_procs_write); this is - * "double-double-toil-and-trouble-check locking". - */ - ret = -EAGAIN; - goto out_unlock_threadgroup; - } - ret = cgroup_attach_proc(cgrp, tsk); - } else + if (threadgroup) + ret = cgroup_attach_proc(cgrp, tsk->group_leader); + else ret = cgroup_attach_task(cgrp, tsk); - -out_unlock_threadgroup: threadgroup_unlock(tsk); + put_task_struct(tsk); out_unlock_cgroup: cgroup_unlock(); @@ -2292,16 +2273,7 @@ static int cgroup_tasks_write(struct cgroup *cgrp, struct cftype *cft, u64 pid) static int cgroup_procs_write(struct cgroup *cgrp, struct cftype *cft, u64 tgid) { - int ret; - do { - /* - * attach_proc fails with -EAGAIN if threadgroup leadership - * changes in the middle of the operation, in which case we need - * to find the task_struct for the new leader and start over. - */ - ret = attach_task_by_pid(cgrp, tgid, true); - } while (ret == -EAGAIN); - return ret; + return attach_task_by_pid(cgrp, tgid, true); } /** -- 1.7.3.1 _______________________________________________ Containers mailing list Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/containers
