Re: [patch] cpusets: allow PF_THREAD_BOUND kworkers to escape froma cpuset

[Mike Galbraith]

On Fri, 2011-09-23 at 11:42 +0200, Mike Galbraith wrote:
> On Fri, 2011-09-23 at 02:12 -0700, David Rientjes wrote:
> > On Fri, 23 Sep 2011, Mike Galbraith wrote:
> > 
> > > Done, your ACK added as well.
> > > 
> > > kworkers can be born in a cpuset, leaving them adrift on an unsinkable ship.
> > > Allow them to be moved to the root cpuset so the cpuset can be destroyed.
> > > 
> > 
> > Thanks Li for the cc since I introduced this flag.
> > 
> > Does this even work?
> 
> Yup, but..
> 
> > You've modified cpuset_can_attach() to allow the attach for the cgroup 
> > interface, but the actual function that attaches the task in the cpuset 
> > code should fail since it does set_cpus_allowed_ptr() which should return 
> > -EINVAL because of the PF_THREAD_BOUND.  That should have emitted a 
> > WARN_ON() if this patch was tested.
> 
> ..you're right that it warns.  It was tested, but I didn't notice that
> it had griped at me.
> 
> > kworkers can always move themselves to the root cpuset, so that's probably 
> > the better way of handling this than requiring userspace to do so.
> 
> kworker is sleeping in a cpuset that the user wants to depopulate and
> destroy.  I wasn't requiring the user to do that, was allowing him to.

So unless there's some reason kworker threads should worry about their
birthplace, seems to me we should just let the cpuset interface handle
the oddball case.

cpusets: allow PF_THREAD_BOUND kworkers to escape from a cpuset

kworkers can be born in a cpuset, leaving them adrift on an unsinkable ship.
Allow them to be moved to the root cpuset so the cpuset can be destroyed

Signed-off-by: Mike Galbraith <efault@xxxxxx>
Acked-by: Li Zefan <lizf@xxxxxxxxxxxxxx>

diff --git a/kernel/cpuset.c b/kernel/cpuset.c
index 10131fd..3769f9e 100644
--- a/kernel/cpuset.c
+++ b/kernel/cpuset.c
@@ -1384,7 +1384,7 @@ static int cpuset_can_attach(struct cgroup_subsys *ss, struct cgroup *cont,
 	 * set_cpus_allowed_ptr() on all attached tasks before cpus_allowed may
 	 * be changed.
 	 */
-	if (tsk->flags & PF_THREAD_BOUND)
+	if (tsk->flags & PF_THREAD_BOUND && cont != cont->top_cgroup)
 		return -EINVAL;
 
 	return 0;
@@ -1426,9 +1426,14 @@ static void cpuset_attach_task(struct cgroup *cont, struct task_struct *tsk)
 	/*
 	 * can_attach beforehand should guarantee that this doesn't fail.
 	 * TODO: have a better way to handle failure here
+	 * 
+	 * Special case: bound kthreads born in a cpuset may be moved to
+	 * the top level cpuset without attempting to diddle their mask.
 	 */
-	err = set_cpus_allowed_ptr(tsk, cpus_attach);
-	WARN_ON_ONCE(err);
+	if (!(tsk->flags & PF_THREAD_BOUND && cont == cont->top_cgroup)) {
+		err = set_cpus_allowed_ptr(tsk, cpus_attach);
+		WARN_ON_ONCE(err);
+	}
 
 	cpuset_change_task_nodemask(tsk, &cpuset_attach_nodemask_to);
 	cpuset_update_task_spread_flag(cs, tsk);


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/