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

KAMEZAWA Hiroyuki
2008-11-02T05:57:25Z

On Sun, 02 Nov 2008 11:19:38 +0530
Balbir Singh <balbir@linux.vnet.ibm.com> wrote:

> 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).
>

Above code's contention level is not different from "only root no children" case.
Just inside-lock is heavier.

Thanks,
-Kame