aboutsummaryrefslogtreecommitdiffstats
path: root/kernel/cgroup.c
diff options
context:
space:
mode:
authorLi Zefan <lizefan@huawei.com>2013-01-14 04:23:26 -0500
committerTejun Heo <tj@kernel.org>2013-01-14 13:44:42 -0500
commit5d65bc0ca1bceb73204dab943922ba3c83276a8c (patch)
tree3ebf35a0a034eb4f8a564d71c46ad7de1fdffcfa /kernel/cgroup.c
parent0ac801fe07374148714b5ef53df90ac5b1673c0c (diff)
cgroup: remove synchronize_rcu() from cgroup_attach_{task|proc}()
These 2 syncronize_rcu()s make attaching a task to a cgroup quite slow, and it can't be ignored in some situations. A real case from Colin Cross: Android uses cgroups heavily to manage thread priorities, putting threads in a background group with reduced cpu.shares when they are not visible to the user, and in a foreground group when they are. Some RPCs from foreground threads to background threads will temporarily move the background thread into the foreground group for the duration of the RPC. This results in many calls to cgroup_attach_task. In cgroup_attach_task() it's task->cgroups that is protected by RCU, and put_css_set() calls kfree_rcu() to free it. If we remove this synchronize_rcu(), there can be threads in RCU-read sections accessing their old cgroup via current->cgroups with concurrent rmdir operation, but this is safe. # time for ((i=0; i<50; i++)) { echo $$ > /mnt/sub/tasks; echo $$ > /mnt/tasks; } real 0m2.524s user 0m0.008s sys 0m0.004s With this patch: real 0m0.004s user 0m0.004s sys 0m0.000s tj: These synchronize_rcu()s are utterly confused. synchornize_rcu() necessarily has to come between two operations to guarantee that the changes made by the former operation are visible to all rcu readers before proceeding to the latter operation. Here, synchornize_rcu() are at the end of attach operations with nothing beyond it. Its only effect would be delaying completion of write(2) to sysfs tasks/procs files until all rcu readers see the change, which doesn't mean anything. Signed-off-by: Li Zefan <lizefan@huawei.com> Signed-off-by: Tejun Heo <tj@kernel.org> Reported-by: Colin Cross <ccross@google.com>
Diffstat (limited to 'kernel/cgroup.c')
-rw-r--r--kernel/cgroup.c2
1 files changed, 0 insertions, 2 deletions
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 54b39081ac04..ce27351e9249 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -1974,7 +1974,6 @@ int cgroup_attach_task(struct cgroup *cgrp, struct task_struct *tsk)
1974 ss->attach(cgrp, &tset); 1974 ss->attach(cgrp, &tset);
1975 } 1975 }
1976 1976
1977 synchronize_rcu();
1978out: 1977out:
1979 if (retval) { 1978 if (retval) {
1980 for_each_subsys(root, ss) { 1979 for_each_subsys(root, ss) {
@@ -2143,7 +2142,6 @@ static int cgroup_attach_proc(struct cgroup *cgrp, struct task_struct *leader)
2143 /* 2142 /*
2144 * step 5: success! and cleanup 2143 * step 5: success! and cleanup
2145 */ 2144 */
2146 synchronize_rcu();
2147 retval = 0; 2145 retval = 0;
2148out_put_css_set_refs: 2146out_put_css_set_refs:
2149 if (retval) { 2147 if (retval) {