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 _______________________________________________ Containers mailing list Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/containers