All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCHSET cgroup/for-3.17] cgroup: remove sane_behavior support on non-default hierarchies
@ 2014-07-02 23:45 ` Tejun Heo
  0 siblings, 0 replies; 11+ messages in thread
From: Tejun Heo @ 2014-07-02 23:45 UTC (permalink / raw)
  To: lizefan; +Cc: cgroups, linux-kernel

Hello,

sane_behavior has been used as a development vehicle for the default
unified hierarchy.  Now that the default hierarchy is in place, the
flag became redundant and confusing as its usage is allowed on all
hierarchies.  There are gonna be either the default hierarchy or
legacy ones.  Let's make that clear by removing sane_behavior support
on non-default hierarchies.

This patchset contains the following four patches.

 0001-cgroup-remove-CGRP_ROOT_OPTION_MASK.patch
 0002-cgroup-make-interface-file-cgroup.sane_behavior-lega.patch
 0003-cgroup-remove-sane_behavior-support-on-non-default-h.patch
 0004-cgroup-clean-up-sane_behavior-handling.patch

0001 is a trivial cleanup.

0002 removes "cgroup.sane_behavior" from the default hierarchy.

0003 removes sane_behavior support on non-default hierarchies.

0004 cleans up sane_behavior handling.

This patchset is on top of a497c3ba1d97 ("Linux 3.16-rc2") and
available in the following git branch.

 git://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup.git review-dfl-instead-of-sane

diffstat follows.  Thanks.

 block/blk-throttle.c   |    6 +-
 include/linux/cgroup.h |  128 ++++++++++++++++++++-----------------------------
 kernel/cgroup.c        |   96 +++++++++++++++---------------------
 kernel/cpuset.c        |   33 +++++-------
 mm/memcontrol.c        |    7 +-
 5 files changed, 117 insertions(+), 153 deletions(-)

--
tejun

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCHSET cgroup/for-3.17] cgroup: remove sane_behavior support on non-default hierarchies
@ 2014-07-02 23:45 ` Tejun Heo
  0 siblings, 0 replies; 11+ messages in thread
From: Tejun Heo @ 2014-07-02 23:45 UTC (permalink / raw)
  To: lizefan-hv44wF8Li93QT0dZR+AlfA
  Cc: cgroups-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA

Hello,

sane_behavior has been used as a development vehicle for the default
unified hierarchy.  Now that the default hierarchy is in place, the
flag became redundant and confusing as its usage is allowed on all
hierarchies.  There are gonna be either the default hierarchy or
legacy ones.  Let's make that clear by removing sane_behavior support
on non-default hierarchies.

This patchset contains the following four patches.

 0001-cgroup-remove-CGRP_ROOT_OPTION_MASK.patch
 0002-cgroup-make-interface-file-cgroup.sane_behavior-lega.patch
 0003-cgroup-remove-sane_behavior-support-on-non-default-h.patch
 0004-cgroup-clean-up-sane_behavior-handling.patch

0001 is a trivial cleanup.

0002 removes "cgroup.sane_behavior" from the default hierarchy.

0003 removes sane_behavior support on non-default hierarchies.

0004 cleans up sane_behavior handling.

This patchset is on top of a497c3ba1d97 ("Linux 3.16-rc2") and
available in the following git branch.

 git://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup.git review-dfl-instead-of-sane

diffstat follows.  Thanks.

 block/blk-throttle.c   |    6 +-
 include/linux/cgroup.h |  128 ++++++++++++++++++++-----------------------------
 kernel/cgroup.c        |   96 +++++++++++++++---------------------
 kernel/cpuset.c        |   33 +++++-------
 mm/memcontrol.c        |    7 +-
 5 files changed, 117 insertions(+), 153 deletions(-)

--
tejun

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/4] cgroup: remove CGRP_ROOT_OPTION_MASK
  2014-07-02 23:45 ` Tejun Heo
  (?)
@ 2014-07-02 23:45 ` Tejun Heo
  -1 siblings, 0 replies; 11+ messages in thread
From: Tejun Heo @ 2014-07-02 23:45 UTC (permalink / raw)
  To: lizefan; +Cc: cgroups, linux-kernel, Tejun Heo

cgroup_root->flags only contains CGRP_ROOT_* flags and there's no
reason to mask the flags.  Remove CGRP_ROOT_OPTION_MASK.

This doesn't cause any behavior differences.

Signed-off-by: Tejun Heo <tj@kernel.org>
---
 include/linux/cgroup.h | 3 ---
 kernel/cgroup.c        | 7 +++----
 2 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index 8a111dd..7748e5b 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -312,9 +312,6 @@ enum {
 
 	CGRP_ROOT_NOPREFIX	= (1 << 1), /* mounted subsystems have no named prefix */
 	CGRP_ROOT_XATTR		= (1 << 2), /* supports extended attributes */
-
-	/* mount options live below bit 16 */
-	CGRP_ROOT_OPTION_MASK	= (1 << 16) - 1,
 };
 
 /*
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 7868fc3..e27f4d4 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -1434,11 +1434,10 @@ static int cgroup_remount(struct kernfs_root *kf_root, int *flags, char *data)
 	removed_mask = root->subsys_mask & ~opts.subsys_mask;
 
 	/* Don't allow flags or name to change at remount */
-	if (((opts.flags ^ root->flags) & CGRP_ROOT_OPTION_MASK) ||
+	if ((opts.flags ^ root->flags) ||
 	    (opts.name && strcmp(opts.name, root->name))) {
 		pr_err("option or name mismatch, new: 0x%x \"%s\", old: 0x%x \"%s\"\n",
-		       opts.flags & CGRP_ROOT_OPTION_MASK, opts.name ?: "",
-		       root->flags & CGRP_ROOT_OPTION_MASK, root->name);
+		       opts.flags, opts.name ?: "", root->flags, root->name);
 		ret = -EINVAL;
 		goto out_unlock;
 	}
@@ -1706,7 +1705,7 @@ static struct dentry *cgroup_mount(struct file_system_type *fs_type,
 			goto out_unlock;
 		}
 
