David Rientjes wrote: > On Thu, 30 Dec 2010, Li Zefan wrote: > >>> This needs to be done with cgroup_lock() instead of callback_mutex since >>> the post_clone() callback will store to cs->mems_allowed on >>> cgroup_clone(). >>> >> Then cpuset_post_clone() breaks the lock rule: >> >> * A task must hold both mutexes to modify cpusets... >> ... >> * If a task is only holding callback_mutex, then it has read-only >> * access to cpusets. >> >> But that's Ok, because cgroup_clone() is called during the creation of >> the new cgroup, so no one can access the cpuset at that time. >> > > I'm saying that if cpusets implements a cgroup_clone() handler that the > locking will break with only callback_mutex here because the only > synchronization after the new cgroup dentry is added is cgroup_lock() that > is always held when a post_clone callback is invoked and reading a mems > file may race since it is accessible before the task is attached in the > cgroup_clone() case. It's not a problem right now but may subtly break if > cpusets were to use cgroup_clone(). > cgroup_clone() was implemented for ns_cgroup, and ns_cgroup is scheduled to be removed in the coming 2.6.38, and so will be cgroup_clone(). As a replacement we've added cgroup.clone_children, and the post_clone() callback will be called in cgroup_create() if the clone_children flag is set. If we want to avoid subtle break in case post_clone() is used in other cases than cgroup creation in the future, we can just hold callback_mutex() in cpuset_post_clone(). _______________________________________________ Containers mailing list Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linux-foundation.org/mailman/listinfo/containers