Linux Container Development
 help / color / mirror / Atom feed
From: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
To: paul-inf54ven1CmVyaH7bEyXVA@public.gmane.org,
	rjw-KKrjLPT3xs0@public.gmane.org,
	lizf-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org
Cc: fweisbec-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	oleg-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
	Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	linux-pm-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org,
	akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org
Subject: [PATCH 06/10] cgroup: improve old cgroup handling in cgroup_attach_proc()
Date: Tue,  1 Nov 2011 16:46:29 -0700	[thread overview]
Message-ID: <1320191193-8110-7-git-send-email-tj@kernel.org> (raw)
In-Reply-To: <1320191193-8110-1-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

cgroup_attach_proc() behaves differently from cgroup_attach_task() in
the following aspects.

* All hooks are invoked even if no task is actually being moved.

* ->can_attach_task() is called for all tasks in the group whether the
  new cgrp is different from the current cgrp or not; however,
  ->attach_task() is skipped if new equals new.  This makes the calls
  asymmetric.

This patch improves old cgroup handling in cgroup_attach_proc() by
looking up the current cgroup at the head, recording it in the flex
array along with the task itself, and using it to remove the above two
differences.  This will also ease further changes.

-v2: nr_todo renamed to nr_migrating_tasks as per Paul Menage's
     suggestion.

Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Acked-by: Paul Menage <paul-inf54ven1CmVyaH7bEyXVA@public.gmane.org>
Reviewed-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A@public.gmane.org>
Cc: Li Zefan <lizf-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
---
 kernel/cgroup.c |   66 +++++++++++++++++++++++++++++++++++--------------------
 1 files changed, 42 insertions(+), 24 deletions(-)

diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 3679fb6..19396d6 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -1757,6 +1757,11 @@ int cgroup_path(const struct cgroup *cgrp, char *buf, int buflen)
 }
 EXPORT_SYMBOL_GPL(cgroup_path);
 
+struct task_and_cgroup {
+	struct task_struct	*task;
+	struct cgroup		*cgrp;
+};
+
 /*
  * cgroup_task_migrate - move a task from one cgroup to another.
  *
@@ -2008,15 +2013,15 @@ static int css_set_prefetch(struct cgroup *cgrp, struct css_set *cg,
  */
 int cgroup_attach_proc(struct cgroup *cgrp, struct task_struct *leader)
 {
-	int retval, i, group_size;
+	int retval, i, group_size, nr_migrating_tasks;
 	struct cgroup_subsys *ss, *failed_ss = NULL;
 	bool cancel_failed_ss = false;
 	/* guaranteed to be initialized later, but the compiler needs this */
-	struct cgroup *oldcgrp = NULL;
 	struct css_set *oldcg;
 	struct cgroupfs_root *root = cgrp->root;
 	/* threadgroup list cursor and array */
 	struct task_struct *tsk;
+	struct task_and_cgroup *tc;
 	struct flex_array *group;
 	/*
 	 * we need to make sure we have css_sets for all the tasks we're
@@ -2035,8 +2040,7 @@ int cgroup_attach_proc(struct cgroup *cgrp, struct task_struct *leader)
 	 */
 	group_size = get_nr_threads(leader);
 	/* flex_array supports very large thread-groups better than kmalloc. */
-	group = flex_array_alloc(sizeof(struct task_struct *), group_size,
-				 GFP_KERNEL);
+	group = flex_array_alloc(sizeof(*tc), group_size, GFP_KERNEL);
 	if (!group)
 		return -ENOMEM;
 	/* pre-allocate to guarantee space while iterating in rcu read-side. */
@@ -2060,8 +2064,10 @@ int cgroup_attach_proc(struct cgroup *cgrp, struct task_struct *leader)
 	}
 	/* take a reference on each task in the group to go in the array. */
 	tsk = leader;
-	i = 0;
+	i = nr_migrating_tasks = 0;
 	do {
+		struct task_and_cgroup ent;
+
 		/* @tsk either already exited or can't exit until the end */
 		if (tsk->flags & PF_EXITING)
 			continue;
@@ -2073,14 +2079,23 @@ int cgroup_attach_proc(struct cgroup *cgrp, struct task_struct *leader)
 		 * 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);
+		ent.task = tsk;
+		ent.cgrp = task_cgroup_from_root(tsk, root);
+		retval = flex_array_put(group, i, &ent, GFP_ATOMIC);
 		BUG_ON(retval != 0);
 		i++;