-		if ((root->flags ^ opts.flags) & CGRP_ROOT_OPTION_MASK) {
+		if (root->flags ^ opts.flags) {
 			if ((root->flags | opts.flags) & CGRP_ROOT_SANE_BEHAVIOR) {
 				pr_err("sane_behavior: new mount options should match the existing superblock\n");
 				ret = -EINVAL;
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/4] cgroup: make interface file "cgroup.sane_behavior" legacy-only
  2014-07-02 23:45 ` Tejun Heo
  (?)
  (?)
@ 2014-07-02 23:45 ` Tejun Heo
  -1 siblings, 0 replies; 11+ messages in thread
From: Tejun Heo @ 2014-07-02 23:45 UTC (permalink / raw)
  To: lizefan; +Cc: cgroups, linux-kernel, Tejun Heo

"cgroup.sane_behavior" is added to help distinguishing whether
sane_behavior is in effect or not.  We now have the default hierarchy
where the flag is always in effect and are planning to remove
supporting sane behavior on the legacy hierarchies making this file on
the default hierarchy rather pointless.  Let's make it legacy only and
thus always zero.

Signed-off-by: Tejun Heo <tj@kernel.org>
---
 kernel/cgroup.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index e27f4d4..be701d9 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -2414,9 +2414,7 @@ static int cgroup_release_agent_show(struct seq_file *seq, void *v)
 
 static int cgroup_sane_behavior_show(struct seq_file *seq, void *v)
 {
-	struct cgroup *cgrp = seq_css(seq)->cgroup;
-
-	seq_printf(seq, "%d\n", cgroup_sane_behavior(cgrp));
+	seq_puts(seq, "0\n");
 	return 0;
 }
 
@@ -4016,7 +4014,7 @@ static struct cftype cgroup_base_files[] = {
 	},
 	{
 		.name = "cgroup.sane_behavior",
-		.flags = CFTYPE_ONLY_ON_ROOT,
+		.flags = CFTYPE_INSANE | CFTYPE_ONLY_ON_ROOT,
 		.seq_show = cgroup_sane_behavior_show,
 	},
 	{
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/4] cgroup: remove sane_behavior support on non-default hierarchies
  2014-07-02 23:45 ` Tejun Heo
                   ` (2 preceding siblings ...)
  (?)
@ 2014-07-02 23:45 ` Tejun Heo
  2014-07-08 19:57   ` Vivek Goyal
  -1 siblings, 1 reply; 11+ messages in thread
From: Tejun Heo @ 2014-07-02 23:45 UTC (permalink / raw)
  To: lizefan
  Cc: cgroups, linux-kernel, Tejun Heo, Vivek Goyal, Johannes Weiner,
	Michal Hocko

sane_behavior has been used as a development vehicle for the default
unified hierarchy.  Now that the default hierarchy is in place, the
flag became redundant and confusing as its usage is allowed on all
hierarchies.  There are gonna be either the default hierarchy or
legacy ones.  Let's make that clear by removing sane_behavior support
on non-default hierarchies.

This patch replaces cgroup_sane_behavior() with cgroup_on_dfl().  The
comment on top of CGRP_ROOT_SANE_BEHAVIOR is moved to on top of
cgroup_on_dfl() with sane_behavior specific part dropped.

On the default and legacy hierarchies w/o sane_behavior, this
shouldn't cause any behavior differences.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Vivek Goyal <vgoyal@redhat.com>
Cc: Li Zefan <lizefan@huawei.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Michal Hocko <mhocko@suse.cz>
---
 block/blk-throttle.c   |   6 +--
 include/linux/cgroup.h | 125 +++++++++++++++++++++----------------------------
 kernel/cgroup.c        |  19 ++++----
 kernel/cpuset.c        |  33 ++++++-------
 mm/memcontrol.c        |   7 +--
 5 files changed, 86 insertions(+), 104 deletions(-)

diff --git a/block/blk-throttle.c b/block/blk-throttle.c
index 3fdb21a..9273d09 100644
--- a/block/blk-throttle.c
+++ b/block/blk-throttle.c
@@ -412,13 +412,13 @@ static void throtl_pd_init(struct blkcg_gq *blkg)
 	int rw;
 
 	/*
-	 * If sane_hierarchy is enabled, we switch to properly hierarchical
+	 * If on the default hierarchy, we switch to properly hierarchical
 	 * behavior where limits on a given throtl_grp are applied to the
 	 * whole subtree rather than just the group itself.  e.g. If 16M
 	 * read_bps limit is set on the root group, the whole system can't
 	 * exceed 16M for the device.
 	 *
-	 * If sane_hierarchy is not enabled, the broken flat hierarchy
+	 * If not on the default hierarchy, the broken flat hierarchy
 	 * behavior is retained where all throtl_grps are treated as if
 	 * they're all separate root groups right below throtl_data.
 	 * Limits of a group don't interact with limits of other groups
@@ -426,7 +426,7 @@ static void throtl_pd_init(struct blkcg_gq *blkg)
 	 */
 	parent_sq = &td->service_queue;
 
-	if (cgroup_sane_behavior(blkg->blkcg->css.cgroup) && blkg->parent)
+	if (cgroup_on_dfl(blkg->blkcg->css.cgroup) && blkg->parent)
 		parent_sq = &blkg_to_tg(blkg->parent)->service_queue;
 
 	throtl_service_queue_init(&tg->service_queue, parent_sq);
diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index 7748e5b..46e4661 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -248,68 +248,7 @@ struct cgroup {
 
 /* cgroup_root->flags */
 enum {
-	/*
-	 * Unfortunately, cgroup core and various controllers are riddled
-	 * with idiosyncrasies and pointless options.  The following flag,
-	 * when set, will force sane behavior - some options are forced on,
-	 * others are disallowed, and some controllers will change their
-	 * hierarchical or other behaviors.
-	 *
-	 * The set of behaviors affected by this flag are still being
-	 * determined and developed and the mount option for this flag is
-	 * prefixed with __DEVEL__.  The prefix will be dropped once we
-	 * reach the point where all behaviors are compatible with the
-	 * planned unified hierarchy, which will automatically turn on this
-	 * flag.
-	 *
-	 * The followings are the behaviors currently affected this flag.
-	 *
-	 * - Mount options "noprefix", "xattr", "clone_children",
-	 *   "release_agent" and "name" are disallowed.
-	 *
-	 * - When mounting an existing superblock, mount options should
-	 *   match.
-	 *
-	 * - Remount is disallowed.
-	 *
-	 * - rename(2) is disallowed.
-	 *
-	 * - "tasks" is removed.  Everything should be at process
-	 *   granularity.  Use "cgroup.procs" instead.
-	 *
-	 * - "cgroup.procs" is not sorted.  pids will be unique unless they
-	 *   got recycled inbetween reads.
-	 *
-	 * - "release_agent" and "notify_on_release" are removed.
-	 *   Replacement notification mechanism will be implemented.
-	 *
-	 * - "cgroup.clone_children" is removed.
-	 *
-	 * - "cgroup.subtree_populated" is available.  Its value is 0 if
-	 *   the cgroup and its descendants contain no task; otherwise, 1.
-	 *   The file also generates kernfs notification which can be
-	 *   monitored through poll and [di]notify when the value of the
-	 *   file changes.
-	 *
-	 * - If mount is requested with sane_behavior but without any
-	 *   subsystem, the default unified hierarchy is mounted.
-	 *
-	 * - cpuset: tasks will be kept in empty cpusets when hotplug happens
-	 *   and take masks of ancestors with non-empty cpus/mems, instead of
-	 *   being moved to an ancestor.
-	 *
-	 * - cpuset: a task can be moved into an empty cpuset, and again it
-	 *   takes masks of ancestors.
-	 *
-	 * - memcg: use_hierarchy is on by default and the cgroup file for
-	 *   the flag is not created.
-	 *
-	 * - blkcg: blk-throttle becomes properly hierarchical.
-	 *
-	 * - debug: disallowed on the default hierarchy.
-	 */
-	CGRP_ROOT_SANE_BEHAVIOR	= (1 << 0),
-
+	CGRP_ROOT_SANE_BEHAVIOR	= (1 << 0), /* __DEVEL__sane_behavior specified */
 	CGRP_ROOT_NOPREFIX	= (1 << 1), /* mounted subsystems have no named prefix */
 	CGRP_ROOT_XATTR		= (1 << 2), /* supports extended attributes */
 };
@@ -523,20 +462,64 @@ struct cftype {
 extern struct cgroup_root cgrp_dfl_root;
 extern struct css_set init_css_set;
 
+/**
+ * cgroup_on_dfl - test whether a cgroup is on the default hierarchy
+ * @cgrp: the cgroup of interest
+ *
+ * The default hierarchy is the v2 interface of cgroup and this function
+ * can be used to test whether a cgroup is on the default hierarchy for
+ * cases where a subsystem should behave differnetly depending on the
+ * interface version.
+ *
+ * The set of behaviors which change on the default hierarchy are still
+ * being determined and the mount option is prefixed with __DEVEL__.
+ *
+ * List of changed behaviors:
+ *
+ * - Mount options "noprefix", "xattr", "clone_children", "release_agent"
+ *   and "name" are disallowed.
+ *
+ * - When mounting an existing superblock, mount options should match.
+ *
+ * - Remount is disallowed.
+ *
+ * - rename(2) is disallowed.
+ *
+ * - "tasks" is removed.  Everything should be at process granularity.  Use
+ *   "cgroup.procs" instead.
+ *
+ * - "cgroup.procs" is not sorted.  pids will be unique unless they got
+ *   recycled inbetween reads.
+ *
+ * - "release_agent" and "notify_on_release" are removed.  Replacement
+ *   notification mechanism will be implemented.
+ *
+ * - "cgroup.clone_children" is removed.
+ *
+ * - "cgroup.subtree_populated" is available.  Its value is 0 if the cgroup
+ *   and its descendants contain no task; otherwise, 1.  The file also
+ *   generates kernfs notification which can be monitored through poll and
+ *   [di]notify when the value of the file changes.
+ *
+ * - cpuset: tasks will be kept in empty cpusets when hotplug happens and
+ *   take masks of ancestors with non-empty cpus/mems, instead of being
+ *   moved to an ancestor.
+ *
+ * - cpuset: a task can be moved into an empty cpuset, and again it takes
+ *   masks of ancestors.
+ *
+ * - memcg: use_hierarchy is on by default and the cgroup file for the flag
+ *   is not created.
+ *
+ * - blkcg: blk-throttle becomes properly hierarchical.
+ *
+ * - debug: disallowed on the default hierarchy.
+ */
 static inline bool cgroup_on_dfl(const struct cgroup *cgrp)
 {
 	return cgrp->root == &cgrp_dfl_root;
 }
 
-/*
- * See the comment above CGRP_ROOT_SANE_BEHAVIOR for details.  This
- * function can be called as long as @cgrp is accessible.
- */
-static inline bool cgroup_sane_behavior(const struct cgroup *cgrp)
-{
-	return cgrp->root->flags & CGRP_ROOT_SANE_BEHAVIOR;
-}
-
 /* no synchronization, the result can only be used as a hint */
 static inline bool cgroup_has_tasks(struct cgroup *cgrp)
 {
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index be701d9..2d7057e 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -1414,8 +1414,8 @@ static int cgroup_remount(struct kernfs_root *kf_root, int *flags, char *data)
 	struct cgroup_sb_opts opts;
 	unsigned int added_mask, removed_mask;
 
-	if (root->flags & CGRP_ROOT_SANE_BEHAVIOR) {
-		pr_err("sane_behavior: remount is not allowed\n");
+	if (root == &cgrp_dfl_root) {
+		pr_err("remount is not allowed\n");
 		return -EINVAL;
 	}
 
@@ -2833,9 +2833,9 @@ static int cgroup_rename(struct kernfs_node *kn, struct kernfs_node *new_parent,
 
 	/*
 	 * This isn't a proper migration and its usefulness is very
-	 * limited.  Disallow if sane_behavior.
+	 * limited.  Disallow on the default hierarchy.
 	 */
-	if (cgroup_sane_behavior(cgrp))
+	if (cgroup_on_dfl(cgrp))
 		return -EPERM;
 
 	/*
@@ -2921,7 +2921,7 @@ static int cgroup_addrm_files(struct cgroup *cgrp, struct cftype cfts[],
 		/* does cft->flags tell us to skip this file on @cgrp? */
 		if ((cft->flags & CFTYPE_ONLY_ON_DFL) && !cgroup_on_dfl(cgrp))
 			continue;
-		if ((cft->flags & CFTYPE_INSANE) && cgroup_sane_behavior(cgrp))
+		if ((cft->flags & CFTYPE_INSANE) && cgroup_on_dfl(cgrp))
 			continue;
 		if ((cft->flags & CFTYPE_NOT_ON_ROOT) && !cgroup_parent(cgrp))
 			continue;
@@ -3654,8 +3654,9 @@ after:
  *
  * All this extra complexity was caused by the original implementation
  * committing to an entirely unnecessary property.  In the long term, we
- * want to do away with it.  Explicitly scramble sort order if
- * sane_behavior so that no such expectation exists in the new interface.
+ * want to do away with it.  Explicitly scramble sort order if on the
+ * default hierarchy so that no such expectation exists in the new
+ * interface.
  *
  * Scrambling is done by swapping every two consecutive bits, which is
  * non-identity one-to-one mapping which disturbs sort order sufficiently.
@@ -3670,7 +3671,7 @@ static pid_t pid_fry(pid_t pid)
 
 static pid_t cgroup_pid_fry(struct cgroup *cgrp, pid_t pid)
 {
-	if (cgroup_sane_behavior(cgrp))
+	if (cgroup_on_dfl(cgrp))
 		return pid_fry(pid);
 	else
 		return pid;
@@ -3773,7 +3774,7 @@ static int pidlist_array_load(struct cgroup *cgrp, enum cgroup_filetype type,
 	css_task_iter_end(&it);
 	length = n;
 	/* now sort & (if procs) strip out duplicates */
-	if (cgroup_sane_behavior(cgrp))
+	if (cgroup_on_dfl(cgrp))
 		sort(array, length, sizeof(pid_t), fried_cmppid, NULL);
 	else
 		sort(array, length, sizeof(pid_t), cmppid, NULL);
diff --git a/kernel/cpuset.c b/kernel/cpuset.c
index f6b33c6..f9d4807 100644
--- a/kernel/cpuset.c
+++ b/kernel/cpuset.c
@@ -1383,12 +1383,9 @@ static int cpuset_can_attach(struct cgroup_subsys_state *css,
 
 	mutex_lock(&cpuset_mutex);
 
-	/*
-	 * We allow to move tasks into an empty cpuset if sane_behavior
-	 * flag is set.
-	 */
+	/* allow moving tasks into an empty cpuset if on default hierarchy */
 	ret = -ENOSPC;
-	if (!cgroup_sane_behavior(css->cgroup) &&
+	if (!cgroup_on_dfl(css->cgroup) &&
 	    (cpumask_empty(cs->cpus_allowed) || nodes_empty(cs->mems_allowed)))
 		goto out_unlock;
 
@@ -2030,7 +2027,7 @@ static void cpuset_hotplug_update_tasks(struct cpuset *cs)
 	static cpumask_t off_cpus;
 	static nodemask_t off_mems;
 	bool is_empty;
-	bool sane = cgroup_sane_behavior(cs->css.cgroup);
+	bool on_dfl = cgroup_on_dfl(cs->css.cgroup);
 
 retry:
 	wait_event(cpuset_attach_wq, cs->attach_in_progress == 0);
@@ -2054,12 +2051,12 @@ retry:
 	mutex_unlock(&callback_mutex);
 
 	/*
-	 * If sane_behavior flag is set, we need to update tasks' cpumask
-	 * for empty cpuset to take on ancestor's cpumask. Otherwise, don't
-	 * call update_tasks_cpumask() if the cpuset becomes empty, as
-	 * the tasks in it will be migrated to an ancestor.
+	 * If on_dfl, we need to update tasks' cpumask for empty cpuset to
+	 * take on ancestor's cpumask. Otherwise, don't call
+	 * update_tasks_cpumask() if the cpuset becomes empty, as the tasks
+	 * in it will be migrated to an ancestor.
 	 */
-	if ((sane && cpumask_empty(cs->cpus_allowed)) ||
+	if ((on_dfl && cpumask_empty(cs->cpus_allowed)) ||
 	    (!cpumask_empty(&off_cpus) && !cpumask_empty(cs->cpus_allowed)))
 		update_tasks_cpumask(cs);
 
@@ -2068,12 +2065,12 @@ retry:
 	mutex_unlock(&callback_mutex);
 
 	/*
-	 * If sane_behavior flag is set, we need to update tasks' nodemask
-	 * for empty cpuset to take on ancestor's nodemask. Otherwise, don't
-	 * call update_tasks_nodemask() if the cpuset becomes empty, as
-	 * the tasks in it will be migratd to an ancestor.
+	 * If on_dfl, we need to update tasks' nodemask for empty cpuset to
+	 * take on ancestor's nodemask. Otherwise, don't call
+	 * update_tasks_nodemask() if the cpuset becomes empty, as the
+	 * tasks in it will be migratd to an ancestor.
 	 */
-	if ((sane && nodes_empty(cs->mems_allowed)) ||
+	if ((on_dfl && nodes_empty(cs->mems_allowed)) ||
 	    (!nodes_empty(off_mems) && !nodes_empty(cs->mems_allowed)))
 		update_tasks_nodemask(cs);
 
@@ -2083,13 +2080,13 @@ retry:
 	mutex_unlock(&cpuset_mutex);
 
 	/*
-	 * If sane_behavior flag is set, we'll keep tasks in empty cpusets.
+	 * If on_dfl, we'll keep tasks in empty cpusets.
 	 *
 	 * Otherwise move tasks to the nearest ancestor with execution
 	 * resources.  This is full cgroup operation which will
 	 * also call back into cpuset.  Should be done outside any lock.
 	 */
-	if (!sane && is_empty)
+	if (!on_dfl && is_empty)
 		remove_tasks_in_empty_cpuset(cs);
 }
 
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index a2c7bcb..f986671 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -7001,16 +7001,17 @@ static void mem_cgroup_move_task(struct cgroup_subsys_state *css,
 
 /*
  * Cgroup retains root cgroups across [un]mount cycles making it necessary
- * to verify sane_behavior flag on each mount attempt.
+ * to verify whether we're attached to the default hierarchy on each mount
+ * attempt.
  */
 static void mem_cgroup_bind(struct cgroup_subsys_state *root_css)
 {
 	/*
-	 * use_hierarchy is forced with sane_behavior.  cgroup core
+	 * use_hierarchy is forced on the default hierarchy.  cgroup core
 	 * guarantees that @root doesn't have any children, so turning it
 	 * on for the root memcg is enough.
 	 */
-	if (cgroup_sane_behavior(root_css->cgroup))
+	if (cgroup_on_dfl(root_css->cgroup))
 		mem_cgroup_from_css(root_css)->use_hierarchy = true;
 }
 
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 4/4] cgroup: clean up sane_behavior handling
  2014-07-02 23:45 ` Tejun Heo
                   ` (3 preceding siblings ...)
  (?)
@ 2014-07-02 23:45 ` Tejun Heo
  -1 siblings, 0 replies; 11+ messages in thread
From: Tejun Heo @ 2014-07-02 23:45 UTC (permalink / raw)
  To: lizefan; +Cc: cgroups, linux-kernel, Tejun Heo

After the previous patch to remove sane_behavior support from
non-default hierarchies, CGRP_ROOT_SANE_BEHAVIOR is used only to
indicate the default hierarchy while parsing mount options.  This
patch makes the following cleanups around it.

* Don't show it in the mount option.  Eventually the default hierarchy
  will be assigned a different filesystem type.

* As sane_behavior is no longer effective on non-default hierarchies
  and the default hierarchy doesn't accept any mount options,
  parse_cgroupfs_options() can consider sane_behavior mount option as
  indicating the default hierarchy and fail if any other options are
  specified with it.  While at it, remove one of the double blank
  lines in the function.

* cgroup_mount() can now simply test CGRP_ROOT_SANE_BEHAVIOR to tell
  whether to mount the default hierarchy or not.

* As CGROUP_ROOT_SANE_BEHAVIOR's only role now is indicating whether
  to select the default hierarchy or not during mount, it doesn't need
  to be set in the default hierarchy itself.  cgroup_init_early()
  updated accordingly.

Signed-off-by: Tejun Heo <tj@kernel.org>
---
 kernel/cgroup.c | 66 +++++++++++++++++++++++----------------------------------
 1 file changed, 27 insertions(+), 39 deletions(-)

diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 2d7057e..1c56924 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -1233,8 +1233,6 @@ static int cgroup_show_options(struct seq_file *seq,
 	for_each_subsys(ss, ssid)
 		if (root->subsys_mask & (1 << ssid))
 			seq_printf(seq, ",%s", ss->name);
-	if (root->flags & CGRP_ROOT_SANE_BEHAVIOR)
-		seq_puts(seq, ",sane_behavior");
 	if (root->flags & CGRP_ROOT_NOPREFIX)
 		seq_puts(seq, ",noprefix");
 	if (root->flags & CGRP_ROOT_XATTR)
@@ -1268,6 +1266,7 @@ static int parse_cgroupfs_options(char *data, struct cgroup_sb_opts *opts)
 	bool all_ss = false, one_ss = false;
 	unsigned int mask = -1U;
 	struct cgroup_subsys *ss;
+	int nr_opts = 0;
 	int i;
 
 #ifdef CONFIG_CPUSETS
@@ -1277,6 +1276,8 @@ static int parse_cgroupfs_options(char *data, struct cgroup_sb_opts *opts)
 	memset(opts, 0, sizeof(*opts));
 
 	while ((token = strsep(&o, ",")) != NULL) {
+		nr_opts++;
+
 		if (!*token)
 			return -EINVAL;
 		if (!strcmp(token, "none")) {
@@ -1361,37 +1362,33 @@ static int parse_cgroupfs_options(char *data, struct cgroup_sb_opts *opts)
 			return -ENOENT;
 	}
 
-	/* Consistency checks */
-
 	if (opts->flags & CGRP_ROOT_SANE_BEHAVIOR) {
 		pr_warn("sane_behavior: this is still under development and its behaviors will change, proceed at your own risk\n");
-
-		if ((opts->flags & (CGRP_ROOT_NOPREFIX | CGRP_ROOT_XATTR)) ||
-		    opts->cpuset_clone_children || opts->release_agent ||
-		    opts->name) {
-			pr_err("sane_behavior: noprefix, xattr, clone_children, release_agent and name are not allowed\n");
+		if (nr_opts != 1) {
+			pr_err("sane_behavior: no other mount options allowed\n");
 			return -EINVAL;
 		}
-	} else {
-		/*
-		 * If the 'all' option was specified select all the
-		 * subsystems, otherwise if 'none', 'name=' and a subsystem
-		 * name options were not specified, let's default to 'all'
-		 */
-		if (all_ss || (!one_ss && !opts->none && !opts->name))
-			for_each_subsys(ss, i)
-				if (!ss->disabled)
-					opts->subsys_mask |= (1 << i);
-
-		/*
-		 * We either have to specify by name or by subsystems. (So
-		 * all empty hierarchies must have a name).
-		 */
-		if (!opts->subsys_mask && !opts->name)
-			return -EINVAL;
+		return 0;
 	}
 
 	/*
+	 * If the 'all' option was specified select all the subsystems,
+	 * otherwise if 'none', 'name=' and a subsystem name options were
+	 * not specified, let's default to 'all'
+	 */
+	if (all_ss || (!one_ss && !opts->none && !opts->name))
+		for_each_subsys(ss, i)
+			if (!ss->disabled)
+				opts->subsys_mask |= (1 << i);
+
+	/*
+	 * We either have to specify by name or by subsystems. (So all
+	 * empty hierarchies must have a name).
+	 */
+	if (!opts->subsys_mask && !opts->name)
+		return -EINVAL;
+
+	/*
 	 * Option noprefix was introduced just for backward compatibility
 	 * with the old cpuset, so we allow noprefix only if mounting just
 	 * the cpuset subsystem.
@@ -1399,7 +1396,6 @@ static int parse_cgroupfs_options(char *data, struct cgroup_sb_opts *opts)
 	if ((opts->flags & CGRP_ROOT_NOPREFIX) && (opts->subsys_mask & mask))
 		return -EINVAL;
 
-
 	/* Can't specify "none" and some subsystems */
 	if (opts->subsys_mask && opts->none)
 		return -EINVAL;
@@ -1668,7 +1664,7 @@ static struct dentry *cgroup_mount(struct file_system_type *fs_type,
 		goto out_unlock;
 
 	/* look for a matching existing root */
-	if (!opts.subsys_mask && !opts.none && !opts.name) {
+	if (opts.flags & CGRP_ROOT_SANE_BEHAVIOR) {
 		cgrp_dfl_root_visible = true;
 		root = &cgrp_dfl_root;
 		cgroup_get(&root->cgrp);
@@ -1705,15 +1701,8 @@ static struct dentry *cgroup_mount(struct file_system_type *fs_type,
 			goto out_unlock;
 		}
 
-		if (root->flags ^ opts.flags) {
-			if ((root->flags | opts.flags) & CGRP_ROOT_SANE_BEHAVIOR) {
-				pr_err("sane_behavior: new mount options should match the existing superblock\n");
-				ret = -EINVAL;
-				goto out_unlock;
-			} else {
-				pr_warn("new mount options do not match the existing superblock, will be ignored\n");
-			}
-		}
+		if (root->flags ^ opts.flags)
+			pr_warn("new mount options do not match the existing superblock, will be ignored\n");
 
 		/*
 		 * A root's lifetime is governed by its root cgroup.
@@ -4692,8 +4681,7 @@ static void __init cgroup_init_subsys(struct cgroup_subsys *ss, bool early)
  */
 int __init cgroup_init_early(void)
 {
-	static struct cgroup_sb_opts __initdata opts =
-		{ .flags = CGRP_ROOT_SANE_BEHAVIOR };
+	static struct cgroup_sb_opts __initdata opts;
 	struct cgroup_subsys *ss;
 	int i;
 
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/4] cgroup: remove sane_behavior support on non-default hierarchies
  2014-07-02 23:45 ` [PATCH 3/4] cgroup: remove sane_behavior support on non-default hierarchies Tejun Heo
@ 2014-07-08 19:57   ` Vivek Goyal
  0 siblings, 0 replies; 11+ messages in thread
From: Vivek Goyal @ 2014-07-08 19:57 UTC (permalink / raw)
  To: Tejun Heo; +Cc: lizefan, cgroups, linux-kernel, Johannes Weiner, Michal Hocko

On Wed, Jul 02, 2014 at 07:45:46PM -0400, Tejun Heo wrote:
> sane_behavior has been used as a development vehicle for the default
> unified hierarchy.  Now that the default hierarchy is in place, the
> flag became redundant and confusing as its usage is allowed on all
> hierarchies.  There are gonna be either the default hierarchy or
> legacy ones.  Let's make that clear by removing sane_behavior support
> on non-default hierarchies.
> 
> This patch replaces cgroup_sane_behavior() with cgroup_on_dfl().  The
> comment on top of CGRP_ROOT_SANE_BEHAVIOR is moved to on top of
> cgroup_on_dfl() with sane_behavior specific part dropped.
> 
> On the default and legacy hierarchies w/o sane_behavior, this
> shouldn't cause any behavior differences.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Cc: Vivek Goyal <vgoyal@redhat.com>
> Cc: Li Zefan <lizefan@huawei.com>
> Cc: Johannes Weiner <hannes@cmpxchg.org>
> Cc: Michal Hocko <mhocko@suse.cz>
> ---
>  block/blk-throttle.c   |   6 +--

blk-throttle change looks good to me.

Acked-by: Vivek Goyal <vgoyal@redhat.com>

Vivek

>  include/linux/cgroup.h | 125 +++++++++++++++++++++----------------------------
>  kernel/cgroup.c        |  19 ++++----
>  kernel/cpuset.c        |  33 ++++++-------
>  mm/memcontrol.c        |   7 +--
>  5 files changed, 86 insertions(+), 104 deletions(-)
> 
> diff --git a/block/blk-throttle.c b/block/blk-throttle.c
> index 3fdb21a..9273d09 100644
> --- a/block/blk-throttle.c
> +++ b/block/blk-throttle.c
> @@ -412,13 +412,13 @@ static void throtl_pd_init(struct blkcg_gq *blkg)
>  	int rw;
>  
>  	/*
> -	 * If sane_hierarchy is enabled, we switch to properly hierarchical
> +	 * If on the default hierarchy, we switch to properly hierarchical
>  	 * behavior where limits on a given throtl_grp are applied to the
>  	 * whole subtree rather than just the group itself.  e.g. If 16M
>  	 * read_bps limit is set on the root group, the whole system can't
>  	 * exceed 16M for the device.
>  	 *
> -	 * If sane_hierarchy is not enabled, the broken flat hierarchy
> +	 * If not on the default hierarchy, the broken flat hierarchy
>  	 * behavior is retained where all throtl_grps are treated as if
>  	 * they're all separate root groups right below throtl_data.
>  	 * Limits of a group don't interact with limits of other groups
> @@ -426,7 +426,7 @@ static void throtl_pd_init(struct blkcg_gq *blkg)
>  	 */
>  	parent_sq = &td->service_queue;
>  
> -	if (cgroup_sane_behavior(blkg->blkcg->css.cgroup) && blkg->parent)
> +	if (cgroup_on_dfl(blkg->blkcg->css.cgroup) && blkg->parent)
>  		parent_sq = &blkg_to_tg(blkg->parent)->service_queue;
>  
>  	throtl_service_queue_init(&tg->service_queue, parent_sq);
> diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
> index 7748e5b..46e4661 100644
> --- a/include/linux/cgroup.h
> +++ b/include/linux/cgroup.h
> @@ -248,68 +248,7 @@ struct cgroup {
>  
>  /* cgroup_root->flags */
>  enum {
> -	/*
> -	 * Unfortunately, cgroup core and various controllers are riddled
> -	 * with idiosyncrasies and pointless options.  The following flag,
> -	 * when set, will force sane behavior - some options are forced on,
> -	 * others are disallowed, and some controllers will change their
> -	 * hierarchical or other behaviors.
> -	 *
> -	 * The set of behaviors affected by this flag are still being
> -	 * determined and developed and the mount option for this flag is
> -	 * prefixed with __DEVEL__.  The prefix will be dropped once we
> -	 * reach the point where all behaviors are compatible with the
> -	 * planned unified hierarchy, which will automatically turn on this
> -	 * flag.
> -	 *
> -	 * The followings are the behaviors currently affected this flag.
> -	 *
> -	 * - Mount options "noprefix", "xattr", "clone_children",
> -	 *   "release_agent" and "name" are disallowed.
> -	 *
> -	 * - When mounting an existing superblock, mount options should
> -	 *   match.
> -	 *
> -	 * - Remount is disallowed.
> -	 *
> -	 * - rename(2) is disallowed.
> -	 *
> -	 * - "tasks" is removed.  Everything should be at process
> -	 *   granularity.  Use "cgroup.procs" instead.
> -	 *
> -	 * - "cgroup.procs" is not sorted.  pids will be unique unless they
> -	 *   got recycled inbetween reads.
> -	 *
> -	 * - "release_agent" and "notify_on_release" are removed.
> -	 *   Replacement notification mechanism will be implemented.
> -	 *
> -	 * - "cgroup.clone_children" is removed.
> -	 *
> -	 * - "cgroup.subtree_populated" is available.  Its value is 0 if
> -	 *   the cgroup and its descendants contain no task; otherwise, 1.
> -	 *   The file also generates kernfs notification which can be
> -	 *   monitored through poll and [di]notify when the value of the
> -	 *   file changes.
> -	 *
> -	 * - If mount is requested with sane_behavior but without any
> -	 *   subsystem, the default unified hierarchy is mounted.
> -	 *
> -	 * - cpuset: tasks will be kept in empty cpusets when hotplug happens
> -	 *   and take masks of ancestors with non-empty cpus/mems, instead of
> -	 *   being moved to an ancestor.
> -	 *
> -	 * - cpuset: a task can be moved into an empty cpuset, and again it
> -	 *   takes masks of ancestors.
> -	 *
> -	 * - memcg: use_hierarchy is on by default and the cgroup file for
> -	 *   the flag is not created.
> -	 *
> -	 * - blkcg: blk-throttle becomes properly hierarchical.
> -	 *
> -	 * - debug: disallowed on the default hierarchy.
> -	 */
> -	CGRP_ROOT_SANE_BEHAVIOR	= (1 << 0),
> -
> +	CGRP_ROOT_SANE_BEHAVIOR	= (1 << 0), /* __DEVEL__sane_behavior specified */
>  	CGRP_ROOT_NOPREFIX	= (1 << 1), /* mounted subsystems have no named prefix */
>  	CGRP_ROOT_XATTR		= (1 << 2), /* supports extended attributes */
>  };
> @@ -523,20 +462,64 @@ struct cftype {
>  extern struct cgroup_root cgrp_dfl_root;
>  extern struct css_set init_css_set;
>  
> +/**
> + * cgroup_on_dfl - test whether a cgroup is on the default hierarchy
> + * @cgrp: the cgroup of interest
> + *
> + * The default hierarchy is the v2 interface of cgroup and this function
> + * can be used to test whether a cgroup is on the default hierarchy for
> + * cases where a subsystem should behave differnetly depending on the
> + * interface version.
> + *
> + * The set of behaviors which change on the default hierarchy are still
> + * being determined and the mount option is prefixed with __DEVEL__.
> + *
> + * List of changed behaviors:
> + *
> + * - Mount options "noprefix", "xattr", "clone_children", "release_agent"
> + *   and "name" are disallowed.
> + *
> + * - When mounting an existing superblock, mount options should match.
> + *
> + * - Remount is disallowed.
> + *
> + * - rename(2) is disallowed.
> + *
> + * - "tasks" is removed.  Everything should be at process granularity.  Use
> + *   "cgroup.procs" instead.
> + *
> + * - "cgroup.procs" is not sorted.  pids will be unique unless they got
> + *   recycled inbetween reads.
> + *
> + * - "release_agent" and "notify_on_release" are removed.  Replacement
> + *   notification mechanism will be implemented.
> + *
> + * - "cgroup.clone_children" is removed.
> + *
> + * - "cgroup.subtree_populated" is available.  Its value is 0 if the cgroup
> + *   and its descendants contain no task; otherwise, 1.  The file also
> + *   generates kernfs notification which can be monitored through poll and
> + *   [di]notify when the value of the file changes.
> + *
> + * - cpuset: tasks will be kept in empty cpusets when hotplug happens and
> + *   take masks of ancestors with non-empty cpus/mems, instead of being
> + *   moved to an ancestor.
> + *
> + * - cpuset: a task can be moved into an empty cpuset, and again it takes
> + *   masks of ancestors.
> + *
> + * - memcg: use_hierarchy is on by default and the cgroup file for the flag
> + *   is not created.
> + *
> + * - blkcg: blk-throttle becomes properly hierarchical.
> + *
> + * - debug: disallowed on the default hierarchy.
> + */
>  static inline bool cgroup_on_dfl(const struct cgroup *cgrp)
>  {
>  	return cgrp->root == &cgrp_dfl_root;
>  }
>  
> -/*
> - * See the comment above CGRP_ROOT_SANE_BEHAVIOR for details.  This
> - * function can be called as long as @cgrp is accessible.
> - */
> -static inline bool cgroup_sane_behavior(const struct cgroup *cgrp)
> -{
> -	return cgrp->root->flags & CGRP_ROOT_SANE_BEHAVIOR;
> -}
> -
>  /* no synchronization, the result can only be used as a hint */
>  static inline bool cgroup_has_tasks(struct cgroup *cgrp)
>  {
> diff --git a/kernel/cgroup.c b/kernel/cgroup.c
> index be701d9..2d7057e 100644
> --- a/kernel/cgroup.c
> +++ b/kernel/cgroup.c
> @@ -1414,8 +1414,8 @@ static int cgroup_remount(struct kernfs_root *kf_root, int *flags, char *data)
>  	struct cgroup_sb_opts opts;
>  	unsigned int added_mask, removed_mask;
>  
> -	if (root->flags & CGRP_ROOT_SANE_BEHAVIOR) {
> -		pr_err("sane_behavior: remount is not allowed\n");
> +	if (root == &cgrp_dfl_root) {
> +		pr_err("remount is not allowed\n");
>  		return -EINVAL;
>  	}
>  
> @@ -2833,9 +2833,9 @@ static int cgroup_rename(struct kernfs_node *kn, struct kernfs_node *new_parent,
>  
>  	/*
>  	 * This isn't a proper migration and its usefulness is very
> -	 * limited.  Disallow if sane_behavior.
> +	 * limited.  Disallow on the default hierarchy.
>  	 */
> -	if (cgroup_sane_behavior(cgrp))
> +	if (cgroup_on_dfl(cgrp))
>  		return -EPERM;
>  
>  	/*
> @@ -2921,7 +2921,7 @@ static int cgroup_addrm_files(struct cgroup *cgrp, struct cftype cfts[],
>  		/* does cft->flags tell us to skip this file on @cgrp? */
>  		if ((cft->flags & CFTYPE_ONLY_ON_DFL) && !cgroup_on_dfl(cgrp))
>  			continue;
> -		if ((cft->flags & CFTYPE_INSANE) && cgroup_sane_behavior(cgrp))
> +		if ((cft->flags & CFTYPE_INSANE) && cgroup_on_dfl(cgrp))
>  			continue;
>  		if ((cft->flags & CFTYPE_NOT_ON_ROOT) && !cgroup_parent(cgrp))
>  			continue;
> @@ -3654,8 +3654,9 @@ after:
>   *
>   * All this extra complexity was caused by the original implementation
>   * committing to an entirely unnecessary property.  In the long term, we
> - * want to do away with it.  Explicitly scramble sort order if
> - * sane_behavior so that no such expectation exists in the new interface.
> + * want to do away with it.  Explicitly scramble sort order if on the
> + * default hierarchy so that no such expectation exists in the new
> + * interface.
>   *
>   * Scrambling is done by swapping every two consecutive bits, which is
>   * non-identity one-to-one mapping which disturbs sort order sufficiently.
> @@ -3670,7 +3671,7 @@ static pid_t pid_fry(pid_t pid)
>  
>  static pid_t cgroup_pid_fry(struct cgroup *cgrp, pid_t pid)
>  {
> -	if (cgroup_sane_behavior(cgrp))
> +	if (cgroup_on_dfl(cgrp))
>  		return pid_fry(pid);
>  	else
>  		return pid;
> @@ -3773,7 +3774,7 @@ static int pidlist_array_load(struct cgroup *cgrp, enum cgroup_filetype type,
>  	css_task_iter_end(&it);
>  	length = n;
>  	/* now sort & (if procs) strip out duplicates */
> -	if (cgroup_sane_behavior(cgrp))
> +	if (cgroup_on_dfl(cgrp))
>  		sort(array, length, sizeof(pid_t), fried_cmppid, NULL);
>  	else
>  		sort(array, length, sizeof(pid_t), cmppid, NULL);
> diff --git a/kernel/cpuset.c b/kernel/cpuset.c
> index f6b33c6..f9d4807 100644
> --- a/kernel/cpuset.c
> +++ b/kernel/cpuset.c
> @@ -1383,12 +1383,9 @@ static int cpuset_can_attach(struct cgroup_subsys_state *css,
>  
>  	mutex_lock(&cpuset_mutex);
>  
> -	/*
> -	 * We allow to move tasks into an empty cpuset if sane_behavior
> -	 * flag is set.
> -	 */
> +	/* allow moving tasks into an empty cpuset if on default hierarchy */
>  	ret = -ENOSPC;
> -	if (!cgroup_sane_behavior(css->cgroup) &&
> +	if (!cgroup_on_dfl(css->cgroup) &&
>  	    (cpumask_empty(cs->cpus_allowed) || nodes_empty(cs->mems_allowed)))
>  		goto out_unlock;
>  
> @@ -2030,7 +2027,7 @@ static void cpuset_hotplug_update_tasks(struct cpuset *cs)
>  	static cpumask_t off_cpus;
>  	static nodemask_t off_mems;
>  	bool is_empty;
> -	bool sane = cgroup_sane_behavior(cs->css.cgroup);
> +	bool on_dfl = cgroup_on_dfl(cs->css.cgroup);
>  
>  retry:
>  	wait_event(cpuset_attach_wq, cs->attach_in_progress == 0);
> @@ -2054,12 +2051,12 @@ retry:
>  	mutex_unlock(&callback_mutex);
>  
>  	/*
> -	 * If sane_behavior flag is set, we need to update tasks' cpumask
> -	 * for empty cpuset to take on ancestor's cpumask. Otherwise, don't
> -	 * call update_tasks_cpumask() if the cpuset becomes empty, as
> -	 * the tasks in it will be migrated to an ancestor.
> +	 * If on_dfl, we need to update tasks' cpumask for empty cpuset to
> +	 * take on ancestor's cpumask. Otherwise, don't call
> +	 * update_tasks_cpumask() if the cpuset becomes empty, as the tasks
> +	 * in it will be migrated to an ancestor.
>  	 */
> -	if ((sane && cpumask_empty(cs->cpus_allowed)) ||
> +	if ((on_dfl && cpumask_empty(cs->cpus_allowed)) ||
>  	    (!cpumask_empty(&off_cpus) && !cpumask_empty(cs->cpus_allowed)))
>  		update_tasks_cpumask(cs);
>  
> @@ -2068,12 +2065,12 @@ retry:
>  	mutex_unlock(&callback_mutex);
>  
>  	/*
> -	 * If sane_behavior flag is set, we need to update tasks' nodemask
> -	 * for empty cpuset to take on ancestor's nodemask. Otherwise, don't
> -	 * call update_tasks_nodemask() if the cpuset becomes empty, as
> -	 * the tasks in it will be migratd to an ancestor.
> +	 * If on_dfl, we need to update tasks' nodemask for empty cpuset to
> +	 * take on ancestor's nodemask. Otherwise, don't call
> +	 * update_tasks_nodemask() if the cpuset becomes empty, as the
> +	 * tasks in it will be migratd to an ancestor.
>  	 */
> -	if ((sane && nodes_empty(cs->mems_allowed)) ||
> +	if ((on_dfl && nodes_empty(cs->mems_allowed)) ||
>  	    (!nodes_empty(off_mems) && !nodes_empty(cs->mems_allowed)))
>  		update_tasks_nodemask(cs);
>  
> @@ -2083,13 +2080,13 @@ retry:
>  	mutex_unlock(&cpuset_mutex);
>  
>  	/*
> -	 * If sane_behavior flag is set, we'll keep tasks in empty cpusets.
> +	 * If on_dfl, we'll keep tasks in empty cpusets.
>  	 *
>  	 * Otherwise move tasks to the nearest ancestor with execution
>  	 * resources.  This is full cgroup operation which will
>  	 * also call back into cpuset.  Should be done outside any lock.
>  	 */
> -	if (!sane && is_empty)
> +	if (!on_dfl && is_empty)
>  		remove_tasks_in_empty_cpuset(cs);
>  }
>  
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index a2c7bcb..f986671 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -7001,16 +7001,17 @@ static void mem_cgroup_move_task(struct cgroup_subsys_state *css,
>  
>  /*
>   * Cgroup retains root cgroups across [un]mount cycles making it necessary
> - * to verify sane_behavior flag on each mount attempt.
> + * to verify whether we're attached to the default hierarchy on each mount
> + * attempt.
>   */
>  static void mem_cgroup_bind(struct cgroup_subsys_state *root_css)
>  {
>  	/*
> -	 * use_hierarchy is forced with sane_behavior.  cgroup core
> +	 * use_hierarchy is forced on the default hierarchy.  cgroup core
>  	 * guarantees that @root doesn't have any children, so turning it
>  	 * on for the root memcg is enough.
>  	 */
> -	if (cgroup_sane_behavior(root_css->cgroup))
> +	if (cgroup_on_dfl(root_css->cgroup))
>  		mem_cgroup_from_css(root_css)->use_hierarchy = true;
>  }
>  
> -- 
> 1.9.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe cgroups" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCHSET cgroup/for-3.17] cgroup: remove sane_behavior support on non-default hierarchies
  2014-07-02 23:45 ` Tejun Heo
@ 2014-07-09  7:25   ` Li Zefan
  -1 siblings, 0 replies; 11+ messages in thread
From: Li Zefan @ 2014-07-09  7:25 UTC (permalink / raw)
  To: Tejun Heo; +Cc: cgroups, linux-kernel

On 2014/7/3 7:45, Tejun Heo wrote:
> Hello,
> 
> sane_behavior has been used as a development vehicle for the default
> unified hierarchy.  Now that the default hierarchy is in place, the
> flag became redundant and confusing as its usage is allowed on all
> hierarchies.  There are gonna be either the default hierarchy or
> legacy ones.  Let's make that clear by removing sane_behavior support
> on non-default hierarchies.
> 
> This patchset contains the following four patches.
> 
>  0001-cgroup-remove-CGRP_ROOT_OPTION_MASK.patch
>  0002-cgroup-make-interface-file-cgroup.sane_behavior-lega.patch
>  0003-cgroup-remove-sane_behavior-support-on-non-default-h.patch
>  0004-cgroup-clean-up-sane_behavior-handling.patch
> 
> 0001 is a trivial cleanup.
> 
> 0002 removes "cgroup.sane_behavior" from the default hierarchy.
> 
> 0003 removes sane_behavior support on non-default hierarchies.
> 
> 0004 cleans up sane_behavior handling.
> 
> This patchset is on top of a497c3ba1d97 ("Linux 3.16-rc2") and
> available in the following git branch.
> 
>  git://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup.git review-dfl-instead-of-sane
> 
> diffstat follows.  Thanks.
> 
>  block/blk-throttle.c   |    6 +-
>  include/linux/cgroup.h |  128 ++++++++++++++++++++-----------------------------
>  kernel/cgroup.c        |   96 +++++++++++++++---------------------
>  kernel/cpuset.c        |   33 +++++-------
>  mm/memcontrol.c        |    7 +-
>  5 files changed, 117 insertions(+), 153 deletions(-)
> 

Acked-by: Li Zefan <lizefan@huawei.com>

I'm rebasing my cpuset patchset against this.


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCHSET cgroup/for-3.17] cgroup: remove sane_behavior support on non-default hierarchies
@ 2014-07-09  7:25   ` Li Zefan
  0 siblings, 0 replies; 11+ messages in thread
From: Li Zefan @ 2014-07-09  7:25 UTC (permalink / raw)
  To: Tejun Heo; +Cc: cgroups, linux-kernel

On 2014/7/3 7:45, Tejun Heo wrote:
> Hello,
> 
> sane_behavior has been used as a development vehicle for the default
> unified hierarchy.  Now that the default hierarchy is in place, the
> flag became redundant and confusing as its usage is allowed on all
> hierarchies.  There are gonna be either the default hierarchy or
> legacy ones.  Let's make that clear by removing sane_behavior support
> on non-default hierarchies.
> 
> This patchset contains the following four patches.
> 
>  0001-cgroup-remove-CGRP_ROOT_OPTION_MASK.patch
>  0002-cgroup-make-interface-file-cgroup.sane_behavior-lega.patch
>  0003-cgroup-remove-sane_behavior-support-on-non-default-h.patch
>  0004-cgroup-clean-up-sane_behavior-handling.patch
> 
> 0001 is a trivial cleanup.
> 
> 0002 removes "cgroup.sane_behavior" from the default hierarchy.
> 
> 0003 removes sane_behavior support on non-default hierarchies.
> 
> 0004 cleans up sane_behavior handling.
> 
> This patchset is on top of a497c3ba1d97 ("Linux 3.16-rc2") and
> available in the following git branch.
> 
>  git://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup.git review-dfl-instead-of-sane
> 
> diffstat follows.  Thanks.
> 
>  block/blk-throttle.c   |    6 +-
>  include/linux/cgroup.h |  128 ++++++++++++++++++++-----------------------------
>  kernel/cgroup.c        |   96 +++++++++++++++---------------------
>  kernel/cpuset.c        |   33 +++++-------
>  mm/memcontrol.c        |    7 +-
>  5 files changed, 117 insertions(+), 153 deletions(-)
> 

Acked-by: Li Zefan <lizefan@huawei.com>

I'm rebasing my cpuset patchset against this.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCHSET cgroup/for-3.17] cgroup: remove sane_behavior support on non-default hierarchies
@ 2014-07-09 14:08   ` Tejun Heo
  0 siblings, 0 replies; 11+ messages in thread
From: Tejun Heo @ 2014-07-09 14:08 UTC (permalink / raw)
  To: lizefan; +Cc: cgroups, linux-kernel

Applied to cgroup/for-3.17.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCHSET cgroup/for-3.17] cgroup: remove sane_behavior support on non-default hierarchies
@ 2014-07-09 14:08   ` Tejun Heo
  0 siblings, 0 replies; 11+ messages in thread
From: Tejun Heo @ 2014-07-09 14:08 UTC (permalink / raw)
  To: lizefan-hv44wF8Li93QT0dZR+AlfA
  Cc: cgroups-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA

Applied to cgroup/for-3.17.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2014-07-09 14:08 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-02 23:45 [PATCHSET cgroup/for-3.17] cgroup: remove sane_behavior support on non-default hierarchies Tejun Heo
2014-07-02 23:45 ` Tejun Heo
2014-07-02 23:45 ` [PATCH 1/4] cgroup: remove CGRP_ROOT_OPTION_MASK Tejun Heo
2014-07-02 23:45 ` [PATCH 2/4] cgroup: make interface file "cgroup.sane_behavior" legacy-only Tejun Heo
2014-07-02 23:45 ` [PATCH 3/4] cgroup: remove sane_behavior support on non-default hierarchies Tejun Heo
2014-07-08 19:57   ` Vivek Goyal
2014-07-02 23:45 ` [PATCH 4/4] cgroup: clean up sane_behavior handling Tejun Heo
2014-07-09  7:25 ` [PATCHSET cgroup/for-3.17] cgroup: remove sane_behavior support on non-default hierarchies Li Zefan
2014-07-09  7:25   ` Li Zefan
2014-07-09 14:08 ` Tejun Heo
2014-07-09 14:08   ` Tejun Heo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.