Re: [mm] [PATCH 2/4] Memory cgroup resource counters for hierarchy

Balbir Singh
2008-11-02T05:49:55Z

KAMEZAWA Hiroyuki wrote:
> On Sun, 02 Nov 2008 00:18:37 +0530
> Balbir Singh <balbir@linux.vnet.ibm.com> wrote:
>
>> Add support for building hierarchies in resource counters. Cgroups allows us
>> to build a deep hierarchy, but we currently don't link the resource counters
>> belonging to the memory controller control groups, which are linked in
>> cgroup hiearchy. This patch provides the infrastructure for resource counters
>> that have the same hiearchy as their cgroup counter parts.
>>
>> These set of patches are based on the resource counter hiearchy patches posted
>> by Pavel Emelianov.
>>
>> NOTE: Building hiearchies is expensive, deeper hierarchies imply charging
>> the all the way up to the root. It is known that hiearchies are expensive,
>> so the user needs to be careful and aware of the trade-offs before creating
>> very deep ones.
>>
> ...isn't it better to add "root_lock" to res_counter rather than taking
> all levels of lock one by one ?
>
> spin_lock(&res_counter->hierarchy_root->lock);
> do all charge/uncharge to hierarchy
> spin_unlock(&res_counter->hierarchy_root->lock);
>
> Hmm ?
>

Good thought process, but that affects and adds code complexity for the case
when hierarchy is enabled/disabled. It is also inefficient, since all charges
will now contend on root lock, in the current process, it is step by step, the
contention only occurs on common parts of the hierarchy (root being the best case).

Thanks for the comments,

--
Balbir