+		if (ent.cgrp != cgrp)
+			nr_migrating_tasks++;
 	} while_each_thread(leader, tsk);
 	/* remember the number of threads in the array for later. */
 	group_size = i;
 	rcu_read_unlock();
 
+	/* methods shouldn't be called if no task is actually migrating */
+	retval = 0;
+	if (!nr_migrating_tasks)
+		goto out_put_tasks;
+
 	/*
 	 * step 1: check that we can legitimately attach to the cgroup.
 	 */
@@ -2096,8 +2111,10 @@ 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++) {
-				tsk = flex_array_get_ptr(group, i);
-				retval = ss->can_attach_task(cgrp, tsk);
+				tc = flex_array_get(group, i);
+				if (tc->cgrp == cgrp)
+					continue;
+				retval = ss->can_attach_task(cgrp, tc->task);
 				if (retval) {
 					failed_ss = ss;
 					cancel_failed_ss = true;
@@ -2113,18 +2130,17 @@ int cgroup_attach_proc(struct cgroup *cgrp, struct task_struct *leader)
 	 */
 	INIT_LIST_HEAD(&newcg_list);
 	for (i = 0; i < group_size; i++) {
-		tsk = flex_array_get_ptr(group, i);
+		tc = flex_array_get(group, i);
 		/* nothing to do if this task is already in the cgroup */
-		oldcgrp = task_cgroup_from_root(tsk, root);
-		if (cgrp == oldcgrp)
+		if (tc->cgrp == cgrp)
 			continue;
 		/* get old css_set pointer */
-		task_lock(tsk);
-		oldcg = tsk->cgroups;
+		task_lock(tc->task);
+		oldcg = tc->task->cgroups;
 		get_css_set(oldcg);
-		task_unlock(tsk);
+		task_unlock(tc->task);
 		/* see if the new one for us is already in the list? */
-		if (css_set_check_fetched(cgrp, tsk, oldcg, &newcg_list)) {
+		if (css_set_check_fetched(cgrp, tc->task, oldcg, &newcg_list)) {
 			/* was already there, nothing to do. */
 			put_css_set(oldcg);
 		} else {
@@ -2147,19 +2163,18 @@ int cgroup_attach_proc(struct cgroup *cgrp, struct task_struct *leader)
 			ss->pre_attach(cgrp);
 	}
 	for (i = 0; i < group_size; i++) {
-		tsk = flex_array_get_ptr(group, i);
+		tc = flex_array_get(group, i);
 		/* leave current thread as it is if it's already there */
-		oldcgrp = task_cgroup_from_root(tsk, root);
-		if (cgrp == oldcgrp)
+		if (tc->cgrp == cgrp)
 			continue;
 
-		retval = cgroup_task_migrate(cgrp, oldcgrp, tsk, true);
+		retval = cgroup_task_migrate(cgrp, tc->cgrp, tc->task, true);
 		BUG_ON(retval != 0);
 
 		/* attach each task to each subsystem */
 		for_each_subsys(root, ss) {
 			if (ss->attach_task)
-				ss->attach_task(cgrp, tsk);
+				ss->attach_task(cgrp, tc->task);
 		}
 	}
 	/* nothing is sensitive to fork() after this point. */
@@ -2170,8 +2185,10 @@ int cgroup_attach_proc(struct cgroup *cgrp, struct task_struct *leader)
 	 * being moved, this call will need to be reworked to communicate that.
 	 */
 	for_each_subsys(root, ss) {
-		if (ss->attach)
-			ss->attach(ss, cgrp, oldcgrp, leader);
+		if (ss->attach) {
+			tc = flex_array_get(group, 0);
+			ss->attach(ss, cgrp, tc->cgrp, tc->task);
+		}
 	}
 
 	/*
@@ -2200,10 +2217,11 @@ out_cancel_attach:
 				ss->cancel_attach(ss, cgrp, leader);
 		}
 	}
+out_put_tasks:
 	/* clean up the array of referenced threads in the group. */
 	for (i = 0; i < group_size; i++) {
-		tsk = flex_array_get_ptr(group, i);
-		put_task_struct(tsk);
+		tc = flex_array_get(group, i);
+		put_task_struct(tc->task);
 	}
 out_free_group_list:
 	flex_array_free(group);
-- 
1.7.3.1

  parent reply	other threads:[~2011-11-01 23:46 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-11-01 23:46 [PATCHSET] cgroup: stable threadgroup during attach & subsys methods consolidation Tejun Heo
     [not found] ` <1320191193-8110-2-git-send-email-tj@kernel.org>
     [not found]   ` <1320191193-8110-2-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2011-11-04  8:38     ` [PATCH 01/10] cgroup: add cgroup_root_mutex KAMEZAWA Hiroyuki
     [not found] ` <1320191193-8110-5-git-send-email-tj@kernel.org>
     [not found]   ` <1320191193-8110-5-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2011-11-04  8:54     ` [PATCH 04/10] cgroup: always lock threadgroup during migration KAMEZAWA Hiroyuki
2011-11-14 18:46     ` Frederic Weisbecker
2011-11-14 18:52       ` Frederic Weisbecker
     [not found]       ` <20111114185242.GF9446@somewhere>
2011-11-21 22:05         ` Tejun Heo
     [not found]   ` <20111104175413.30afaf8e.kamezawa.hiroyu@jp.fujitsu.com>
     [not found]     ` <20111104175413.30afaf8e.kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A@public.gmane.org>
2011-11-04 15:21       ` Tejun Heo
     [not found] ` <1320191193-8110-6-git-send-email-tj@kernel.org>
     [not found]   ` <1320191193-8110-6-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2011-11-14 20:06     ` [PATCH 05/10] cgroup: subsys->attach_task() should be called after migration Frederic Weisbecker
     [not found]   ` <20111114200553.GG9446@somewhere>
2011-11-21 22:04     ` Tejun Heo
     [not found] ` <1320191193-8110-9-git-send-email-tj@kernel.org>
     [not found]   ` <1320191193-8110-9-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2011-11-04  9:08     ` [PATCH 08/10] cgroup: don't use subsys->can_attach_task() or ->attach_task() KAMEZAWA Hiroyuki
2011-11-14 23:54     ` Frederic Weisbecker
     [not found] ` <1320191193-8110-4-git-send-email-tj@kernel.org>
     [not found]   ` <1320191193-8110-4-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2011-11-04  8:45     ` [PATCH 03/10] threadgroup: extend threadgroup_lock() to cover exit and exec KAMEZAWA Hiroyuki
2011-11-13 16:44     ` Frederic Weisbecker
2011-11-14 13:54       ` Frederic Weisbecker
2011-11-21 22:03         ` Tejun Heo
     [not found]           ` <20111121220326.GM25776-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2011-11-23 14:34             ` Frederic Weisbecker
2011-11-21 21:58       ` Tejun Heo
     [not found]         ` <20111121215839.GL25776-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2011-11-23 14:02           ` Frederic Weisbecker
     [not found]             ` <20111123140139.GB10669-oHC15RC7JGTpAmv0O++HtFaTQe2KTcn/@public.gmane.org>
2011-11-24 21:22               ` Tejun Heo
2011-11-13 18:20     ` Frederic Weisbecker
2011-11-24 22:50     ` [PATCH UPDATED " Tejun Heo
     [not found]   ` <20111124225054.GA14828@google.com>
     [not found]     ` <20111124225054.GA14828-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2011-11-25  4:02       ` Linus Torvalds
2011-11-25 14:01       ` Frederic Weisbecker
     [not found]     ` <CA+55aFxq1wztMzYhKaY5RHazLBDz4pSXUgiGzTj2wA6EJcDbAw@mail.gmail.com>
     [not found]       ` <CA+55aFxq1wztMzYhKaY5RHazLBDz4pSXUgiGzTj2wA6EJcDbAw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2011-11-27 19:21         ` Tejun Heo
     [not found]           ` <20111127192155.GB4266-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2011-11-27 21:25             ` Tejun Heo
     [not found]               ` <20111127212558.GE4266-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2011-12-01 19:29                 ` Tejun Heo
     [not found]     ` <20111125140136.GC23307@somewhere.redhat.com>
     [not found]       ` <20111125140136.GC23307-oHC15RC7JGTpAmv0O++HtFaTQe2KTcn/@public.gmane.org>
2011-11-27 19:30         ` Tejun Heo
     [not found]           ` <20111127193001.GC4266-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2011-12-02 16:28             ` Frederic Weisbecker
     [not found]           ` <20111202162753.GA19752@somewhere.redhat.com>
     [not found]             ` <20111202162753.GA19752-oHC15RC7JGTpAmv0O++HtFaTQe2KTcn/@public.gmane.org>
2011-12-05 18:43               ` Tejun Heo
     [not found]                 ` <20111205184315.GJ627-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2011-12-07 15:30                   ` Frederic Weisbecker
     [not found]                 ` <20111207153046.GC13252@somewhere.redhat.com>
     [not found]                   ` <20111207153046.GC13252-oHC15RC7JGTpAmv0O++HtFaTQe2KTcn/@public.gmane.org>
2011-12-07 18:22                     ` Tejun Heo
     [not found]                   ` <20111207182214.GA7610@google.com>
     [not found]                     ` <20111207182214.GA7610-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2011-12-08 20:50                       ` [PATCH UPDATED AGAIN " Tejun Heo
     [not found]                     ` <20111208205055.GB12108@google.com>
     [not found]                       ` <20111208205055.GB12108-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2011-12-09 23:42                         ` Frederic Weisbecker
2011-12-13  1:33                           ` Tejun Heo
     [not found]                           ` <20111213013334.GC25802@google.com>
     [not found]                             ` <20111213013334.GC25802-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2011-12-13  2:17                               ` Tejun Heo
     [not found] ` <1320191193-8110-1-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2011-11-01 23:46   ` [PATCH 01/10] cgroup: add cgroup_root_mutex Tejun Heo
2011-11-01 23:46   ` [PATCH 02/10] threadgroup: rename signal->threadgroup_fork_lock to ->group_rwsem Tejun Heo
     [not found]     ` <1320191193-8110-3-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2011-11-04  8:40       ` KAMEZAWA Hiroyuki
     [not found]     ` <20111104174032.e0c4fc11.kamezawa.hiroyu@jp.fujitsu.com>
     [not found]       ` <20111104174032.e0c4fc11.kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A@public.gmane.org>
2011-11-04 15:16         ` Tejun Heo
2011-11-01 23:46   ` [PATCH 03/10] threadgroup: extend threadgroup_lock() to cover exit and exec Tejun Heo
2011-11-01 23:46   ` [PATCH 04/10] cgroup: always lock threadgroup during migration Tejun Heo
2011-11-01 23:46   ` [PATCH 05/10] cgroup: subsys->attach_task() should be called after migration Tejun Heo
2011-11-01 23:46   ` Tejun Heo [this message]
     [not found]     ` <1320191193-8110-7-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2011-11-14 20:37       ` [PATCH 06/10] cgroup: improve old cgroup handling in cgroup_attach_proc() Frederic Weisbecker
2011-11-01 23:46   ` [PATCH 07/10] cgroup: introduce cgroup_taskset and use it in subsys->can_attach(), cancel_attach() and attach() Tejun Heo
     [not found]     ` <1320191193-8110-8-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2011-11-14 21:16       ` Frederic Weisbecker
2011-11-01 23:46   ` [PATCH 08/10] cgroup: don't use subsys->can_attach_task() or ->attach_task() Tejun Heo
2011-11-01 23:46   ` [PATCH 09/10] cgroup, cpuset: don't use ss->pre_attach() Tejun Heo
     [not found]     ` <1320191193-8110-10-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2011-11-15  0:51       ` Frederic Weisbecker
2011-11-01 23:46   ` [PATCH 10/10] cgroup: kill subsys->can_attach_task(), pre_attach() and attach_task() Tejun Heo
     [not found]     ` <1320191193-8110-11-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2011-11-04  9:10       ` KAMEZAWA Hiroyuki
2011-11-15  0:54       ` Frederic Weisbecker
2011-11-21 22:07   ` [PATCHSET] cgroup: stable threadgroup during attach & subsys methods consolidation Tejun Heo
     [not found]     ` <20111121220719.GP25776-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2011-11-22  2:27       ` Li Zefan
     [not found]         ` <4ECB089C.3080208-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
2011-11-22 16:20           ` Tejun Heo
2011-11-24 22:51   ` Tejun Heo

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=1320191193-8110-7-git-send-email-tj@kernel.org \
    --to=tj-dgejt+ai2ygdnm+yrofe0a@public.gmane.org \
    --cc=akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org \
    --cc=containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org \
    --cc=fweisbec-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-pm-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org \
    --cc=lizf-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org \
    --cc=oleg-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
    --cc=paul-inf54ven1CmVyaH7bEyXVA@public.gmane.org \
    --cc=rjw-KKrjLPT3xs0@public.gmane.org \
    /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