* [PATCH 1/9] cgroup: minor updates around cgroup_clear_directory()
[not found] ` <1372463145-4245-1-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
@ 2013-06-28 23:45 ` Tejun Heo
[not found] ` <1372463145-4245-2-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2013-06-28 23:45 ` Tejun Heo
` (20 subsequent siblings)
21 siblings, 1 reply; 45+ messages in thread
From: Tejun Heo @ 2013-06-28 23:45 UTC (permalink / raw)
To: lizefan-hv44wF8Li93QT0dZR+AlfA
Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
cgroups-u79uwXL29TY76Z2rM5mHXA, Tejun Heo
* Rename it to cgroup_clear_dir() and make it take the pointer to the
target cgroup instead of the the dentry. This makes the function
consistent with its counterpart - cgroup_populate_dir().
* Move cgroup_clear_directory() invocation from cgroup_d_remove_dir()
to cgroup_remount() so that the function doesn't have to determine
the cgroup pointer back from the dentry. cgroup_d_remove_dir() now
only deals with vfs, which is slightly cleaner.
This patch doesn't introduce any functional differences.
Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
kernel/cgroup.c | 19 ++++++++-----------
1 file changed, 8 insertions(+), 11 deletions(-)
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 5a2fcf5..64877de 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -957,15 +957,14 @@ static void cgroup_rm_file(struct cgroup *cgrp, const struct cftype *cft)
}
/**
- * cgroup_clear_directory - selective removal of base and subsystem files
- * @dir: directory containing the files
+ * cgroup_clear_dir - selective removal of base and subsystem files
+ * @cgrp: target cgroup
* @base_files: true if the base files should be removed
* @subsys_mask: mask of the subsystem ids whose files should be removed
*/
-static void cgroup_clear_directory(struct dentry *dir, bool base_files,
- unsigned long subsys_mask)
+static void cgroup_clear_dir(struct cgroup *cgrp, bool base_files,
+ unsigned long subsys_mask)
{
- struct cgroup *cgrp = __d_cgrp(dir);
struct cgroup_subsys *ss;
for_each_root_subsys(cgrp->root, ss) {
@@ -987,9 +986,6 @@ static void cgroup_clear_directory(struct dentry *dir, bool base_files,
static void cgroup_d_remove_dir(struct dentry *dentry)
{
struct dentry *parent;
- struct cgroupfs_root *root = dentry->d_sb->s_fs_info;
-
- cgroup_clear_directory(dentry, true, root->subsys_mask);
parent = dentry->d_parent;
spin_lock(&parent->d_lock);
@@ -1376,7 +1372,7 @@ static int cgroup_remount(struct super_block *sb, int *flags, char *data)
* this before rebind_subsystems, since rebind_subsystems may
* change this hierarchy's subsys_list.
*/
- cgroup_clear_directory(cgrp->dentry, false, removed_mask);
+ cgroup_clear_dir(cgrp, false, removed_mask);
ret = rebind_subsystems(root, added_mask, removed_mask);
if (ret) {
@@ -4541,9 +4537,10 @@ static int cgroup_destroy_locked(struct cgroup *cgrp)
raw_spin_unlock(&release_list_lock);
/*
- * Remove @cgrp directory. The removal puts the base ref but we
- * aren't quite done with @cgrp yet, so hold onto it.
+ * Clear and remove @cgrp directory. The removal puts the base ref
+ * but we aren't quite done with @cgrp yet, so hold onto it.
*/
+ cgroup_clear_dir(cgrp, true, cgrp->root->subsys_mask);
dget(d);
cgroup_d_remove_dir(d);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH 1/9] cgroup: minor updates around cgroup_clear_directory()
[not found] ` <1372463145-4245-1-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2013-06-28 23:45 ` [PATCH 1/9] cgroup: minor updates around cgroup_clear_directory() Tejun Heo
@ 2013-06-28 23:45 ` Tejun Heo
2013-06-28 23:45 ` [PATCH 2/9] cgroup: fix error path of cgroup_addrm_files() Tejun Heo
` (19 subsequent siblings)
21 siblings, 0 replies; 45+ messages in thread
From: Tejun Heo @ 2013-06-28 23:45 UTC (permalink / raw)
To: lizefan-hv44wF8Li93QT0dZR+AlfA
Cc: Tejun Heo, cgroups-u79uwXL29TY76Z2rM5mHXA,
containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
* Rename it to cgroup_clear_dir() and make it take the pointer to the
target cgroup instead of the the dentry. This makes the function
consistent with its counterpart - cgroup_populate_dir().
* Move cgroup_clear_directory() invocation from cgroup_d_remove_dir()
to cgroup_remount() so that the function doesn't have to determine
the cgroup pointer back from the dentry. cgroup_d_remove_dir() now
only deals with vfs, which is slightly cleaner.
This patch doesn't introduce any functional differences.
Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
kernel/cgroup.c | 19 ++++++++-----------
1 file changed, 8 insertions(+), 11 deletions(-)
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 5a2fcf5..64877de 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -957,15 +957,14 @@ static void cgroup_rm_file(struct cgroup *cgrp, const struct cftype *cft)
}
/**
- * cgroup_clear_directory - selective removal of base and subsystem files
- * @dir: directory containing the files
+ * cgroup_clear_dir - selective removal of base and subsystem files
+ * @cgrp: target cgroup
* @base_files: true if the base files should be removed
* @subsys_mask: mask of the subsystem ids whose files should be removed
*/
-static void cgroup_clear_directory(struct dentry *dir, bool base_files,
- unsigned long subsys_mask)
+static void cgroup_clear_dir(struct cgroup *cgrp, bool base_files,
+ unsigned long subsys_mask)
{
- struct cgroup *cgrp = __d_cgrp(dir);
struct cgroup_subsys *ss;
for_each_root_subsys(cgrp->root, ss) {
@@ -987,9 +986,6 @@ static void cgroup_clear_directory(struct dentry *dir, bool base_files,
static void cgroup_d_remove_dir(struct dentry *dentry)
{
struct dentry *parent;
- struct cgroupfs_root *root = dentry->d_sb->s_fs_info;
-
- cgroup_clear_directory(dentry, true, root->subsys_mask);
parent = dentry->d_parent;
spin_lock(&parent->d_lock);
@@ -1376,7 +1372,7 @@ static int cgroup_remount(struct super_block *sb, int *flags, char *data)
* this before rebind_subsystems, since rebind_subsystems may
* change this hierarchy's subsys_list.
*/
- cgroup_clear_directory(cgrp->dentry, false, removed_mask);
+ cgroup_clear_dir(cgrp, false, removed_mask);
ret = rebind_subsystems(root, added_mask, removed_mask);
if (ret) {
@@ -4541,9 +4537,10 @@ static int cgroup_destroy_locked(struct cgroup *cgrp)
raw_spin_unlock(&release_list_lock);
/*
- * Remove @cgrp directory. The removal puts the base ref but we
- * aren't quite done with @cgrp yet, so hold onto it.
+ * Clear and remove @cgrp directory. The removal puts the base ref
+ * but we aren't quite done with @cgrp yet, so hold onto it.
*/
+ cgroup_clear_dir(cgrp, true, cgrp->root->subsys_mask);
dget(d);
cgroup_d_remove_dir(d);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH 2/9] cgroup: fix error path of cgroup_addrm_files()
[not found] ` <1372463145-4245-1-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2013-06-28 23:45 ` [PATCH 1/9] cgroup: minor updates around cgroup_clear_directory() Tejun Heo
2013-06-28 23:45 ` Tejun Heo
@ 2013-06-28 23:45 ` Tejun Heo
[not found] ` <1372463145-4245-3-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2013-06-28 23:45 ` Tejun Heo
` (18 subsequent siblings)
21 siblings, 1 reply; 45+ messages in thread
From: Tejun Heo @ 2013-06-28 23:45 UTC (permalink / raw)
To: lizefan-hv44wF8Li93QT0dZR+AlfA
Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
cgroups-u79uwXL29TY76Z2rM5mHXA, Tejun Heo
cgroup_addrm_files() mishandled error return value from
cgroup_add_file() and returns error iff the last file fails to create.
As we're in the process of cleaning up file add/rm error handling and
will reliably propagate file creation failures, there's no point in
keeping adding files after a failure.
Replace the broken error collection logic with immediate error return.
While at it, add lockdep assertions and function comment.
Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
kernel/cgroup.c | 28 ++++++++++++++++++++++------
1 file changed, 22 insertions(+), 6 deletions(-)
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 64877de..3df8e92 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -2780,11 +2780,26 @@ out:
return error;
}
+/**
+ * cgroup_addrm_files - add or remove files to a cgroup directory
+ * @cgrp: the target cgroup
+ * @subsys: the subsystem of files to be added
+ * @cfts: array of cftypes to be added
+ * @is_add: whether to add or remove
+ *
+ * Depending on @is_add, add or remove files defined by @cfts on @cgrp.
+ * All @cfts should belong to @subsys. For removals, this function never
+ * fails. If addition fails, this function doesn't remove files already
+ * added. The caller is responsible for cleaning up.
+ */
static int cgroup_addrm_files(struct cgroup *cgrp, struct cgroup_subsys *subsys,
struct cftype cfts[], bool is_add)
{
struct cftype *cft;
- int err, ret = 0;
+ int ret;
+
+ lockdep_assert_held(&cgrp->dentry->d_inode->i_mutex);
+ lockdep_assert_held(&cgroup_mutex);
for (cft = cfts; cft->name[0] != '\0'; cft++) {
/* does cft->flags tell us to skip this file on @cgrp? */
@@ -2796,16 +2811,17 @@ static int cgroup_addrm_files(struct cgroup *cgrp, struct cgroup_subsys *subsys,
continue;
if (is_add) {
- err = cgroup_add_file(cgrp, subsys, cft);
- if (err)
+ ret = cgroup_add_file(cgrp, subsys, cft);
+ if (ret) {
pr_warn("cgroup_addrm_files: failed to add %s, err=%d\n",
- cft->name, err);
- ret = err;
+ cft->name, ret);
+ return ret;
+ }
} else {
cgroup_rm_file(cgrp, cft);
}
}
- return ret;
+ return 0;
}
static void cgroup_cfts_prepare(void)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH 2/9] cgroup: fix error path of cgroup_addrm_files()
[not found] ` <1372463145-4245-1-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
` (2 preceding siblings ...)
2013-06-28 23:45 ` [PATCH 2/9] cgroup: fix error path of cgroup_addrm_files() Tejun Heo
@ 2013-06-28 23:45 ` Tejun Heo
2013-06-28 23:45 ` [PATCH 3/9] cgroup: fix cgroup_add_cftypes() error handling Tejun Heo
` (17 subsequent siblings)
21 siblings, 0 replies; 45+ messages in thread
From: Tejun Heo @ 2013-06-28 23:45 UTC (permalink / raw)
To: lizefan-hv44wF8Li93QT0dZR+AlfA
Cc: Tejun Heo, cgroups-u79uwXL29TY76Z2rM5mHXA,
containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
cgroup_addrm_files() mishandled error return value from
cgroup_add_file() and returns error iff the last file fails to create.
As we're in the process of cleaning up file add/rm error handling and
will reliably propagate file creation failures, there's no point in
keeping adding files after a failure.
Replace the broken error collection logic with immediate error return.
While at it, add lockdep assertions and function comment.
Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
kernel/cgroup.c | 28 ++++++++++++++++++++++------
1 file changed, 22 insertions(+), 6 deletions(-)
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 64877de..3df8e92 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -2780,11 +2780,26 @@ out:
return error;
}
+/**
+ * cgroup_addrm_files - add or remove files to a cgroup directory
+ * @cgrp: the target cgroup
+ * @subsys: the subsystem of files to be added
+ * @cfts: array of cftypes to be added
+ * @is_add: whether to add or remove
+ *
+ * Depending on @is_add, add or remove files defined by @cfts on @cgrp.
+ * All @cfts should belong to @subsys. For removals, this function never
+ * fails. If addition fails, this function doesn't remove files already
+ * added. The caller is responsible for cleaning up.
+ */
static int cgroup_addrm_files(struct cgroup *cgrp, struct cgroup_subsys *subsys,
struct cftype cfts[], bool is_add)
{
struct cftype *cft;
- int err, ret = 0;
+ int ret;
+
+ lockdep_assert_held(&cgrp->dentry->d_inode->i_mutex);
+ lockdep_assert_held(&cgroup_mutex);
for (cft = cfts; cft->name[0] != '\0'; cft++) {
/* does cft->flags tell us to skip this file on @cgrp? */
@@ -2796,16 +2811,17 @@ static int cgroup_addrm_files(struct cgroup *cgrp, struct cgroup_subsys *subsys,
continue;
if (is_add) {
- err = cgroup_add_file(cgrp, subsys, cft);
- if (err)
+ ret = cgroup_add_file(cgrp, subsys, cft);
+ if (ret) {
pr_warn("cgroup_addrm_files: failed to add %s, err=%d\n",
- cft->name, err);
- ret = err;
+ cft->name, ret);
+ return ret;
+ }
} else {
cgroup_rm_file(cgrp, cft);
}
}
- return ret;
+ return 0;
}
static void cgroup_cfts_prepare(void)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH 3/9] cgroup: fix cgroup_add_cftypes() error handling
[not found] ` <1372463145-4245-1-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
` (3 preceding siblings ...)
2013-06-28 23:45 ` Tejun Heo
@ 2013-06-28 23:45 ` Tejun Heo
2013-06-28 23:45 ` Tejun Heo
` (16 subsequent siblings)
21 siblings, 0 replies; 45+ messages in thread
From: Tejun Heo @ 2013-06-28 23:45 UTC (permalink / raw)
To: lizefan-hv44wF8Li93QT0dZR+AlfA
Cc: Tejun Heo, cgroups-u79uwXL29TY76Z2rM5mHXA,
containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
cgroup_add_cftypes() uses cgroup_cfts_commit() to actually create the
files; however, both functions ignore actual file creation errors and
just assume success. This can lead to, for example, blkio hierarchy
with some of the cgroups with only subset of interface files populated
after cfq-iosched is loaded under heavy memory pressure, which is
nasty.
This patch updates cgroup_cfts_commit() and cgroup_add_cftypes() to
guarantee that all files are created on success and no file is created
on failure.
Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
kernel/cgroup.c | 26 ++++++++++++++++++--------
1 file changed, 18 insertions(+), 8 deletions(-)
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 3df8e92..4682d81 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -2836,8 +2836,8 @@ static void cgroup_cfts_prepare(void)
mutex_lock(&cgroup_mutex);
}
-static void cgroup_cfts_commit(struct cgroup_subsys *ss,
- struct cftype *cfts, bool is_add)
+static int cgroup_cfts_commit(struct cgroup_subsys *ss,
+ struct cftype *cfts, bool is_add)
__releases(&cgroup_mutex)
{
LIST_HEAD(pending);
@@ -2846,12 +2846,13 @@ static void cgroup_cfts_commit(struct cgroup_subsys *ss,
struct dentry *prev = NULL;
struct inode *inode;
u64 update_before;
+ int ret = 0;
/* %NULL @cfts indicates abort and don't bother if @ss isn't attached */
if (!cfts || ss->root == &cgroup_dummy_root ||
!atomic_inc_not_zero(&sb->s_active)) {
mutex_unlock(&cgroup_mutex);
- return;
+ return 0;
}
/*
@@ -2867,10 +2868,13 @@ static void cgroup_cfts_commit(struct cgroup_subsys *ss,
inode = root->dentry->d_inode;
mutex_lock(&inode->i_mutex);
mutex_lock(&cgroup_mutex);
- cgroup_addrm_files(root, ss, cfts, is_add);
+ ret = cgroup_addrm_files(root, ss, cfts, is_add);
mutex_unlock(&cgroup_mutex);
mutex_unlock(&inode->i_mutex);
+ if (ret)
+ goto out_deact;
+
/* add/rm files for all cgroups created before */
rcu_read_lock();
cgroup_for_each_descendant_pre(cgrp, root) {
@@ -2887,15 +2891,19 @@ static void cgroup_cfts_commit(struct cgroup_subsys *ss,
mutex_lock(&inode->i_mutex);
mutex_lock(&cgroup_mutex);
if (cgrp->serial_nr < update_before && !cgroup_is_dead(cgrp))
- cgroup_addrm_files(cgrp, ss, cfts, is_add);
+ ret = cgroup_addrm_files(cgrp, ss, cfts, is_add);
mutex_unlock(&cgroup_mutex);
mutex_unlock(&inode->i_mutex);
rcu_read_lock();
+ if (ret)
+ break;
}
rcu_read_unlock();
dput(prev);
+out_deact:
deactivate_super(sb);
+ return ret;
}
/**
@@ -2915,6 +2923,7 @@ static void cgroup_cfts_commit(struct cgroup_subsys *ss,
int cgroup_add_cftypes(struct cgroup_subsys *ss, struct cftype *cfts)
{
struct cftype_set *set;
+ int ret;
set = kzalloc(sizeof(*set), GFP_KERNEL);
if (!set)
@@ -2923,9 +2932,10 @@ int cgroup_add_cftypes(struct cgroup_subsys *ss, struct cftype *cfts)
cgroup_cfts_prepare();
set->cfts = cfts;
list_add_tail(&set->node, &ss->cftsets);
- cgroup_cfts_commit(ss, cfts, true);
-
- return 0;
+ ret = cgroup_cfts_commit(ss, cfts, true);
+ if (ret)
+ cgroup_rm_cftypes(ss, cfts);
+ return ret;
}
EXPORT_SYMBOL_GPL(cgroup_add_cftypes);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH 3/9] cgroup: fix cgroup_add_cftypes() error handling
[not found] ` <1372463145-4245-1-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
` (4 preceding siblings ...)
2013-06-28 23:45 ` [PATCH 3/9] cgroup: fix cgroup_add_cftypes() error handling Tejun Heo
@ 2013-06-28 23:45 ` Tejun Heo
[not found] ` <1372463145-4245-4-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2013-06-28 23:45 ` [PATCH 4/9] cgroup: separate out cgroup_base_files[] handling out of cgroup_populate/clear_dir() Tejun Heo
` (15 subsequent siblings)
21 siblings, 1 reply; 45+ messages in thread
From: Tejun Heo @ 2013-06-28 23:45 UTC (permalink / raw)
To: lizefan-hv44wF8Li93QT0dZR+AlfA
Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
cgroups-u79uwXL29TY76Z2rM5mHXA, Tejun Heo
cgroup_add_cftypes() uses cgroup_cfts_commit() to actually create the
files; however, both functions ignore actual file creation errors and
just assume success. This can lead to, for example, blkio hierarchy
with some of the cgroups with only subset of interface files populated
after cfq-iosched is loaded under heavy memory pressure, which is
nasty.
This patch updates cgroup_cfts_commit() and cgroup_add_cftypes() to
guarantee that all files are created on success and no file is created
on failure.
Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
kernel/cgroup.c | 26 ++++++++++++++++++--------
1 file changed, 18 insertions(+), 8 deletions(-)
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 3df8e92..4682d81 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -2836,8 +2836,8 @@ static void cgroup_cfts_prepare(void)
mutex_lock(&cgroup_mutex);
}
-static void cgroup_cfts_commit(struct cgroup_subsys *ss,
- struct cftype *cfts, bool is_add)
+static int cgroup_cfts_commit(struct cgroup_subsys *ss,
+ struct cftype *cfts, bool is_add)
__releases(&cgroup_mutex)
{
LIST_HEAD(pending);
@@ -2846,12 +2846,13 @@ static void cgroup_cfts_commit(struct cgroup_subsys *ss,
struct dentry *prev = NULL;
struct inode *inode;
u64 update_before;
+ int ret = 0;
/* %NULL @cfts indicates abort and don't bother if @ss isn't attached */
if (!cfts || ss->root == &cgroup_dummy_root ||
!atomic_inc_not_zero(&sb->s_active)) {
mutex_unlock(&cgroup_mutex);
- return;
+ return 0;
}
/*
@@ -2867,10 +2868,13 @@ static void cgroup_cfts_commit(struct cgroup_subsys *ss,
inode = root->dentry->d_inode;
mutex_lock(&inode->i_mutex);
mutex_lock(&cgroup_mutex);
- cgroup_addrm_files(root, ss, cfts, is_add);
+ ret = cgroup_addrm_files(root, ss, cfts, is_add);
mutex_unlock(&cgroup_mutex);
mutex_unlock(&inode->i_mutex);
+ if (ret)
+ goto out_deact;
+
/* add/rm files for all cgroups created before */
rcu_read_lock();
cgroup_for_each_descendant_pre(cgrp, root) {
@@ -2887,15 +2891,19 @@ static void cgroup_cfts_commit(struct cgroup_subsys *ss,
mutex_lock(&inode->i_mutex);
mutex_lock(&cgroup_mutex);
if (cgrp->serial_nr < update_before && !cgroup_is_dead(cgrp))
- cgroup_addrm_files(cgrp, ss, cfts, is_add);
+ ret = cgroup_addrm_files(cgrp, ss, cfts, is_add);
mutex_unlock(&cgroup_mutex);
mutex_unlock(&inode->i_mutex);
rcu_read_lock();
+ if (ret)
+ break;
}
rcu_read_unlock();
dput(prev);
+out_deact:
deactivate_super(sb);
+ return ret;
}
/**
@@ -2915,6 +2923,7 @@ static void cgroup_cfts_commit(struct cgroup_subsys *ss,
int cgroup_add_cftypes(struct cgroup_subsys *ss, struct cftype *cfts)
{
struct cftype_set *set;
+ int ret;
set = kzalloc(sizeof(*set), GFP_KERNEL);
if (!set)
@@ -2923,9 +2932,10 @@ int cgroup_add_cftypes(struct cgroup_subsys *ss, struct cftype *cfts)
cgroup_cfts_prepare();
set->cfts = cfts;
list_add_tail(&set->node, &ss->cftsets);
- cgroup_cfts_commit(ss, cfts, true);
-
- return 0;
+ ret = cgroup_cfts_commit(ss, cfts, true);
+ if (ret)
+ cgroup_rm_cftypes(ss, cfts);
+ return ret;
}
EXPORT_SYMBOL_GPL(cgroup_add_cftypes);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH 4/9] cgroup: separate out cgroup_base_files[] handling out of cgroup_populate/clear_dir()
[not found] ` <1372463145-4245-1-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
` (5 preceding siblings ...)
2013-06-28 23:45 ` Tejun Heo
@ 2013-06-28 23:45 ` Tejun Heo
[not found] ` <1372463145-4245-5-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2013-06-28 23:45 ` Tejun Heo
` (14 subsequent siblings)
21 siblings, 1 reply; 45+ messages in thread
From: Tejun Heo @ 2013-06-28 23:45 UTC (permalink / raw)
To: lizefan-hv44wF8Li93QT0dZR+AlfA
Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
cgroups-u79uwXL29TY76Z2rM5mHXA, Tejun Heo
cgroup_populate/clear_dir() currently take @base_files and adds and
removes, respectively, cgroup_base_files[] to the directory. File
additions and removals are being reorganized for proper error handling
and more dynamic handling for the unified hierarchy, and mixing base
and subsys file handling into the same functions gets a bit confusing.
This patch moves base file handling out of cgroup_populate/clear_dir()
into their users - cgroup_mount(), cgroup_create() and
cgroup_destroy_locked().
Note that this changes the behavior of base file removal. If
@base_files is %true, cgroup_clear_dir() used to delete files
regardless of cftype until there's no files left. Now, only files
with matching cfts are removed. As files can only be created by the
base or registered cftypes, this shouldn't result in any behavior
difference.
Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
kernel/cgroup.c | 46 +++++++++++++++++++---------------------------
1 file changed, 19 insertions(+), 27 deletions(-)
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 4682d81..254d54e 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -215,6 +215,8 @@ static u64 cgroup_serial_nr_next = 1;
*/
static int need_forkexit_callback __read_mostly;
+static struct cftype cgroup_base_files[];
+
static void cgroup_offline_fn(struct work_struct *work);
static int cgroup_destroy_locked(struct cgroup *cgrp);
static int cgroup_addrm_files(struct cgroup *cgrp, struct cgroup_subsys *subsys,
@@ -804,8 +806,7 @@ static struct cgroup *task_cgroup_from_root(struct task_struct *task,
static int cgroup_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode);
static struct dentry *cgroup_lookup(struct inode *, struct dentry *, unsigned int);
static int cgroup_rmdir(struct inode *unused_dir, struct dentry *dentry);
-static int cgroup_populate_dir(struct cgroup *cgrp, bool base_files,
- unsigned long subsys_mask);
+static int cgroup_populate_dir(struct cgroup *cgrp, unsigned long subsys_mask);
static const struct inode_operations cgroup_dir_inode_operations;
static const struct file_operations proc_cgroupstats_operations;
@@ -957,13 +958,11 @@ static void cgroup_rm_file(struct cgroup *cgrp, const struct cftype *cft)
}
/**
- * cgroup_clear_dir - selective removal of base and subsystem files
+ * cgroup_clear_dir - remove subsys files in a cgroup directory
* @cgrp: target cgroup
- * @base_files: true if the base files should be removed
* @subsys_mask: mask of the subsystem ids whose files should be removed
*/
-static void cgroup_clear_dir(struct cgroup *cgrp, bool base_files,
- unsigned long subsys_mask)
+static void cgroup_clear_dir(struct cgroup *cgrp, unsigned long subsys_mask)
{
struct cgroup_subsys *ss;
@@ -974,10 +973,6 @@ static void cgroup_clear_dir(struct cgroup *cgrp, bool base_files,
list_for_each_entry(set, &ss->cftsets, node)
cgroup_addrm_files(cgrp, NULL, set->cfts, false);
}
- if (base_files) {
- while (!list_empty(&cgrp->files))
- cgroup_rm_file(cgrp, NULL);
- }
}
/*
@@ -1372,17 +1367,17 @@ static int cgroup_remount(struct super_block *sb, int *flags, char *data)
* this before rebind_subsystems, since rebind_subsystems may
* change this hierarchy's subsys_list.
*/
- cgroup_clear_dir(cgrp, false, removed_mask);
+ cgroup_clear_dir(cgrp, removed_mask);
ret = rebind_subsystems(root, added_mask, removed_mask);
if (ret) {
/* rebind_subsystems failed, re-populate the removed files */
- cgroup_populate_dir(cgrp, false, removed_mask);
+ cgroup_populate_dir(cgrp, removed_mask);
goto out_unlock;
}
/* re-populate subsystem files */
- cgroup_populate_dir(cgrp, false, added_mask);
+ cgroup_populate_dir(cgrp, added_mask);
if (opts.release_agent)
strcpy(root->release_agent_path, opts.release_agent);
@@ -1687,7 +1682,8 @@ static struct dentry *cgroup_mount(struct file_system_type *fs_type,
BUG_ON(root->number_of_cgroups != 1);
cred = override_creds(&init_cred);
- cgroup_populate_dir(root_cgrp, true, root->subsys_mask);
+ cgroup_addrm_files(root_cgrp, NULL, cgroup_base_files, true);
+ cgroup_populate_dir(root_cgrp, root->subsys_mask);
revert_creds(cred);
mutex_unlock(&cgroup_root_mutex);
mutex_unlock(&cgroup_mutex);
@@ -4172,23 +4168,14 @@ static struct cftype cgroup_base_files[] = {
};
/**
- * cgroup_populate_dir - selectively creation of files in a directory
+ * cgroup_populate_dir - create subsys files in a cgroup directory
* @cgrp: target cgroup
- * @base_files: true if the base files should be added
* @subsys_mask: mask of the subsystem ids whose files should be added
*/
-static int cgroup_populate_dir(struct cgroup *cgrp, bool base_files,
- unsigned long subsys_mask)
+static int cgroup_populate_dir(struct cgroup *cgrp, unsigned long subsys_mask)
{
- int err;
struct cgroup_subsys *ss;
- if (base_files) {
- err = cgroup_addrm_files(cgrp, NULL, cgroup_base_files, true);
- if (err < 0)
- return err;
- }
-
/* process cftsets of each subsystem */
for_each_root_subsys(cgrp->root, ss) {
struct cftype_set *set;
@@ -4410,7 +4397,11 @@ static long cgroup_create(struct cgroup *parent, struct dentry *dentry,
}
}
- err = cgroup_populate_dir(cgrp, true, root->subsys_mask);
+ err = cgroup_addrm_files(cgrp, NULL, cgroup_base_files, true);
+ if (err)
+ goto err_destroy;
+
+ err = cgroup_populate_dir(cgrp, root->subsys_mask);
if (err)
goto err_destroy;
@@ -4566,7 +4557,8 @@ static int cgroup_destroy_locked(struct cgroup *cgrp)
* Clear and remove @cgrp directory. The removal puts the base ref
* but we aren't quite done with @cgrp yet, so hold onto it.
*/
- cgroup_clear_dir(cgrp, true, cgrp->root->subsys_mask);
+ cgroup_clear_dir(cgrp, cgrp->root->subsys_mask);
+ cgroup_addrm_files(cgrp, NULL, cgroup_base_files, false);
dget(d);
cgroup_d_remove_dir(d);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH 4/9] cgroup: separate out cgroup_base_files[] handling out of cgroup_populate/clear_dir()
[not found] ` <1372463145-4245-1-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
` (6 preceding siblings ...)
2013-06-28 23:45 ` [PATCH 4/9] cgroup: separate out cgroup_base_files[] handling out of cgroup_populate/clear_dir() Tejun Heo
@ 2013-06-28 23:45 ` Tejun Heo
2013-06-28 23:45 ` [PATCH 5/9] cgroup: update error handling in cgroup_populate_dir() Tejun Heo
` (13 subsequent siblings)
21 siblings, 0 replies; 45+ messages in thread
From: Tejun Heo @ 2013-06-28 23:45 UTC (permalink / raw)
To: lizefan-hv44wF8Li93QT0dZR+AlfA
Cc: Tejun Heo, cgroups-u79uwXL29TY76Z2rM5mHXA,
containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
cgroup_populate/clear_dir() currently take @base_files and adds and
removes, respectively, cgroup_base_files[] to the directory. File
additions and removals are being reorganized for proper error handling
and more dynamic handling for the unified hierarchy, and mixing base
and subsys file handling into the same functions gets a bit confusing.
This patch moves base file handling out of cgroup_populate/clear_dir()
into their users - cgroup_mount(), cgroup_create() and
cgroup_destroy_locked().
Note that this changes the behavior of base file removal. If
@base_files is %true, cgroup_clear_dir() used to delete files
regardless of cftype until there's no files left. Now, only files
with matching cfts are removed. As files can only be created by the
base or registered cftypes, this shouldn't result in any behavior
difference.
Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
kernel/cgroup.c | 46 +++++++++++++++++++---------------------------
1 file changed, 19 insertions(+), 27 deletions(-)
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 4682d81..254d54e 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -215,6 +215,8 @@ static u64 cgroup_serial_nr_next = 1;
*/
static int need_forkexit_callback __read_mostly;
+static struct cftype cgroup_base_files[];
+
static void cgroup_offline_fn(struct work_struct *work);
static int cgroup_destroy_locked(struct cgroup *cgrp);
static int cgroup_addrm_files(struct cgroup *cgrp, struct cgroup_subsys *subsys,
@@ -804,8 +806,7 @@ static struct cgroup *task_cgroup_from_root(struct task_struct *task,
static int cgroup_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode);
static struct dentry *cgroup_lookup(struct inode *, struct dentry *, unsigned int);
static int cgroup_rmdir(struct inode *unused_dir, struct dentry *dentry);
-static int cgroup_populate_dir(struct cgroup *cgrp, bool base_files,
- unsigned long subsys_mask);
+static int cgroup_populate_dir(struct cgroup *cgrp, unsigned long subsys_mask);
static const struct inode_operations cgroup_dir_inode_operations;
static const struct file_operations proc_cgroupstats_operations;
@@ -957,13 +958,11 @@ static void cgroup_rm_file(struct cgroup *cgrp, const struct cftype *cft)
}
/**
- * cgroup_clear_dir - selective removal of base and subsystem files
+ * cgroup_clear_dir - remove subsys files in a cgroup directory
* @cgrp: target cgroup
- * @base_files: true if the base files should be removed
* @subsys_mask: mask of the subsystem ids whose files should be removed
*/
-static void cgroup_clear_dir(struct cgroup *cgrp, bool base_files,
- unsigned long subsys_mask)
+static void cgroup_clear_dir(struct cgroup *cgrp, unsigned long subsys_mask)
{
struct cgroup_subsys *ss;
@@ -974,10 +973,6 @@ static void cgroup_clear_dir(struct cgroup *cgrp, bool base_files,
list_for_each_entry(set, &ss->cftsets, node)
cgroup_addrm_files(cgrp, NULL, set->cfts, false);
}
- if (base_files) {
- while (!list_empty(&cgrp->files))
- cgroup_rm_file(cgrp, NULL);
- }
}
/*
@@ -1372,17 +1367,17 @@ static int cgroup_remount(struct super_block *sb, int *flags, char *data)
* this before rebind_subsystems, since rebind_subsystems may
* change this hierarchy's subsys_list.
*/
- cgroup_clear_dir(cgrp, false, removed_mask);
+ cgroup_clear_dir(cgrp, removed_mask);
ret = rebind_subsystems(root, added_mask, removed_mask);
if (ret) {
/* rebind_subsystems failed, re-populate the removed files */
- cgroup_populate_dir(cgrp, false, removed_mask);
+ cgroup_populate_dir(cgrp, removed_mask);
goto out_unlock;
}
/* re-populate subsystem files */
- cgroup_populate_dir(cgrp, false, added_mask);
+ cgroup_populate_dir(cgrp, added_mask);
if (opts.release_agent)
strcpy(root->release_agent_path, opts.release_agent);
@@ -1687,7 +1682,8 @@ static struct dentry *cgroup_mount(struct file_system_type *fs_type,
BUG_ON(root->number_of_cgroups != 1);
cred = override_creds(&init_cred);
- cgroup_populate_dir(root_cgrp, true, root->subsys_mask);
+ cgroup_addrm_files(root_cgrp, NULL, cgroup_base_files, true);
+ cgroup_populate_dir(root_cgrp, root->subsys_mask);
revert_creds(cred);
mutex_unlock(&cgroup_root_mutex);
mutex_unlock(&cgroup_mutex);
@@ -4172,23 +4168,14 @@ static struct cftype cgroup_base_files[] = {
};
/**
- * cgroup_populate_dir - selectively creation of files in a directory
+ * cgroup_populate_dir - create subsys files in a cgroup directory
* @cgrp: target cgroup
- * @base_files: true if the base files should be added
* @subsys_mask: mask of the subsystem ids whose files should be added
*/
-static int cgroup_populate_dir(struct cgroup *cgrp, bool base_files,
- unsigned long subsys_mask)
+static int cgroup_populate_dir(struct cgroup *cgrp, unsigned long subsys_mask)
{
- int err;
struct cgroup_subsys *ss;
- if (base_files) {
- err = cgroup_addrm_files(cgrp, NULL, cgroup_base_files, true);
- if (err < 0)
- return err;
- }
-
/* process cftsets of each subsystem */
for_each_root_subsys(cgrp->root, ss) {
struct cftype_set *set;
@@ -4410,7 +4397,11 @@ static long cgroup_create(struct cgroup *parent, struct dentry *dentry,
}
}
- err = cgroup_populate_dir(cgrp, true, root->subsys_mask);
+ err = cgroup_addrm_files(cgrp, NULL, cgroup_base_files, true);
+ if (err)
+ goto err_destroy;
+
+ err = cgroup_populate_dir(cgrp, root->subsys_mask);
if (err)
goto err_destroy;
@@ -4566,7 +4557,8 @@ static int cgroup_destroy_locked(struct cgroup *cgrp)
* Clear and remove @cgrp directory. The removal puts the base ref
* but we aren't quite done with @cgrp yet, so hold onto it.
*/
- cgroup_clear_dir(cgrp, true, cgrp->root->subsys_mask);
+ cgroup_clear_dir(cgrp, cgrp->root->subsys_mask);
+ cgroup_addrm_files(cgrp, NULL, cgroup_base_files, false);
dget(d);
cgroup_d_remove_dir(d);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH 5/9] cgroup: update error handling in cgroup_populate_dir()
[not found] ` <1372463145-4245-1-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
` (7 preceding siblings ...)
2013-06-28 23:45 ` Tejun Heo
@ 2013-06-28 23:45 ` Tejun Heo
[not found] ` <1372463145-4245-6-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2013-06-28 23:45 ` [PATCH " Tejun Heo
` (12 subsequent siblings)
21 siblings, 1 reply; 45+ messages in thread
From: Tejun Heo @ 2013-06-28 23:45 UTC (permalink / raw)
To: lizefan-hv44wF8Li93QT0dZR+AlfA
Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
cgroups-u79uwXL29TY76Z2rM5mHXA, Tejun Heo
cgroup_populate_dir() didn't use to check whether the actual file
creations were successful and could return success with only subset of
the requested files created, which is nasty.
This patch udpates cgroup_populate_dir() so that it either succeeds
with all files or fails with no file.
Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
kernel/cgroup.c | 24 ++++++++++++++++++------
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 254d54e..3475267 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -965,10 +965,12 @@ static void cgroup_rm_file(struct cgroup *cgrp, const struct cftype *cft)
static void cgroup_clear_dir(struct cgroup *cgrp, unsigned long subsys_mask)
{
struct cgroup_subsys *ss;
+ int i;
- for_each_root_subsys(cgrp->root, ss) {
+ for_each_subsys(ss, i) {
struct cftype_set *set;
- if (!test_bit(ss->subsys_id, &subsys_mask))
+
+ if (!test_bit(i, &subsys_mask))
continue;
list_for_each_entry(set, &ss->cftsets, node)
cgroup_addrm_files(cgrp, NULL, set->cfts, false);
@@ -4171,19 +4173,26 @@ static struct cftype cgroup_base_files[] = {
* cgroup_populate_dir - create subsys files in a cgroup directory
* @cgrp: target cgroup
* @subsys_mask: mask of the subsystem ids whose files should be added
+ *
+ * On failure, no file is added.
*/
static int cgroup_populate_dir(struct cgroup *cgrp, unsigned long subsys_mask)
{
struct cgroup_subsys *ss;
+ int i, ret = 0;
/* process cftsets of each subsystem */
- for_each_root_subsys(cgrp->root, ss) {
+ for_each_subsys(ss, i) {
struct cftype_set *set;
- if (!test_bit(ss->subsys_id, &subsys_mask))
+
+ if (!test_bit(i, &subsys_mask))
continue;
- list_for_each_entry(set, &ss->cftsets, node)
- cgroup_addrm_files(cgrp, ss, set->cfts, true);
+ list_for_each_entry(set, &ss->cftsets, node) {
+ ret = cgroup_addrm_files(cgrp, ss, set->cfts, true);
+ if (ret < 0)
+ goto err;
+ }
}
/* This cgroup is ready now */
@@ -4201,6 +4210,9 @@ static int cgroup_populate_dir(struct cgroup *cgrp, unsigned long subsys_mask)
}
return 0;
+err:
+ cgroup_clear_dir(cgrp, subsys_mask);
+ return ret;
}
static void css_dput_fn(struct work_struct *work)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH 5/9] cgroup: update error handling in cgroup_populate_dir()
[not found] ` <1372463145-4245-1-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
` (8 preceding siblings ...)
2013-06-28 23:45 ` [PATCH 5/9] cgroup: update error handling in cgroup_populate_dir() Tejun Heo
@ 2013-06-28 23:45 ` Tejun Heo
2013-06-28 23:45 ` [PATCH 6/9] cgroup: make rebind_subsystems() handle file additions and removals with proper error handling Tejun Heo
` (11 subsequent siblings)
21 siblings, 0 replies; 45+ messages in thread
From: Tejun Heo @ 2013-06-28 23:45 UTC (permalink / raw)
To: lizefan-hv44wF8Li93QT0dZR+AlfA
Cc: Tejun Heo, cgroups-u79uwXL29TY76Z2rM5mHXA,
containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
cgroup_populate_dir() didn't use to check whether the actual file
creations were successful and could return success with only subset of
the requested files created, which is nasty.
This patch udpates cgroup_populate_dir() so that it either succeeds
with all files or fails with no file.
Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
kernel/cgroup.c | 24 ++++++++++++++++++------
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 254d54e..3475267 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -965,10 +965,12 @@ static void cgroup_rm_file(struct cgroup *cgrp, const struct cftype *cft)
static void cgroup_clear_dir(struct cgroup *cgrp, unsigned long subsys_mask)
{
struct cgroup_subsys *ss;
+ int i;
- for_each_root_subsys(cgrp->root, ss) {
+ for_each_subsys(ss, i) {
struct cftype_set *set;
- if (!test_bit(ss->subsys_id, &subsys_mask))
+
+ if (!test_bit(i, &subsys_mask))
continue;
list_for_each_entry(set, &ss->cftsets, node)
cgroup_addrm_files(cgrp, NULL, set->cfts, false);
@@ -4171,19 +4173,26 @@ static struct cftype cgroup_base_files[] = {
* cgroup_populate_dir - create subsys files in a cgroup directory
* @cgrp: target cgroup
* @subsys_mask: mask of the subsystem ids whose files should be added
+ *
+ * On failure, no file is added.
*/
static int cgroup_populate_dir(struct cgroup *cgrp, unsigned long subsys_mask)
{
struct cgroup_subsys *ss;
+ int i, ret = 0;
/* process cftsets of each subsystem */
- for_each_root_subsys(cgrp->root, ss) {
+ for_each_subsys(ss, i) {
struct cftype_set *set;
- if (!test_bit(ss->subsys_id, &subsys_mask))
+
+ if (!test_bit(i, &subsys_mask))
continue;
- list_for_each_entry(set, &ss->cftsets, node)
- cgroup_addrm_files(cgrp, ss, set->cfts, true);
+ list_for_each_entry(set, &ss->cftsets, node) {
+ ret = cgroup_addrm_files(cgrp, ss, set->cfts, true);
+ if (ret < 0)
+ goto err;
+ }
}
/* This cgroup is ready now */
@@ -4201,6 +4210,9 @@ static int cgroup_populate_dir(struct cgroup *cgrp, unsigned long subsys_mask)
}
return 0;
+err:
+ cgroup_clear_dir(cgrp, subsys_mask);
+ return ret;
}
static void css_dput_fn(struct work_struct *work)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH 6/9] cgroup: make rebind_subsystems() handle file additions and removals with proper error handling
[not found] ` <1372463145-4245-1-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
` (9 preceding siblings ...)
2013-06-28 23:45 ` [PATCH " Tejun Heo
@ 2013-06-28 23:45 ` Tejun Heo
2013-06-28 23:45 ` Tejun Heo
` (10 subsequent siblings)
21 siblings, 0 replies; 45+ messages in thread
From: Tejun Heo @ 2013-06-28 23:45 UTC (permalink / raw)
To: lizefan-hv44wF8Li93QT0dZR+AlfA
Cc: Tejun Heo, cgroups-u79uwXL29TY76Z2rM5mHXA,
containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
Currently, creating and removing cgroup files in the root directory
are handled separately from the actual subsystem binding and unbinding
which happens in rebind_subsystems(). Also, rebind_subsystems() users
aren't handling file creation errors properly. Let's integrate
top_cgroup file handling into rebind_subsystems() so that it's simpler
to use and everyone handles file creation errors correctly.
* On a successful return, rebind_subsystems() is guaranteed to have
created all files of the new subsystems and deleted the ones
belonging to the removed subsystems. After a failure, no file is
created or removed.
* cgroup_remount() no longer needs to make explicit populate/clear
calls as it's all handled by rebind_subsystems(), and it gets proper
error handling automatically.
* cgroup_mount() has been updated such that the root dentry and cgroup
are linked before rebind_subsystems(). Also, the init_cred dancing
and base file handling are moved right above rebind_subsystems()
call and proper error handling for the base files is added.
* cgroup_kill_sb() calls rebind_subsystems() to unbind all subsystems
which now implies removing all subsystem files which requires the
directory's i_mutex. Grab it. This means that files on the root
cgroup are removed earlier - they used to be deleted from generic
super_block cleanup from vfs. This doesn't lead to any functional
difference and it's cleaner to do the clean up explicitly for all
files.
Combined with the previous changes, this makes all cgroup file
creation errors handled correctly.
Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
kernel/cgroup.c | 61 ++++++++++++++++++++++++++++++---------------------------
1 file changed, 32 insertions(+), 29 deletions(-)
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 3475267..f0f8bb9 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -1003,7 +1003,7 @@ static int rebind_subsystems(struct cgroupfs_root *root,
{
struct cgroup *cgrp = &root->top_cgroup;
struct cgroup_subsys *ss;
- int i;
+ int i, ret;
BUG_ON(!mutex_is_locked(&cgroup_mutex));
BUG_ON(!mutex_is_locked(&cgroup_root_mutex));
@@ -1028,7 +1028,16 @@ static int rebind_subsystems(struct cgroupfs_root *root,
if (root->number_of_cgroups > 1)
return -EBUSY;
- /* Process each subsystem */
+ ret = cgroup_populate_dir(cgrp, added_mask);
+ if (ret)
+ return ret;
+
+ /*
+ * Nothing can fail from this point on. Remove files for the
+ * removed subsystems and rebind each subsystem.
+ */
+ cgroup_clear_dir(cgrp, removed_mask);
+
for_each_subsys(ss, i) {
unsigned long bit = 1UL << i;
@@ -1364,22 +1373,9 @@ static int cgroup_remount(struct super_block *sb, int *flags, char *data)
goto out_unlock;
}
- /*
- * Clear out the files of subsystems that should be removed, do
- * this before rebind_subsystems, since rebind_subsystems may
- * change this hierarchy's subsys_list.
- */
- cgroup_clear_dir(cgrp, removed_mask);
-
ret = rebind_subsystems(root, added_mask, removed_mask);
- if (ret) {
- /* rebind_subsystems failed, re-populate the removed files */
- cgroup_populate_dir(cgrp, removed_mask);
+ if (ret)
goto out_unlock;
- }
-
- /* re-populate subsystem files */
- cgroup_populate_dir(cgrp, added_mask);
if (opts.release_agent)
strcpy(root->release_agent_path, opts.release_agent);
@@ -1579,6 +1575,7 @@ static struct dentry *cgroup_mount(struct file_system_type *fs_type,
struct super_block *sb;
struct cgroupfs_root *new_root;
struct inode *inode;
+ const struct cred *cred;
/* First find the desired set of subsystems */
mutex_lock(&cgroup_mutex);
@@ -1613,7 +1610,6 @@ static struct dentry *cgroup_mount(struct file_system_type *fs_type,
struct list_head tmp_links;
struct cgroup *root_cgrp = &root->top_cgroup;
struct cgroupfs_root *existing_root;
- const struct cred *cred;
int i;
struct css_set *cset;
@@ -1651,26 +1647,32 @@ static struct dentry *cgroup_mount(struct file_system_type *fs_type,
if (ret)
goto unlock_drop;
+ sb->s_root->d_fsdata = root_cgrp;
+ root_cgrp->dentry = sb->s_root;
+
+ cred = override_creds(&init_cred);
+
+ ret = cgroup_addrm_files(root_cgrp, NULL, cgroup_base_files, true);
+ if (ret)
+ goto rm_base_files;
+
ret = rebind_subsystems(root, root->subsys_mask, 0);
- if (ret == -EBUSY) {
+ if (ret) {
free_cgrp_cset_links(&tmp_links);
- goto unlock_drop;
+ goto rm_base_files;
}
+
+ revert_creds(cred);
+
/*
* There must be no failure case after here, since rebinding
* takes care of subsystems' refcounts, which are explicitly
* dropped in the failure exit path.
*/
- /* EBUSY should be the only error here */
- BUG_ON(ret);
-
list_add(&root->root_list, &cgroup_roots);
cgroup_root_count++;
- sb->s_root->d_fsdata = root_cgrp;
- root->top_cgroup.dentry = sb->s_root;
-
/* Link the top cgroup in this hierarchy into all
* the css_set objects */
write_lock(&css_set_lock);
@@ -1683,10 +1685,6 @@ static struct dentry *cgroup_mount(struct file_system_type *fs_type,
BUG_ON(!list_empty(&root_cgrp->children));
BUG_ON(root->number_of_cgroups != 1);
- cred = override_creds(&init_cred);
- cgroup_addrm_files(root_cgrp, NULL, cgroup_base_files, true);
- cgroup_populate_dir(root_cgrp, root->subsys_mask);
- revert_creds(cred);
mutex_unlock(&cgroup_root_mutex);
mutex_unlock(&cgroup_mutex);
mutex_unlock(&inode->i_mutex);
@@ -1715,6 +1713,9 @@ static struct dentry *cgroup_mount(struct file_system_type *fs_type,
kfree(opts.name);
return dget(sb->s_root);
+ rm_base_files:
+ cgroup_addrm_files(&root->top_cgroup, NULL, cgroup_base_files, false);
+ revert_creds(cred);
unlock_drop:
cgroup_exit_root_id(root);
mutex_unlock(&cgroup_root_mutex);
@@ -1741,6 +1742,7 @@ static void cgroup_kill_sb(struct super_block *sb) {
BUG_ON(root->number_of_cgroups != 1);
BUG_ON(!list_empty(&cgrp->children));
+ mutex_lock(&cgrp->dentry->d_inode->i_mutex);
mutex_lock(&cgroup_mutex);
mutex_lock(&cgroup_root_mutex);
@@ -1773,6 +1775,7 @@ static void cgroup_kill_sb(struct super_block *sb) {
mutex_unlock(&cgroup_root_mutex);
mutex_unlock(&cgroup_mutex);
+ mutex_unlock(&cgrp->dentry->d_inode->i_mutex);
simple_xattrs_free(&cgrp->xattrs);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH 6/9] cgroup: make rebind_subsystems() handle file additions and removals with proper error handling
[not found] ` <1372463145-4245-1-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
` (10 preceding siblings ...)
2013-06-28 23:45 ` [PATCH 6/9] cgroup: make rebind_subsystems() handle file additions and removals with proper error handling Tejun Heo
@ 2013-06-28 23:45 ` Tejun Heo
[not found] ` <1372463145-4245-7-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2013-06-28 23:45 ` [PATCH 7/9] cgroup: cosmetic follow-up cleanups Tejun Heo
` (9 subsequent siblings)
21 siblings, 1 reply; 45+ messages in thread
From: Tejun Heo @ 2013-06-28 23:45 UTC (permalink / raw)
To: lizefan-hv44wF8Li93QT0dZR+AlfA
Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
cgroups-u79uwXL29TY76Z2rM5mHXA, Tejun Heo
Currently, creating and removing cgroup files in the root directory
are handled separately from the actual subsystem binding and unbinding
which happens in rebind_subsystems(). Also, rebind_subsystems() users
aren't handling file creation errors properly. Let's integrate
top_cgroup file handling into rebind_subsystems() so that it's simpler
to use and everyone handles file creation errors correctly.
* On a successful return, rebind_subsystems() is guaranteed to have
created all files of the new subsystems and deleted the ones
belonging to the removed subsystems. After a failure, no file is
created or removed.
* cgroup_remount() no longer needs to make explicit populate/clear
calls as it's all handled by rebind_subsystems(), and it gets proper
error handling automatically.
* cgroup_mount() has been updated such that the root dentry and cgroup
are linked before rebind_subsystems(). Also, the init_cred dancing
and base file handling are moved right above rebind_subsystems()
call and proper error handling for the base files is added.
* cgroup_kill_sb() calls rebind_subsystems() to unbind all subsystems
which now implies removing all subsystem files which requires the
directory's i_mutex. Grab it. This means that files on the root
cgroup are removed earlier - they used to be deleted from generic
super_block cleanup from vfs. This doesn't lead to any functional
difference and it's cleaner to do the clean up explicitly for all
files.
Combined with the previous changes, this makes all cgroup file
creation errors handled correctly.
Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
kernel/cgroup.c | 61 ++++++++++++++++++++++++++++++---------------------------
1 file changed, 32 insertions(+), 29 deletions(-)
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 3475267..f0f8bb9 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -1003,7 +1003,7 @@ static int rebind_subsystems(struct cgroupfs_root *root,
{
struct cgroup *cgrp = &root->top_cgroup;
struct cgroup_subsys *ss;
- int i;
+ int i, ret;
BUG_ON(!mutex_is_locked(&cgroup_mutex));
BUG_ON(!mutex_is_locked(&cgroup_root_mutex));
@@ -1028,7 +1028,16 @@ static int rebind_subsystems(struct cgroupfs_root *root,
if (root->number_of_cgroups > 1)
return -EBUSY;
- /* Process each subsystem */
+ ret = cgroup_populate_dir(cgrp, added_mask);
+ if (ret)
+ return ret;
+
+ /*
+ * Nothing can fail from this point on. Remove files for the
+ * removed subsystems and rebind each subsystem.
+ */
+ cgroup_clear_dir(cgrp, removed_mask);
+
for_each_subsys(ss, i) {
unsigned long bit = 1UL << i;
@@ -1364,22 +1373,9 @@ static int cgroup_remount(struct super_block *sb, int *flags, char *data)
goto out_unlock;
}
- /*
- * Clear out the files of subsystems that should be removed, do
- * this before rebind_subsystems, since rebind_subsystems may
- * change this hierarchy's subsys_list.
- */
- cgroup_clear_dir(cgrp, removed_mask);
-
ret = rebind_subsystems(root, added_mask, removed_mask);
- if (ret) {
- /* rebind_subsystems failed, re-populate the removed files */
- cgroup_populate_dir(cgrp, removed_mask);
+ if (ret)
goto out_unlock;
- }
-
- /* re-populate subsystem files */
- cgroup_populate_dir(cgrp, added_mask);
if (opts.release_agent)
strcpy(root->release_agent_path, opts.release_agent);
@@ -1579,6 +1575,7 @@ static struct dentry *cgroup_mount(struct file_system_type *fs_type,
struct super_block *sb;
struct cgroupfs_root *new_root;
struct inode *inode;
+ const struct cred *cred;
/* First find the desired set of subsystems */
mutex_lock(&cgroup_mutex);
@@ -1613,7 +1610,6 @@ static struct dentry *cgroup_mount(struct file_system_type *fs_type,
struct list_head tmp_links;
struct cgroup *root_cgrp = &root->top_cgroup;
struct cgroupfs_root *existing_root;
- const struct cred *cred;
int i;
struct css_set *cset;
@@ -1651,26 +1647,32 @@ static struct dentry *cgroup_mount(struct file_system_type *fs_type,
if (ret)
goto unlock_drop;
+ sb->s_root->d_fsdata = root_cgrp;
+ root_cgrp->dentry = sb->s_root;
+
+ cred = override_creds(&init_cred);
+
+ ret = cgroup_addrm_files(root_cgrp, NULL, cgroup_base_files, true);
+ if (ret)
+ goto rm_base_files;
+
ret = rebind_subsystems(root, root->subsys_mask, 0);
- if (ret == -EBUSY) {
+ if (ret) {
free_cgrp_cset_links(&tmp_links);
- goto unlock_drop;
+ goto rm_base_files;
}
+
+ revert_creds(cred);
+
/*
* There must be no failure case after here, since rebinding
* takes care of subsystems' refcounts, which are explicitly
* dropped in the failure exit path.
*/
- /* EBUSY should be the only error here */
- BUG_ON(ret);
-
list_add(&root->root_list, &cgroup_roots);
cgroup_root_count++;
- sb->s_root->d_fsdata = root_cgrp;
- root->top_cgroup.dentry = sb->s_root;
-
/* Link the top cgroup in this hierarchy into all
* the css_set objects */
write_lock(&css_set_lock);
@@ -1683,10 +1685,6 @@ static struct dentry *cgroup_mount(struct file_system_type *fs_type,
BUG_ON(!list_empty(&root_cgrp->children));
BUG_ON(root->number_of_cgroups != 1);
- cred = override_creds(&init_cred);
- cgroup_addrm_files(root_cgrp, NULL, cgroup_base_files, true);
- cgroup_populate_dir(root_cgrp, root->subsys_mask);
- revert_creds(cred);
mutex_unlock(&cgroup_root_mutex);
mutex_unlock(&cgroup_mutex);
mutex_unlock(&inode->i_mutex);
@@ -1715,6 +1713,9 @@ static struct dentry *cgroup_mount(struct file_system_type *fs_type,
kfree(opts.name);
return dget(sb->s_root);
+ rm_base_files:
+ cgroup_addrm_files(&root->top_cgroup, NULL, cgroup_base_files, false);
+ revert_creds(cred);
unlock_drop:
cgroup_exit_root_id(root);
mutex_unlock(&cgroup_root_mutex);
@@ -1741,6 +1742,7 @@ static void cgroup_kill_sb(struct super_block *sb) {
BUG_ON(root->number_of_cgroups != 1);
BUG_ON(!list_empty(&cgrp->children));
+ mutex_lock(&cgrp->dentry->d_inode->i_mutex);
mutex_lock(&cgroup_mutex);
mutex_lock(&cgroup_root_mutex);
@@ -1773,6 +1775,7 @@ static void cgroup_kill_sb(struct super_block *sb) {
mutex_unlock(&cgroup_root_mutex);
mutex_unlock(&cgroup_mutex);
+ mutex_unlock(&cgrp->dentry->d_inode->i_mutex);
simple_xattrs_free(&cgrp->xattrs);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH 7/9] cgroup: cosmetic follow-up cleanups
[not found] ` <1372463145-4245-1-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
` (11 preceding siblings ...)
2013-06-28 23:45 ` Tejun Heo
@ 2013-06-28 23:45 ` Tejun Heo
[not found] ` <1372463145-4245-8-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2013-06-28 23:45 ` Tejun Heo
` (8 subsequent siblings)
21 siblings, 1 reply; 45+ messages in thread
From: Tejun Heo @ 2013-06-28 23:45 UTC (permalink / raw)
To: lizefan-hv44wF8Li93QT0dZR+AlfA
Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
cgroups-u79uwXL29TY76Z2rM5mHXA, Tejun Heo
* Busy subsystem check in rebind_subsystems() is unnecessarily
verbose. Restructure it for brevity.
* The init_cred dancing in cgroup_mount() has a very high WTF factor.
Add a comment explaining what's going on and point to the original
commit.
Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
kernel/cgroup.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index f0f8bb9..51632b4 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -1009,17 +1009,9 @@ static int rebind_subsystems(struct cgroupfs_root *root,
BUG_ON(!mutex_is_locked(&cgroup_root_mutex));
/* Check that any added subsystems are currently free */
- for_each_subsys(ss, i) {
- unsigned long bit = 1UL << i;
-
- if (!(bit & added_mask))
- continue;
-
- if (ss->root != &cgroup_dummy_root) {
- /* Subsystem isn't free */
+ for_each_subsys(ss, i)
+ if (test_bit(i, &added_mask) && ss->root != &cgroup_dummy_root)
return -EBUSY;
- }
- }
/* Currently we don't handle adding/removing subsystems when
* any child cgroups exist. This is theoretically supportable
@@ -1650,6 +1642,13 @@ static struct dentry *cgroup_mount(struct file_system_type *fs_type,
sb->s_root->d_fsdata = root_cgrp;
root_cgrp->dentry = sb->s_root;
+ /*
+ * We're inside get_sb() and will call lookup_one_len() to
+ * create the root files, which doesn't work if SELinux is
+ * in use. The following cred dancing somehow works around
+ * it. See 2ce9738ba ("cgroupfs: use init_cred when
+ * populating new cgroupfs mount") for more details.
+ */
cred = override_creds(&init_cred);
ret = cgroup_addrm_files(root_cgrp, NULL, cgroup_base_files, true);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH 7/9] cgroup: cosmetic follow-up cleanups
[not found] ` <1372463145-4245-1-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
` (12 preceding siblings ...)
2013-06-28 23:45 ` [PATCH 7/9] cgroup: cosmetic follow-up cleanups Tejun Heo
@ 2013-06-28 23:45 ` Tejun Heo
2013-06-28 23:45 ` [PATCH 8/9] cgroup: move number_of_cgroups test out of rebind_subsystems() into cgroup_remount() Tejun Heo
` (7 subsequent siblings)
21 siblings, 0 replies; 45+ messages in thread
From: Tejun Heo @ 2013-06-28 23:45 UTC (permalink / raw)
To: lizefan-hv44wF8Li93QT0dZR+AlfA
Cc: Tejun Heo, cgroups-u79uwXL29TY76Z2rM5mHXA,
containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
* Busy subsystem check in rebind_subsystems() is unnecessarily
verbose. Restructure it for brevity.
* The init_cred dancing in cgroup_mount() has a very high WTF factor.
Add a comment explaining what's going on and point to the original
commit.
Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
kernel/cgroup.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index f0f8bb9..51632b4 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -1009,17 +1009,9 @@ static int rebind_subsystems(struct cgroupfs_root *root,
BUG_ON(!mutex_is_locked(&cgroup_root_mutex));
/* Check that any added subsystems are currently free */
- for_each_subsys(ss, i) {
- unsigned long bit = 1UL << i;
-
- if (!(bit & added_mask))
- continue;
-
- if (ss->root != &cgroup_dummy_root) {
- /* Subsystem isn't free */
+ for_each_subsys(ss, i)
+ if (test_bit(i, &added_mask) && ss->root != &cgroup_dummy_root)
return -EBUSY;
- }
- }
/* Currently we don't handle adding/removing subsystems when
* any child cgroups exist. This is theoretically supportable
@@ -1650,6 +1642,13 @@ static struct dentry *cgroup_mount(struct file_system_type *fs_type,
sb->s_root->d_fsdata = root_cgrp;
root_cgrp->dentry = sb->s_root;
+ /*
+ * We're inside get_sb() and will call lookup_one_len() to
+ * create the root files, which doesn't work if SELinux is
+ * in use. The following cred dancing somehow works around
+ * it. See 2ce9738ba ("cgroupfs: use init_cred when
+ * populating new cgroupfs mount") for more details.
+ */
cred = override_creds(&init_cred);
ret = cgroup_addrm_files(root_cgrp, NULL, cgroup_base_files, true);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH 8/9] cgroup: move number_of_cgroups test out of rebind_subsystems() into cgroup_remount()
[not found] ` <1372463145-4245-1-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
` (13 preceding siblings ...)
2013-06-28 23:45 ` Tejun Heo
@ 2013-06-28 23:45 ` Tejun Heo
2013-06-28 23:45 ` Tejun Heo
` (6 subsequent siblings)
21 siblings, 0 replies; 45+ messages in thread
From: Tejun Heo @ 2013-06-28 23:45 UTC (permalink / raw)
To: lizefan-hv44wF8Li93QT0dZR+AlfA
Cc: Tejun Heo, cgroups-u79uwXL29TY76Z2rM5mHXA,
containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
rebind_subsystems() currently fails if the hierarchy has any !root
cgroups; however, on the planned unified hierarchy,
rebind_subsystems() will be used while populated. Move the test to
cgroup_remount(), which is the only place the test is necessary
anyway.
As it's impossible for the other two callers of rebind_subsystems() to
have populated hierarchy, this doesn't make any behavior changes.
Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
kernel/cgroup.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 51632b4..17c8985 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -1013,13 +1013,6 @@ static int rebind_subsystems(struct cgroupfs_root *root,
if (test_bit(i, &added_mask) && ss->root != &cgroup_dummy_root)
return -EBUSY;
- /* Currently we don't handle adding/removing subsystems when
- * any child cgroups exist. This is theoretically supportable
- * but involves complex error handling, so it's being left until
- * later */
- if (root->number_of_cgroups > 1)
- return -EBUSY;
-
ret = cgroup_populate_dir(cgrp, added_mask);
if (ret)
return ret;
@@ -1365,6 +1358,12 @@ static int cgroup_remount(struct super_block *sb, int *flags, char *data)
goto out_unlock;
}
+ /* remounting is not allowed for populated hierarchies */
+ if (root->number_of_cgroups > 1) {
+ ret = -EBUSY;
+ goto out_unlock;
+ }
+
ret = rebind_subsystems(root, added_mask, removed_mask);
if (ret)
goto out_unlock;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH 8/9] cgroup: move number_of_cgroups test out of rebind_subsystems() into cgroup_remount()
[not found] ` <1372463145-4245-1-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
` (14 preceding siblings ...)
2013-06-28 23:45 ` [PATCH 8/9] cgroup: move number_of_cgroups test out of rebind_subsystems() into cgroup_remount() Tejun Heo
@ 2013-06-28 23:45 ` Tejun Heo
[not found] ` <1372463145-4245-9-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2013-06-28 23:45 ` [PATCH 9/9] blkcg: make blkcg_policy_register() correctly handle cgroup_add_cftypes() failures Tejun Heo
` (5 subsequent siblings)
21 siblings, 1 reply; 45+ messages in thread
From: Tejun Heo @ 2013-06-28 23:45 UTC (permalink / raw)
To: lizefan-hv44wF8Li93QT0dZR+AlfA
Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
cgroups-u79uwXL29TY76Z2rM5mHXA, Tejun Heo
rebind_subsystems() currently fails if the hierarchy has any !root
cgroups; however, on the planned unified hierarchy,
rebind_subsystems() will be used while populated. Move the test to
cgroup_remount(), which is the only place the test is necessary
anyway.
As it's impossible for the other two callers of rebind_subsystems() to
have populated hierarchy, this doesn't make any behavior changes.
Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
kernel/cgroup.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 51632b4..17c8985 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -1013,13 +1013,6 @@ static int rebind_subsystems(struct cgroupfs_root *root,
if (test_bit(i, &added_mask) && ss->root != &cgroup_dummy_root)
return -EBUSY;
- /* Currently we don't handle adding/removing subsystems when
- * any child cgroups exist. This is theoretically supportable
- * but involves complex error handling, so it's being left until
- * later */
- if (root->number_of_cgroups > 1)
- return -EBUSY;
-
ret = cgroup_populate_dir(cgrp, added_mask);
if (ret)
return ret;
@@ -1365,6 +1358,12 @@ static int cgroup_remount(struct super_block *sb, int *flags, char *data)
goto out_unlock;
}
+ /* remounting is not allowed for populated hierarchies */
+ if (root->number_of_cgroups > 1) {
+ ret = -EBUSY;
+ goto out_unlock;
+ }
+
ret = rebind_subsystems(root, added_mask, removed_mask);
if (ret)
goto out_unlock;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH 9/9] blkcg: make blkcg_policy_register() correctly handle cgroup_add_cftypes() failures
[not found] ` <1372463145-4245-1-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
` (15 preceding siblings ...)
2013-06-28 23:45 ` Tejun Heo
@ 2013-06-28 23:45 ` Tejun Heo
2013-06-28 23:45 ` Tejun Heo
` (4 subsequent siblings)
21 siblings, 0 replies; 45+ messages in thread
From: Tejun Heo @ 2013-06-28 23:45 UTC (permalink / raw)
To: lizefan-hv44wF8Li93QT0dZR+AlfA
Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
cgroups-u79uwXL29TY76Z2rM5mHXA, Tejun Heo, Vivek Goyal,
Jens Axboe
Hello, Jens.
How should this one be routed? It can go through either block or
cgroup tree.
Thanks!
------------------------- 8< -------------------------
From: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Subject: blkcg: make blkcg_policy_register() correctly handle cgroup_add_cftypes() failures
blkcg_policy_register() currently triggers WARN when
cgroup_add_cftypes() fails and proceeds, which is a rather crappy way
to handle errors. It was written that way as cgroup_add_cftypes()
itself didn't handle errors correctly. Now that cgroup_add_cftypes()
correctly handles errors in itself, let's handle its failure properly.
Remove the WARN_ON() and cancel and fail policy registration on
cgroup_add_cftypes() failure.
Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Vivek Goyal <vgoyal-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Cc: Jens Axboe <axboe-tSWWG44O7X1aa/9Udqfwiw@public.gmane.org>
---
block/blk-cgroup.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index e8918ff..b27a9d2 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -1127,10 +1127,13 @@ int blkcg_policy_register(struct blkcg_policy *pol)
pol->plid = i;
blkcg_policy[i] = pol;
- /* everything is in place, add intf files for the new policy */
- if (pol->cftypes)
- WARN_ON(cgroup_add_cftypes(&blkio_subsys, pol->cftypes));
+ /* try to add intf files for the new policy */
ret = 0;
+ if (pol->cftypes) {
+ ret = cgroup_add_cftypes(&blkio_subsys, pol->cftypes);
+ if (ret)
+ blkcg_policy[i] = NULL;
+ }
out_unlock:
mutex_unlock(&blkcg_pol_mutex);
return ret;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH 9/9] blkcg: make blkcg_policy_register() correctly handle cgroup_add_cftypes() failures
[not found] ` <1372463145-4245-1-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
` (16 preceding siblings ...)
2013-06-28 23:45 ` [PATCH 9/9] blkcg: make blkcg_policy_register() correctly handle cgroup_add_cftypes() failures Tejun Heo
@ 2013-06-28 23:45 ` Tejun Heo
2013-07-11 17:19 ` [PATCH 5.5/9] cgroup: use for_each_subsys() instead of for_each_root_subsys() in cgroup_populate/clear_dir() Tejun Heo
` (3 subsequent siblings)
21 siblings, 0 replies; 45+ messages in thread
From: Tejun Heo @ 2013-06-28 23:45 UTC (permalink / raw)
To: lizefan-hv44wF8Li93QT0dZR+AlfA
Cc: Tejun Heo, cgroups-u79uwXL29TY76Z2rM5mHXA,
containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
Jens Axboe, Vivek Goyal
Hello, Jens.
How should this one be routed? It can go through either block or
cgroup tree.
Thanks!
------------------------- 8< -------------------------
From: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Subject: blkcg: make blkcg_policy_register() correctly handle cgroup_add_cftypes() failures
blkcg_policy_register() currently triggers WARN when
cgroup_add_cftypes() fails and proceeds, which is a rather crappy way
to handle errors. It was written that way as cgroup_add_cftypes()
itself didn't handle errors correctly. Now that cgroup_add_cftypes()
correctly handles errors in itself, let's handle its failure properly.
Remove the WARN_ON() and cancel and fail policy registration on
cgroup_add_cftypes() failure.
Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Vivek Goyal <vgoyal-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Cc: Jens Axboe <axboe-tSWWG44O7X1aa/9Udqfwiw@public.gmane.org>
---
block/blk-cgroup.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index e8918ff..b27a9d2 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -1127,10 +1127,13 @@ int blkcg_policy_register(struct blkcg_policy *pol)
pol->plid = i;
blkcg_policy[i] = pol;
- /* everything is in place, add intf files for the new policy */
- if (pol->cftypes)
- WARN_ON(cgroup_add_cftypes(&blkio_subsys, pol->cftypes));
+ /* try to add intf files for the new policy */
ret = 0;
+ if (pol->cftypes) {
+ ret = cgroup_add_cftypes(&blkio_subsys, pol->cftypes);
+ if (ret)
+ blkcg_policy[i] = NULL;
+ }
out_unlock:
mutex_unlock(&blkcg_pol_mutex);
return ret;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH 5.5/9] cgroup: use for_each_subsys() instead of for_each_root_subsys() in cgroup_populate/clear_dir()
[not found] ` <1372463145-4245-1-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
` (17 preceding siblings ...)
2013-06-28 23:45 ` Tejun Heo
@ 2013-07-11 17:19 ` Tejun Heo
2013-07-11 17:19 ` Tejun Heo
` (2 subsequent siblings)
21 siblings, 0 replies; 45+ messages in thread
From: Tejun Heo @ 2013-07-11 17:19 UTC (permalink / raw)
To: lizefan-hv44wF8Li93QT0dZR+AlfA
Cc: cgroups-u79uwXL29TY76Z2rM5mHXA,
containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
rebind_subsystems() will be updated to handle file creations and
removals with proper error handling and to do that will need to
perform file operations before actually adding the subsystem to the
hierarchy.
To enable such usage, update cgroup_populate/clear_dir() to use
for_each_subsys() instead of for_each_root_subsys() so that they
operate on all subsystems specified by @subsys_mask whether that
subsystem is currently bound to the hierarchy or not.
Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
for_each_subsys() conversions separated out. Doesn't change the end
result from the original posting.
Thanks.
kernel/cgroup.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -965,10 +965,12 @@ static void cgroup_rm_file(struct cgroup
static void cgroup_clear_dir(struct cgroup *cgrp, unsigned long subsys_mask)
{
struct cgroup_subsys *ss;
+ int i;
- for_each_root_subsys(cgrp->root, ss) {
+ for_each_subsys(ss, i) {
struct cftype_set *set;
- if (!test_bit(ss->subsys_id, &subsys_mask))
+
+ if (!test_bit(i, &subsys_mask))
continue;
list_for_each_entry(set, &ss->cftsets, node)
cgroup_addrm_files(cgrp, NULL, set->cfts, false);
@@ -4177,12 +4179,13 @@ static struct cftype cgroup_base_files[]
static int cgroup_populate_dir(struct cgroup *cgrp, unsigned long subsys_mask)
{
struct cgroup_subsys *ss;
- int ret = 0;
+ int i, ret = 0;
/* process cftsets of each subsystem */
- for_each_root_subsys(cgrp->root, ss) {
+ for_each_subsys(ss, i) {
struct cftype_set *set;
- if (!test_bit(ss->subsys_id, &subsys_mask))
+
+ if (!test_bit(i, &subsys_mask))
continue;
list_for_each_entry(set, &ss->cftsets, node) {
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 5.5/9] cgroup: use for_each_subsys() instead of for_each_root_subsys() in cgroup_populate/clear_dir()
[not found] ` <1372463145-4245-1-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
` (18 preceding siblings ...)
2013-07-11 17:19 ` [PATCH 5.5/9] cgroup: use for_each_subsys() instead of for_each_root_subsys() in cgroup_populate/clear_dir() Tejun Heo
@ 2013-07-11 17:19 ` Tejun Heo
[not found] ` <20130711171927.GE10195-9pTldWuhBndy/B6EtB590w@public.gmane.org>
2013-07-12 7:45 ` [PATCHSET] cgroup: fix and clean up cgroup file creations and removals Tejun Heo
2013-07-12 7:45 ` Tejun Heo
21 siblings, 1 reply; 45+ messages in thread
From: Tejun Heo @ 2013-07-11 17:19 UTC (permalink / raw)
To: lizefan-hv44wF8Li93QT0dZR+AlfA
Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
cgroups-u79uwXL29TY76Z2rM5mHXA
rebind_subsystems() will be updated to handle file creations and
removals with proper error handling and to do that will need to
perform file operations before actually adding the subsystem to the
hierarchy.
To enable such usage, update cgroup_populate/clear_dir() to use
for_each_subsys() instead of for_each_root_subsys() so that they
operate on all subsystems specified by @subsys_mask whether that
subsystem is currently bound to the hierarchy or not.
Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
for_each_subsys() conversions separated out. Doesn't change the end
result from the original posting.
Thanks.
kernel/cgroup.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -965,10 +965,12 @@ static void cgroup_rm_file(struct cgroup
static void cgroup_clear_dir(struct cgroup *cgrp, unsigned long subsys_mask)
{
struct cgroup_subsys *ss;
+ int i;
- for_each_root_subsys(cgrp->root, ss) {
+ for_each_subsys(ss, i) {
struct cftype_set *set;
- if (!test_bit(ss->subsys_id, &subsys_mask))
+
+ if (!test_bit(i, &subsys_mask))
continue;
list_for_each_entry(set, &ss->cftsets, node)
cgroup_addrm_files(cgrp, NULL, set->cfts, false);
@@ -4177,12 +4179,13 @@ static struct cftype cgroup_base_files[]
static int cgroup_populate_dir(struct cgroup *cgrp, unsigned long subsys_mask)
{
struct cgroup_subsys *ss;
- int ret = 0;
+ int i, ret = 0;
/* process cftsets of each subsystem */
- for_each_root_subsys(cgrp->root, ss) {
+ for_each_subsys(ss, i) {
struct cftype_set *set;
- if (!test_bit(ss->subsys_id, &subsys_mask))
+
+ if (!test_bit(i, &subsys_mask))
continue;
list_for_each_entry(set, &ss->cftsets, node) {
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCHSET] cgroup: fix and clean up cgroup file creations and removals
[not found] ` <1372463145-4245-1-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
` (19 preceding siblings ...)
2013-07-11 17:19 ` Tejun Heo
@ 2013-07-12 7:45 ` Tejun Heo
2013-07-12 7:45 ` Tejun Heo
21 siblings, 0 replies; 45+ messages in thread
From: Tejun Heo @ 2013-07-12 7:45 UTC (permalink / raw)
To: lizefan-hv44wF8Li93QT0dZR+AlfA
Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
cgroups-u79uwXL29TY76Z2rM5mHXA
On Fri, Jun 28, 2013 at 04:45:36PM -0700, Tejun Heo wrote:
> cgroup has been pretty lax about errors while creating cgroupfs files.
> In most cases, it just ignores the errors and continues - cgroups
> created or subsystems bound under heavy memory pressure could be
> missing some files. As such low memory conditions aren't too
> frequent, this hasn't caused a lot of problems but this is lousy and
> we'll be creating and removing cgroupfs files a lot more dynamically
> with unified hierarchy. Let's get it in proper shape.
Applied to cgroup/for-3.12.
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCHSET] cgroup: fix and clean up cgroup file creations and removals
[not found] ` <1372463145-4245-1-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
` (20 preceding siblings ...)
2013-07-12 7:45 ` [PATCHSET] cgroup: fix and clean up cgroup file creations and removals Tejun Heo
@ 2013-07-12 7:45 ` Tejun Heo
21 siblings, 0 replies; 45+ messages in thread
From: Tejun Heo @ 2013-07-12 7:45 UTC (permalink / raw)
To: lizefan-hv44wF8Li93QT0dZR+AlfA
Cc: cgroups-u79uwXL29TY76Z2rM5mHXA,
containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
On Fri, Jun 28, 2013 at 04:45:36PM -0700, Tejun Heo wrote:
> cgroup has been pretty lax about errors while creating cgroupfs files.
> In most cases, it just ignores the errors and continues - cgroups
> created or subsystems bound under heavy memory pressure could be
> missing some files. As such low memory conditions aren't too
> frequent, this hasn't caused a lot of problems but this is lousy and
> we'll be creating and removing cgroupfs files a lot more dynamically
> with unified hierarchy. Let's get it in proper shape.
Applied to cgroup/for-3.12.
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 45+ messages in thread