2012/10/21 Tejun Heo <[email protected]>: > 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. :)
Sorry and I'm currently stuck in some airport and too lazy to reorder the above lines :) > >> 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. Agreed. and Acked-by: Frederic Weisbecker <[email protected]> -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
