Re: + hugetlb-cgroup-simplify-pre_destroy-callback.patch added to -mm tree

[Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>]

(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@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>