Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932204AbaGPJqW (ORCPT ); Wed, 16 Jul 2014 05:46:22 -0400 Received: from cantor2.suse.de ([195.135.220.15]:49155 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755419AbaGPJqS (ORCPT ); Wed, 16 Jul 2014 05:46:18 -0400 Date: Wed, 16 Jul 2014 11:46:16 +0200 From: Michal Hocko To: Tejun Heo Cc: lizefan@huawei.com, cgroups@vger.kernel.org, linux-kernel@vger.kernel.org, Johannes Weiner , Vivek Goyal , Peter Zijlstra , Paul Mackerras , Ingo Molnar , Arnaldo Carvalho de Melo , Aristeu Rozanski , "Aneesh Kumar K.V" Subject: Re: [PATCH 4/6] cgroup: distinguish the default and legacy hierarchies when handling cftypes Message-ID: <20140716094616.GH7121@dhcp22.suse.cz> References: <1405352648-4279-1-git-send-email-tj@kernel.org> <1405352648-4279-5-git-send-email-tj@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1405352648-4279-5-git-send-email-tj@kernel.org> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon 14-07-14 11:44:06, Tejun Heo wrote: > Until now, cftype arrays carried files for both the default and legacy > hierarchies and the files which needed to be used on only one of them > were flagged with either CFTYPE_ONLY_ON_DFL or CFTYPE_INSANE. This > gets confusing very quickly and we may end up exposing interface files > to the default hierarchy without thinking it through. > > This patch makes cgroup core provide separate sets of interfaces for > cftype handling so that the cftypes for the default and legacy > hierarchies are clearly distinguished. The previous two patches > renamed the existing ones so that they clearly indicate that they're > for the legacy hierarchies. This patch adds the interface for the > default hierarchy and apply them selectively depending on the > hierarchy type. > > * cftypes added through cgroup_subsys->dfl_cftypes and > cgroup_add_dfl_cftypes() only show up on the default hierarchy. > > * cftypes added through cgroup_subsys->legacy_cftypes and > cgroup_add_legacy_cftypes() only show up on the legacy hierarchies. > > * cgroup_subsys->dfl_cftypes and ->legacy_cftypes can point to the > same array for the cases where the interface files are identical on > both types of hierarchies. > > * This makes all the existing subsystem interface files legacy-only by > default and all subsystems will have no interface file created when > enabled on the default hierarchy. Each subsystem should explicitly > review and compose the interface for the default hierarchy. > > * A boot param "cgroup__DEVEL__legacy_files_on_dfl" is added which > makes subsystems which haven't decided the interface files for the > default hierarchy to present the legacy files on the default > hierarchy so that its behavior on the default hierarchy can be > tested. As the awkward name suggests, this is for development only. > > * memcg's CFTYPE_INSANE on "use_hierarchy" is noop now as the whole > array isn't used on the default hierarchy. The flag is removed. > > v2: Updated documentation for cgroup__DEVEL__legacy_files_on_dfl. > > v3: Clear CFTYPE_ONLY_ON_DFL and CFTYPE_INSANE when cfts are removed > as suggested by Li. > > Signed-off-by: Tejun Heo > Acked-by: Neil Horman > Cc: Johannes Weiner > Cc: Michal Hocko > Cc: Li Zefan > Cc: Vivek Goyal > Cc: Peter Zijlstra > Cc: Paul Mackerras > Cc: Ingo Molnar > Cc: Arnaldo Carvalho de Melo > Cc: Aristeu Rozanski > Cc: Aneesh Kumar K.V As mentioned on the other email I will cook up a patch to allow basic knobs for memcg. Acked-by: Michal Hocko > --- > Documentation/cgroups/unified-hierarchy.txt | 18 ++++++--- > include/linux/cgroup.h | 9 ++++- > kernel/cgroup.c | 62 +++++++++++++++++++++++++++-- > mm/memcontrol.c | 1 - > 4 files changed, 78 insertions(+), 12 deletions(-) > > diff --git a/Documentation/cgroups/unified-hierarchy.txt b/Documentation/cgroups/unified-hierarchy.txt > index a7a2205..4f45632 100644 > --- a/Documentation/cgroups/unified-hierarchy.txt > +++ b/Documentation/cgroups/unified-hierarchy.txt > @@ -94,12 +94,18 @@ change soon. > > mount -t cgroup -o __DEVEL__sane_behavior cgroup $MOUNT_POINT > > -All controllers which are not bound to other hierarchies are > -automatically bound to unified hierarchy and show up at the root of > -it. Controllers which are enabled only in the root of unified > -hierarchy can be bound to other hierarchies. This allows mixing > -unified hierarchy with the traditional multiple hierarchies in a fully > -backward compatible way. > +All controllers which support the unified hierarchy and are not bound > +to other hierarchies are automatically bound to unified hierarchy and > +show up at the root of it. Controllers which are enabled only in the > +root of unified hierarchy can be bound to other hierarchies. This > +allows mixing unified hierarchy with the traditional multiple > +hierarchies in a fully backward compatible way. > + > +For development purposes, the following boot parameter makes all > +controllers to appear on the unified hierarchy whether supported or > +not. > + > + cgroup__DEVEL__legacy_files_on_dfl > > A controller can be moved across hierarchies only after the controller > is no longer referenced in its current hierarchy. Because per-cgroup > diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h > index f5f0fee..9f76236 100644 > --- a/include/linux/cgroup.h > +++ b/include/linux/cgroup.h > @@ -590,6 +590,7 @@ static inline void pr_cont_cgroup_path(struct cgroup *cgrp) > > char *task_cgroup_path(struct task_struct *task, char *buf, size_t buflen); > > +int cgroup_add_dfl_cftypes(struct cgroup_subsys *ss, struct cftype *cfts); > int cgroup_add_legacy_cftypes(struct cgroup_subsys *ss, struct cftype *cfts); > int cgroup_rm_cftypes(struct cftype *cfts); > > @@ -671,8 +672,12 @@ struct cgroup_subsys { > */ > struct list_head cfts; > > - /* base cftypes, automatically registered with subsys itself */ > - struct cftype *legacy_cftypes; /* used on the legacy hierarchies */ > + /* > + * Base cftypes which are automatically registered. The two can > + * point to the same array. > + */ > + struct cftype *dfl_cftypes; /* for the default hierarchy */ > + struct cftype *legacy_cftypes; /* for the legacy hierarchies */ > > /* > * A subsystem may depend on other subsystems. When such subsystem > diff --git a/kernel/cgroup.c b/kernel/cgroup.c > index c275aa4..374ebdf 100644 > --- a/kernel/cgroup.c > +++ b/kernel/cgroup.c > @@ -149,6 +149,12 @@ struct cgroup_root cgrp_dfl_root; > */ > static bool cgrp_dfl_root_visible; > > +/* > + * Set by the boot param of the same name and makes subsystems with NULL > + * ->dfl_files to use ->legacy_files on the default hierarchy. > + */ > +static bool cgroup_legacy_files_on_dfl; > + > /* some controllers are not supported in the default hierarchy */ > static const unsigned int cgrp_dfl_root_inhibit_ss_mask = 0 > #ifdef CONFIG_CGROUP_DEBUG > @@ -3085,6 +3091,9 @@ static void cgroup_exit_cftypes(struct cftype *cfts) > kfree(cft->kf_ops); > cft->kf_ops = NULL; > cft->ss = NULL; > + > + /* revert flags set by cgroup core while adding @cfts */ > + cft->flags &= ~(CFTYPE_ONLY_ON_DFL | CFTYPE_INSANE); > } > } > > @@ -3195,8 +3204,37 @@ static int cgroup_add_cftypes(struct cgroup_subsys *ss, struct cftype *cfts) > return ret; > } > > +/** > + * cgroup_add_dfl_cftypes - add an array of cftypes for default hierarchy > + * @ss: target cgroup subsystem > + * @cfts: zero-length name terminated array of cftypes > + * > + * Similar to cgroup_add_cftypes() but the added files are only used for > + * the default hierarchy. > + */ > +int cgroup_add_dfl_cftypes(struct cgroup_subsys *ss, struct cftype *cfts) > +{ > + struct cftype *cft; > + > + for (cft = cfts; cft && cft->name[0] != '\0'; cft++) > + cft->flags |= CFTYPE_ONLY_ON_DFL; > + return cgroup_add_cftypes(ss, cfts); > +} > + > +/** > + * cgroup_add_legacy_cftypes - add an array of cftypes for legacy hierarchies > + * @ss: target cgroup subsystem > + * @cfts: zero-length name terminated array of cftypes > + * > + * Similar to cgroup_add_cftypes() but the added files are only used for > + * the legacy hierarchies. > + */ > int cgroup_add_legacy_cftypes(struct cgroup_subsys *ss, struct cftype *cfts) > { > + struct cftype *cft; > + > + for (cft = cfts; cft && cft->name[0] != '\0'; cft++) > + cft->flags |= CFTYPE_INSANE; > return cgroup_add_cftypes(ss, cfts); > } > > @@ -4893,9 +4931,19 @@ int __init cgroup_init(void) > * disabled flag and cftype registration needs kmalloc, > * both of which aren't available during early_init. > */ > - if (!ss->disabled) { > - cgrp_dfl_root.subsys_mask |= 1 << ss->id; > - WARN_ON(cgroup_add_cftypes(ss, ss->legacy_cftypes)); > + if (ss->disabled) > + continue; > + > + cgrp_dfl_root.subsys_mask |= 1 << ss->id; > + > + if (cgroup_legacy_files_on_dfl && !ss->dfl_cftypes) > + ss->dfl_cftypes = ss->legacy_cftypes; > + > + if (ss->dfl_cftypes == ss->legacy_cftypes) { > + WARN_ON(cgroup_add_cftypes(ss, ss->dfl_cftypes)); > + } else { > + WARN_ON(cgroup_add_dfl_cftypes(ss, ss->dfl_cftypes)); > + WARN_ON(cgroup_add_legacy_cftypes(ss, ss->legacy_cftypes)); > } > } > > @@ -5291,6 +5339,14 @@ static int __init cgroup_disable(char *str) > } > __setup("cgroup_disable=", cgroup_disable); > > +static int __init cgroup_set_legacy_files_on_dfl(char *str) > +{ > + printk("cgroup: using legacy files on the default hierarchy\n"); > + cgroup_legacy_files_on_dfl = true; > + return 0; > +} > +__setup("cgroup__DEVEL__legacy_files_on_dfl", cgroup_set_legacy_files_on_dfl); > + > /** > * css_tryget_online_from_dir - get corresponding css from a cgroup dentry > * @dentry: directory dentry of interest > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index b6b3c6f..45c10c6 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -6003,7 +6003,6 @@ static struct cftype mem_cgroup_files[] = { > }, > { > .name = "use_hierarchy", > - .flags = CFTYPE_INSANE, > .write_u64 = mem_cgroup_hierarchy_write, > .read_u64 = mem_cgroup_hierarchy_read, > }, > -- > 1.9.3 > -- Michal Hocko SUSE Labs -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/