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

[David Rientjes <rientjes@google.com>]

On Fri, 23 Sep 2011, Mike Galbraith wrote:

> I don't see why it's a kworker code problem.  The kworker code couldn't
> care less about cpusets.  But I don't care who fixes it or how.
> 

The problem is that PF_THREAD_BOUND means that nothing other than the task 
itself may change the set of allowed cpus to be different.  For this 
reason, they have always been declined to be moved amongst cpusets because 
even though their bound cpu may be a subset of the cpuset's cpumask at the 
time of attach, that cpuset mask may subsequently change and therefore try 
to force the PF_THREAD_BOUND off the cpu it is bound to resulting in an 
inconsistency between the cpuset's mask and an attached task's mask.

Allowing a PF_THREAD_BOUND thread to move to the root cpuset isn't so much 
of a problem because it's guaranteed to never exclude the bound cpu, but 
the problem already exists if the kworker is in the child cpuset and 
userspace attempts to change the mems of that cpuset to be disjoint.  That 
results in an inconsistency because the scheduler will refuse it.

> cpusets: disallow moving kthreadd into a cpuset.
> 
> If kthreadd is moved into a cpuset, it's child may then acquire
> PF_THREAD_BOUND and become trapped, making it's cpuset immortal.
> 
> Signed-off-by: Mike Galbraith <efault@gmx.de>
> 
> diff --git a/kernel/cpuset.c b/kernel/cpuset.c
> index 10131fd..0d9cd04 100644
> --- a/kernel/cpuset.c
> +++ b/kernel/cpuset.c
> @@ -59,6 +59,7 @@
>  #include <linux/mutex.h>
>  #include <linux/workqueue.h>
>  #include <linux/cgroup.h>
> +#include <linux/kthread.h>
>  
>  /*
>   * Workqueue for cpuset related tasks.
> @@ -1382,9 +1383,10 @@ static int cpuset_can_attach(struct cgroup_subsys *ss, struct cgroup *cont,
>  	 * set of allowed nodes is unnecessary.  Thus, cpusets are not
>  	 * applicable for such threads.  This prevents checking for success of
>  	 * set_cpus_allowed_ptr() on all attached tasks before cpus_allowed may
> -	 * be changed.
> +	 * be changed.  We also disallow attaching kthreadd, to prevent it's
> +	 * child from becoming trapped should it then acquire PF_THREAD_BOUND.
>  	 */
> -	if (tsk->flags & PF_THREAD_BOUND)
> +	if (tsk->flags & PF_THREAD_BOUND || tsk == kthreadd_task)
>  		return -EINVAL;
>  
>  	return 0;

I like this much better, let's wait to hear from Tejun because he may 
shead some light on whether PF_THREAD_BOUND is really necessary for 
kworkers at all times.

Thanks.