(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, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>