Messages in this thread | | | Date | Wed, 25 Nov 2015 14:51:38 -0500 | From | Tejun Heo <> | Subject | Re: [PATCH?] race between cgroup_subsys->fork() and cgroup_migrate() |
| |
Hello, Oleg.
On Wed, Nov 25, 2015 at 05:34:27PM +0100, Oleg Nesterov wrote: > IOW. Suppose that the new child is moved right before cgroup_post_fork() does > > for_each_subsys_which(...) > ss->fork(child); > > doesn't this mean that after ss->fork() we do the same sequence > > pids_uncharge(old_pids, 1); > pids_charge(pids, 1);
You're absolutely right.
> twice? Note that threadgroup_change_begin/end depends on CLONE_THREAD. > So we can actually hit WARN_ON() in pids_cancel(). > > However, we can't simply remove this uncharge/charge afaics. We need this in > case when the parent was moved to another cgroup before cgroup_post_fork(), > and then css_set_move_task() moves the child. > > I know almost nothing about cgroups, perhaps I missed something, please > correct me.
I can't think of anything better than what you're proposing. Thanks a lot for tracking it down and fixing it.
> If am right. How about the patch below? percpu_down_read() is cheap. And > we can simplify cgroup_pids after this change. > > And. We can probably unify cgroup_threadgroup_rwsem and dup_mmap_sem. > Note that if we take cgroup_threadgroup_rwsem for reading if CLONE_THREAD, > otherwise we take another percpu-rwsem in dup_mmap(), dup_mmap_sem.
Sounds perfect. As this needs to go through -stable, can you please resend the patch with proper description and SOB? Please also update the now incorrect comment in can_attach.
Thanks a lot!
-- tejun
| |