cgroups.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC][PATCH] cgroup: disallow subtree controller enable/disable while there are tasks
@ 2015-05-04 10:11 Zefan Li
       [not found] ` <554745D1.1000206-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
  0 siblings, 1 reply; 2+ messages in thread
From: Zefan Li @ 2015-05-04 10:11 UTC (permalink / raw)
  To: Tejun Heo; +Cc: Mike Galbraith, LKML, Cgroups

It's not safe if we enable/disable some controllers while there are
tasks in the substree, because task migration can fail.

For example rt bandwidth controller disallow having rt tasks in the
cgroup if no bandwidth has been allocated, which is the case when
a cgroup is just created.

A kernel warning is triggered immediately by doing:

  # mkdir child
  # <launch an rt task>
  # echo PID > child/cgroup.procs
  # echo '+cpu' > cgroup.subtree_control
  -bash: echo: write error: Invalid argument

------------[ cut here ]------------
WARNING: CPU: 0 PID: 4067 at kernel/cgroup.c:2677 cgroup_update_dfl_csses+0x2aa/0x3a0()
cgroup: failed to update controllers for the default hierarchy (-22), further operations may crash or hang


Disable this until we eliminate all ss->can_attach callbacks.

Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Fixes: f8f22e53a262 ("cgroup: implement dynamic subtree controller enable/disable on the default hierarchy")
Signed-off-by: Zefan Li <lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
---
 kernel/cgroup.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 29a7b2c..fdd5393 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -2840,6 +2840,20 @@ static ssize_t cgroup_subtree_control_write(struct kernfs_open_file *of,
 	}
 
 	/*
+	 * FIXME: currently we can't enable/disable controllers while
+	 * there are tasks in the subtree, because task migration
+	 * can fail.
+	 */
+	cgroup_for_each_live_child(child, cgrp) {
+		if (cgroup_has_tasks(child)) {
+			ret = -EBUSY;
+			goto err_undo_css;
+		}
+	}
+
+	/*
+	 * FIXME: this won't do anything because of the above check.
+	 *
 	 * At this point, cgroup_e_css() results reflect the new csses
 	 * making the following cgroup_update_dfl_csses() properly update
 	 * css associations of all tasks in the subtree.
-- 
1.8.0.2

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

* Re: [RFC][PATCH] cgroup: disallow subtree controller enable/disable while there are tasks
       [not found] ` <554745D1.1000206-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
@ 2015-05-04 16:15   ` Tejun Heo
  0 siblings, 0 replies; 2+ messages in thread
From: Tejun Heo @ 2015-05-04 16:15 UTC (permalink / raw)
  To: Zefan Li; +Cc: Mike Galbraith, LKML, Cgroups

On Mon, May 04, 2015 at 06:11:29PM +0800, Zefan Li wrote:
> It's not safe if we enable/disable some controllers while there are
> tasks in the substree, because task migration can fail.
> 
> For example rt bandwidth controller disallow having rt tasks in the
> cgroup if no bandwidth has been allocated, which is the case when
> a cgroup is just created.
> 
> A kernel warning is triggered immediately by doing:

Well, the whole thing is behind the devel flag for a reason.  Let's
see how we can resolve the RT case and then determine what to do.

Thanks.

-- 
tejun

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

end of thread, other threads:[~2015-05-04 16:15 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-05-04 10:11 [RFC][PATCH] cgroup: disallow subtree controller enable/disable while there are tasks Zefan Li
     [not found] ` <554745D1.1000206-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
2015-05-04 16:15   ` Tejun Heo

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).