Messages in this thread | | | Date | Sat, 20 Oct 2012 15:37:09 -0700 | From | Tejun Heo <> | Subject | Re: [PATCH cgroup/for-3.7-fixes 1/2] Revert "cgroup: Remove task_lock() from cgroup_post_fork()" |
| |
Hello, Frederic.
On Sat, Oct 20, 2012 at 02:21:43PM -0400, Frederic Weisbecker wrote: > CPU 0 > CPU 1 > > cgroup_task_migrate { > task_lock(p) > rcu_assign_pointer(tsk->cgroups, newcg); > task_unlock(tsk); > > write_lock(&css_set_lock); > if (!list_empty(&tsk->cg_list)) > list_move(&tsk->cg_list, &newcg->tasks); > write_unlock(&css_set_lock); > > write_lock(&css_set_lock); > put_css_set(oldcg); > list_add(&child->cg_list, &child->cgroups->tasks); (1)
Man, that's confusing. :)
> On (1), child->cgroups should have the value of newcg and not oldcg > due to the memory ordering implied by the locking of css_set_lock. Now > I can't guarantee that because I'm no memory ordering expert. And even > if it's safe, it's so very non obvious that I now agree with you: > let's revert the patch and restart with a better base by gathering > all the cgroup fork code in the current cgroup_post_fork place.
Aye aye, let's move everything to cgroup_post_fork() and then we don't have to worry about grabbing task_lock multiple times.
Thanks.
-- tejun
| |