From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michal Hocko Subject: Re: [PATCH REPOST RFC cgroup/for-3.7] cgroup: mark subsystems with broken hierarchy support and whine if cgroups are nested for them Date: Tue, 11 Sep 2012 12:04:33 +0200 Message-ID: <20120911100433.GC8058@dhcp22.suse.cz> References: <20120910223125.GC7677@google.com> <20120910223355.GD7677@google.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <20120910223355.GD7677-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: containers-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org Errors-To: containers-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org To: Tejun Heo Cc: Neil Horman , "Serge E. Hallyn" , containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Paul Mackerras , "Aneesh Kumar K.V" , Arnaldo Carvalho de Melo , Johannes Weiner , Thomas Graf , cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Paul Turner , Ingo Molnar , Vivek Goyal List-Id: containers.vger.kernel.org I like the approach in general but see the comments bellow: On Mon 10-09-12 15:33:55, Tejun Heo wrote: [...] > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -3855,12 +3855,17 @@ static int mem_cgroup_hierarchy_write(st > */ > if ((!parent_memcg || !parent_memcg->use_hierarchy) && > (val == 1 || val == 0)) { > - if (list_empty(&cont->children)) > + if (list_empty(&cont->children)) { > memcg->use_hierarchy = val; > - else > + /* we're fully hierarchical iff root uses hierarchy */ > + if (mem_cgroup_is_root(memcg)) > + mem_cgroup_subsys.broken_hierarchy = !val; > + } else { > retval = -EBUSY; > - } else > + } > + } else { > retval = -EINVAL; > + } > > out: > cgroup_unlock(); > @@ -4953,6 +4958,7 @@ mem_cgroup_create(struct cgroup *cont) > &per_cpu(memcg_stock, cpu); > INIT_WORK(&stock->work, drain_local_stock); > } > + mem_cgroup_subsys.broken_hierarchy = !memcg->use_hierarchy; Hmmm, this will warn even if we have root (default use_hierarchy=0) \ A (use_hierarchy=1) \ B <- here which is unfortunate because it will add a noise to a reasonable configuration. I think this is fixable if you move the warning after cgroup_subsys_state::create and broken_hierarchy would be set only if parent is not root and use_hierarchy==0 in mem_cgroup_create. Something like: diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 795e525..d5c93ab 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -4973,6 +4973,13 @@ mem_cgroup_create(struct cgroup *cont) } else { res_counter_init(&memcg->res, NULL); res_counter_init(&memcg->memsw, NULL); + /* + * Deeper hierachy with use_hierarchy == false doesn't make + * much sense so let cgroup subsystem know about this unfortunate + * state in our controller. + */ + if (parent && parent != root_mem_cgroup) + mem_cgroup_subsys.broken_hierarchy = true; } memcg->last_scanned_node = MAX_NUMNODES; INIT_LIST_HEAD(&memcg->oom_notify); What do you think? > hotcpu_notifier(memcg_cpu_hotplug_callback, 0); > } else { > parent = mem_cgroup_from_cont(cont->parent); > --- a/net/core/netprio_cgroup.c > +++ b/net/core/netprio_cgroup.c > @@ -330,7 +330,17 @@ struct cgroup_subsys net_prio_subsys = { > .subsys_id = net_prio_subsys_id, > #endif > .base_cftypes = ss_files, > - .module = THIS_MODULE > + .module = THIS_MODULE, > + > + /* > + * net_prio has artificial limit on the number of cgroups and > + * disallows nesting making it impossible to co-mount it with other > + * hierarchical subsystems. Remove the artificially low PRIOIDX_SZ > + * limit and properly nest configuration such that children follow > + * their parents' configurations by default and are allowed to > + * override and remove the following. > + */ > + .broken_hierarchy = trye, typo -- Michal Hocko SUSE Labs