(2012/07/21 11:46), Tejun Heo wrote:
Hello, Kamezawa-san.
On Sat, Jul 21, 2012 at 11:14:21AM +0900, Kamezawa Hiroyuki wrote:
I'm sorry I misunderstand. The problem is following.
CPU A CPU B
mutex_unlock()
mutex_lock()
->pre_destroy() attach task
commit res->usage=0 mutex_unlock()
increase res->usage
detach task
mutex_lock()
check css's refcount=0
....continue destroy.
Now, I thinks memcg's check is not enough but putting the -EBUSY there
not to forget this race.
I think a patch to stop task-attach and create child cgroup if CGRP_WAIT_ON_RMDIR
is set is required. And that's enough..
The *ONLY* reason we're not marking the cgroup dead after the checking
whether the cgroup has children or task at the top of cgroup_rmdir()
is because memcg might fail ->pre_destroy() and cancel the cgroup
removal. We can't commit to removal because memcg might fail.
Now, if memcg drops the deprecated behavior, we can simply commit to
removal *before* starting calling pre_destroy() and it doesn't matter
at all whether we hold cgroup_mutex across pre_destroy or not and
cgroup core will simply deny any addition to the cgroup committed to
death. (and remove a handsome amount of ugly code in the process)
So, the *ONLY* reason this can't be fixed properly from cgroup core is
because memcg's pre_destory() might fail and it doesn't make much
sense to me to implement add a workaround at this point when the whole
problem will go away once memcg's pre_destroy() is updated.
So, please update memcg and drop the __DEPRECATED flag, so that the
cgroup core can drop at least this particular part of misdesign. :(
Maybe it's better to remove memcg's pre_destroy() at all and do the job
in asynchronus thread called by ->destroy().
I'll cook a patch again.
-Kame
--
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