linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ben Blum <bblum@andrew.cmu.edu>
To: Ben Blum <bblum@andrew.cmu.edu>
Cc: linux-kernel@vger.kernel.org,
	containers@lists.linux-foundation.org, akpm@linux-foundation.org,
	ebiederm@xmission.com, lizf@cn.fujitsu.com, matthltc@us.ibm.com,
	menage@google.com, oleg@redhat.com,
	David Rientjes <rientjes@google.com>,
	Miao Xie <miaox@cn.fujitsu.com>
Subject: [PATCH v8 4/3] cgroups: use flex_array in attach_proc
Date: Wed, 16 Feb 2011 14:22:00 -0500	[thread overview]
Message-ID: <20110216192200.GA11980@ghc17.ghc.andrew.cmu.edu> (raw)
In-Reply-To: <20110208013950.GF31569@ghc17.ghc.andrew.cmu.edu>

Convert cgroup_attach_proc to use flex_array.

From: Ben Blum <bblum@andrew.cmu.edu>

The cgroup_attach_proc implementation requires a pre-allocated array to store
task pointers to atomically move a thread-group, but asking for a monolithic
array with kmalloc() may be unreliable for very large groups. Using flex_array
provides the same functionality with less risk of failure.

This is a post-patch for cgroup-procs-write.patch.

Signed-off-by: Ben Blum <bblum@andrew.cmu.edu>
---
 kernel/cgroup.c |   37 ++++++++++++++++++++++++++++---------
 1 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 58b364a..feba784 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -57,6 +57,7 @@
 #include <linux/vmalloc.h> /* TODO: replace with more sophisticated array */
 #include <linux/eventfd.h>
 #include <linux/poll.h>
+#include <linux/flex_array.h> /* used in cgroup_attach_proc */
 
 #include <asm/atomic.h>
 
@@ -1985,7 +1986,7 @@ int cgroup_attach_proc(struct cgroup *cgrp, struct task_struct *leader)
 	struct cgroupfs_root *root = cgrp->root;
 	/* threadgroup list cursor and array */
 	struct task_struct *tsk;
-	struct task_struct **group;
+	struct flex_array *group;
 	/*
 	 * we need to make sure we have css_sets for all the tasks we're
 	 * going to move -before- we actually start moving them, so that in
@@ -2002,9 +2003,15 @@ int cgroup_attach_proc(struct cgroup *cgrp, struct task_struct *leader)
 	 * and if threads exit, this will just be an over-estimate.
 	 */
 	group_size = get_nr_threads(leader);
-	group = kmalloc(group_size * sizeof(*group), GFP_KERNEL);
+	/* flex_array supports very large thread-groups better than kmalloc. */
+	group = flex_array_alloc(sizeof(struct task_struct *), group_size,
+				 GFP_KERNEL);
 	if (!group)
 		return -ENOMEM;
+	/* pre-allocate to guarantee space while iterating in rcu read-side. */
+	retval = flex_array_prealloc(group, 0, group_size - 1, GFP_KERNEL);
+	if (retval)
+		goto out_free_group_list;
 
 	/* prevent changes to the threadgroup list while we take a snapshot. */
 	rcu_read_lock();
@@ -2027,7 +2034,12 @@ int cgroup_attach_proc(struct cgroup *cgrp, struct task_struct *leader)
 		/* as per above, nr_threads may decrease, but not increase. */
 		BUG_ON(i >= group_size);
 		get_task_struct(tsk);
-		group[i] = tsk;
+		/*
+		 * saying GFP_ATOMIC has no effect here because we did prealloc
+		 * earlier, but it's good form to communicate our expectations.
+		 */
+		retval = flex_array_put_ptr(group, i, tsk, GFP_ATOMIC);
+		BUG_ON(retval != 0);
 		i++;
 	} while_each_thread(leader, tsk);
 	/* remember the number of threads in the array for later. */
