On 06/19/2012 04:16 AM, Kamezawa Hiroyuki wrote:
> (2012/06/18 21:43), Glauber Costa wrote:
>> On 06/18/2012 04:37 PM, Kamezawa Hiroyuki wrote:
>>> (2012/06/18 19:28), Glauber Costa wrote:
>>>> The current memcg slab cache management fails to present satisfatory hierarchical
>>>> behavior in the following scenario:
>>>>
>>>> -> /cgroups/memory/A/B/C
>>>>
>>>> * kmem limit set at A
>>>> * A and B empty taskwise
>>>> * bash in C does find /
>>>>
>>>> Because kmem_accounted is a boolean that was not set for C, no accounting
>>>> would be done. This is, however, not what we expect.
>>>>
>>>
>>> Hmm....do we need this new routines even while we have mem_cgroup_iter() ?
>>>
>>> Doesn't this work ?
>>>
>>> struct mem_cgroup {
>>> .....
>>> bool kmem_accounted_this;
>>> atomic_t kmem_accounted;
>>> ....
>>> }
>>>
>>> at set limit
>>>
>>> ....set_limit(memcg) {
>>>
>>> if (newly accounted) {
>>> mem_cgroup_iter() {
>>> atomic_inc(&iter->kmem_accounted)
>>> }
>>> } else {
>>> mem_cgroup_iter() {
>>> atomic_dec(&iter->kmem_accounted);
>>> }
>>> }
>>>
>>>
>>> hm ? Then, you can see kmem is accounted or not by atomic_read(&memcg->kmem_accounted);
>>>
>>
>> Accounted by itself / parent is still useful, and I see no reason to use
>> an atomic + bool if we can use a pair of bits.
>>
>> As for the routine, I guess mem_cgroup_iter will work... It does a lot
>> more than I need, but for the sake of using what's already in there, I
>> can switch to it with no problems.
>>
>
> Hmm. please start from reusing existing routines.
> If it's not enough, some enhancement for generic cgroup will be welcomed
> rather than completely new one only for memcg.
>
And now that I am trying to adapt the code to the new function, I
remember clearly why I done this way. Sorry for my failed memory.
That has to do with the order of the walk. I need to enforce hierarchy,
which means whenever a cgroup has !use_hierarchy, I need to cut out that
branch, but continue scanning the tree for other branches.
That is a lot easier to do with depth-search tree walks like the one
proposed in this patch. for_each_mem_cgroup() seems to walk the tree in
css-creation order. Which means we need to keep track of parents that
has hierarchy disabled at all times ( can be many ), and always test for
ancestorship - which is expensive, but I don't particularly care.
But I'll give another shot with this one.
--
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>