On Thu, Mar 07, 2013 at 10:01:39AM -0800, Tejun Heo wrote: > Hello, Oleg. > > On Thu, Mar 07, 2013 at 06:25:45PM +0100, Oleg Nesterov wrote: > > > [ 944.011126] Chain exists of: > > > &sb->s_type->i_mutex_key#9 --> cgroup_mutex --> &sig->cred_guard_mutex > > > > > > [ 944.012745] Possible unsafe locking scenario: > > > > > > [ 944.013617] CPU0 CPU1 > > > [ 944.014280] ---- ---- > > > [ 944.014942] lock(&sig->cred_guard_mutex); > > > [ 944.021332] lock(cgroup_mutex); > > > [ 944.028094] lock(&sig->cred_guard_mutex); > > > [ 944.035007] lock(&sb->s_type->i_mutex_key#9); > > > [ 944.041602] > > > > And cgroup_mount() does i_mutex -> cgroup_mutex... > > Hmmm... > > > Add cc's. I do not think we can move open_exec() outside of cred_guard_mutex. > > We can change do_execve_common(), but binfmt->load_binary() does open() too. > > > > And it is not easy to avoid ->cred_guard_mutex in threadgroup_lock(), we can't > > change de_thread() to do threadgroup_change_begin/end... > > > > Or perhaps we can? It doesn't need to sleep under ->group_rwsem, we only > > need it around ->group_leader changing. Otherwise cgroup_attach_proc() > > can rely on do_exit()->threadgroup_change_begin() ? > > Using cred_guard_mutex was mostly to avoid adding another locking in > de_thread() path as it already had one. We can add group_rwsem > locking deeper inside and avoid this problem. > > > But perhaps someone can suggest another fix in cgroup.c. > > Another possibility is moving cgroup_lock outside threadgroup_lock(), > which was impossible before because of cgroup_lock abuses in specific > controller implementations but most of that have been updated and we > should now be pretty close to being able to make cgroup_lock outer to > most other locks. Appending a completely untested patch below. This probably doesn't help as the dependency involves i_mutex. I think Oleg's proposed patch should work. Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe cgroups" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