@@ -2050,7 +2062,9 @@ int cgroup_attach_proc(struct cgroup *cgrp, struct task_struct *leader)
 		if (ss->can_attach_task) {
 			/* run on each task in the threadgroup. */
 			for (i = 0; i < group_size; i++) {
-				retval = ss->can_attach_task(cgrp, group[i]);
+				tsk = flex_array_get_ptr(group, i);
+				BUG_ON(tsk == NULL);
+				retval = ss->can_attach_task(cgrp, tsk);
 				if (retval) {
 					failed_ss = ss;
 					goto out_cancel_attach;
@@ -2065,7 +2079,8 @@ int cgroup_attach_proc(struct cgroup *cgrp, struct task_struct *leader)
 	 */
 	INIT_LIST_HEAD(&newcg_list);
 	for (i = 0; i < group_size; i++) {
-		tsk = group[i];
+		tsk = flex_array_get_ptr(group, i);
+		BUG_ON(tsk == NULL);
 		/* nothing to do if this task is already in the cgroup */
 		oldcgrp = task_cgroup_from_root(tsk, root);
 		if (cgrp == oldcgrp)
@@ -2104,7 +2119,8 @@ int cgroup_attach_proc(struct cgroup *cgrp, struct task_struct *leader)
 			ss->pre_attach(cgrp);
 	}
 	for (i = 0; i < group_size; i++) {
-		tsk = group[i];
+		tsk = flex_array_get_ptr(group, i);
+		BUG_ON(tsk == NULL);
 		/* leave current thread as it is if it's already there */
 		oldcgrp = task_cgroup_from_root(tsk, root);
 		if (cgrp == oldcgrp)
@@ -2154,10 +2170,13 @@ out_cancel_attach:
 		}
 	}
 	/* clean up the array of referenced threads in the group. */
-	for (i = 0; i < group_size; i++)
-		put_task_struct(group[i]);
+	for (i = 0; i < group_size; i++) {
+		tsk = flex_array_get_ptr(group, i);
+		BUG_ON(tsk == NULL);
+		put_task_struct(tsk);
+	}
 out_free_group_list:
-	kfree(group);
+	flex_array_free(group);
 	return retval;
 }
 

  reply	other threads:[~2011-02-16 19:23 UTC|newest]

Thread overview: 73+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-07-30 23:56 [PATCH v4 0/2] cgroups: implement moving a threadgroup's threads atomically with cgroup.procs Ben Blum
2010-07-30 23:57 ` [PATCH v4 1/2] cgroups: read-write lock CLONE_THREAD forking per threadgroup Ben Blum
2010-08-04  3:44   ` Paul Menage
2010-08-04  4:33     ` Ben Blum
2010-08-04  4:34       ` Paul Menage
2010-08-06  6:02         ` Ben Blum
2010-08-06  7:08           ` KAMEZAWA Hiroyuki
2010-08-04 16:34     ` Brian K. White
2010-07-30 23:59 ` [PATCH v4 2/2] cgroups: make procs file writable Ben Blum
2010-08-04  1:08   ` KAMEZAWA Hiroyuki
2010-08-04  4:28     ` Ben Blum
2010-08-04  4:30     ` Paul Menage
2010-08-04  4:38       ` Ben Blum
2010-08-04  4:46         ` Paul Menage
2010-08-03 19:58 ` [PATCH v4 0/2] cgroups: implement moving a threadgroup's threads atomically with cgroup.procs Andrew Morton
2010-08-03 23:45   ` KAMEZAWA Hiroyuki
2010-08-04  2:00   ` Li Zefan
2010-08-11  5:46 ` [PATCH v5 0/3] " Ben Blum
2010-08-11  5:47   ` [PATCH v5 1/3] cgroups: read-write lock CLONE_THREAD forking per threadgroup Ben Blum
2010-08-23 23:35     ` Paul Menage
2010-08-11  5:48   ` [PATCH v5 2/3] cgroups: add can_attach callback for checking all threads in a group Ben Blum
2010-08-23 23:31     ` Paul Menage
2010-08-11  5:48   ` [PATCH v5 3/3] cgroups: make procs file writable Ben Blum
2010-08-24 18:08     ` Paul Menage
2010-12-24  8:22   ` [PATCH v6 0/3] cgroups: implement moving a threadgroup's threads atomically with cgroup.procs Ben Blum
2010-12-24  8:23     ` [PATCH v6 1/3] cgroups: read-write lock CLONE_THREAD forking per threadgroup Ben Blum
2010-12-24  8:24     ` [PATCH v6 2/3] cgroups: add can_attach callback for checking all threads in a group Ben Blum
2010-12-24  8:24     ` [PATCH v6 3/3] cgroups: make procs file writable Ben Blum
2011-01-12 23:26       ` Paul E. McKenney
2010-12-26 12:09     ` [PATCH v7 0/3] cgroups: implement moving a threadgroup's threads atomically with cgroup.procs Ben Blum
2010-12-26 12:09       ` [PATCH v7 1/3] cgroups: read-write lock CLONE_THREAD forking per threadgroup Ben Blum
2011-01-24  8:38         ` Paul Menage
2011-01-24 21:05         ` Andrew Morton
2011-02-04 21:25           ` Ben Blum
2011-02-04 21:36             ` Andrew Morton
2011-02-04 21:43               ` Ben Blum
2011-02-14  5:31           ` Paul Menage
2010-12-26 12:11       ` [PATCH v7 2/3] cgroups: add atomic-context per-thread subsystem callbacks Ben Blum
2011-01-24  8:38         ` Paul Menage
2011-01-24 15:32           ` Ben Blum
2010-12-26 12:12       ` [PATCH v7 3/3] cgroups: make procs file writable Ben Blum
2011-02-08  1:35       ` [PATCH v8 0/3] cgroups: implement moving a threadgroup's threads atomically with cgroup.procs Ben Blum
2011-02-08  1:37         ` [PATCH v8 1/3] cgroups: read-write lock CLONE_THREAD forking per threadgroup Ben Blum
2011-03-03 17:54           ` Paul Menage
2011-02-08  1:39         ` [PATCH v8 2/3] cgroups: add per-thread subsystem callbacks Ben Blum
2011-03-03 17:59           ` Paul Menage
2011-02-08  1:39         ` [PATCH v8 3/3] cgroups: make procs file writable Ben Blum
2011-02-16 19:22           ` Ben Blum [this message]
2011-03-03 17:48             ` [PATCH v8 4/3] cgroups: use flex_array in attach_proc Paul Menage
2011-03-22  5:15               ` Ben Blum
2011-03-22  5:19                 ` [PATCH v8.5 " Ben Blum
2011-03-03 18:38           ` [PATCH v8 3/3] cgroups: make procs file writable Paul Menage
2011-03-10  6:18             ` Ben Blum
2011-03-10 20:01               ` Paul Menage
2011-03-15 21:13                 ` Ben Blum
2011-03-18 16:54                   ` Paul Menage
2011-03-22  5:18                     ` [PATCH v8.5 " Ben Blum
2011-03-29 23:27                       ` Paul Menage
2011-03-29 23:39                         ` Andrew Morton
2011-03-22  5:08               ` [PATCH v8 " Ben Blum
2011-02-09 23:10         ` [PATCH v8 0/3] cgroups: implement moving a threadgroup's threads atomically with cgroup.procs Andrew Morton
2011-02-10  1:02           ` KAMEZAWA Hiroyuki
2011-02-10  1:36             ` Ben Blum
2011-02-14  6:12             ` Paul Menage
2011-02-14  6:12           ` Paul Menage
2011-04-06 19:44         ` [PATCH v8.75 0/4] " Ben Blum
2011-04-06 19:45           ` [PATCH v8.75 1/4] cgroups: read-write lock CLONE_THREAD forking per threadgroup Ben Blum
2011-04-06 19:46           ` [PATCH v8.75 2/4] cgroups: add per-thread subsystem callbacks Ben Blum
2011-04-06 19:46           ` [PATCH v8.75 3/4] cgroups: make procs file writable Ben Blum
2011-04-06 19:47           ` [PATCH v8.75 4/4] cgroups: use flex_array in attach_proc Ben Blum
2011-04-12 23:25           ` [PATCH v8.75 0/4] cgroups: implement moving a threadgroup's threads atomically with cgroup.procs Andrew Morton
2011-04-12 23:59             ` Ben Blum
2011-04-13  2:07             ` Li Zefan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110216192200.GA11980@ghc17.ghc.andrew.cmu.edu \
    --to=bblum@andrew.cmu.edu \
    --cc=akpm@linux-foundation.org \
    --cc=containers@lists.linux-foundation.org \
    --cc=ebiederm@xmission.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lizf@cn.fujitsu.com \
    --cc=matthltc@us.ibm.com \
    --cc=menage@google.com \
    --cc=miaox@cn.fujitsu.com \
    --cc=oleg@redhat.com \
    --cc=rientjes@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).