[Devel] [PATCH rh7 2/2] ms/cgroup: make cgroup_addrm_files() clean up after itself on failures

[Konstantin Khorenko]

From: Tejun Heo <tj at kernel.org>

After a file creation failure, cgroup_addrm_files() it didn't remove
the files which had already been created.  When cgroup_populate_dir()
is the caller, this is fine as the caller performs cleanup; however,
for other callers, this may leave unactivated dangling files behind.
As kernfs directory removals are recursive, this doesn't lead to
permanent memory leak but it can, for example, fail future attempts to
create those files again.

There's no point in keeping around this sort of subtlety and it gets
in the way of planned updates to file handling.  This patch makes
cgroup_addrm_files() clean up after itself on failures.

Signed-off-by: Tejun Heo <tj at kernel.org>
Cc: Li Zefan <lizefan at huawei.com>
Cc: Johannes Weiner <hannes at cmpxchg.org>

During that bug mentioned below investigation we have seen
messages from cgroup_addrm_files() about failed attempt to add a
directory, so let's backport this patch.

https://virtuozzo.atlassian.net/browse/PSBM-155867
(cherry picked from commit 6732ed853af942ba20ddbd091336acc7df569119)
Signed-off-by: Konstantin Khorenko <khorenko at virtuozzo.com>
---
 kernel/cgroup.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index f8af3c6dc5df..348563b2d41f 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -3141,20 +3141,19 @@ static int cgroup_add_file(struct cgroup *cgrp, struct cgroup_subsys *subsys,
  * @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.
+ * For removals, this function never fails.
  */
 static int cgroup_addrm_files(struct cgroup *cgrp, struct cgroup_subsys *subsys,
 			      struct cftype cfts[], bool is_add)
 {
-	struct cftype *cft;
+	struct cftype *cft, *cft_end = NULL;
 	int ret;
 
 	lockdep_assert_held(&cgrp->dentry->d_inode->i_mutex);
 	lockdep_assert_held(&cgroup_mutex);
 
-	for (cft = cfts; cft->name[0] != '\0'; cft++) {
+restart:
+	for (cft = cfts; cft != cft_end && cft->name[0] != '\0'; cft++) {
 		/* does cft->flags tell us to skip this file on @cgrp? */
 		if ((cft->flags & CFTYPE_INSANE) && cgroup_sane_behavior(cgrp))
 			continue;
@@ -3168,7 +3167,9 @@ static int cgroup_addrm_files(struct cgroup *cgrp, struct cgroup_subsys *subsys,
 			if (ret) {
 				pr_warn("cgroup_addrm_files: failed to add %s, err=%d\n",
 					cft->name, ret);
-				return ret;
+				cft_end = cft;
+				is_add = false;
+				goto restart;
 			}
 		} else {
 			cgroup_rm_file(cgrp, cft);
-- 
2.24.3