On Thu, 28 Feb 2008, Paul Jackson wrote:

quoted text
> I don't have strong opinions either way on this patch; it adds an error
> check that makes sense.  I haven't seen much problem not having this check,
> nor do I know of any code that depends on doing what this check prohibits.
> 

How about moving watchdog/0 to a cpuset with exclusive access to only cpu 
1?

quoted text
> Except for three details:
> 
>  1) +	if (unlikely((p->flags & PF_CPU_BOUND) && p != current &&
>     +	    	     !cpus_equal(p->cpus_allowed, new_mask))) {
>     +		ret = -EINVAL;
> 
>     The check for equal cpus allowed seems too strong.  Shouldn't you be
>     checking that all of task p's cpus_allowed would still be allowed in
>     the new mask:
> 
>     +	if (unlikely((p->flags & PF_CPU_BOUND) && p != current &&
>     +	    	     !cpus_subset(p->cpus_allowed, new_mask))) {
>     +		ret = -EINVAL;
> 

That's a convenient way for a kthread to temporarily expand its set of 
cpus_allowed and then never be able to remove the added cpus again.  Do 
you have any examples in the tree where a kthread does this?

quoted text
>  2) Doesn't this leave out a check for the flip side -- shrinking
>     the cpus allowed by a cpuset so that it no longer contains those
>     required by any PF_CPU_BOUND tasks in that cpuset?  I'm not sure
>     if this second check is a good idea or not.
> 

That's why the check in set_cpus_allowed() is

	cpus_equal(p->cpus_allowed, newmask)

since it prevents PF_CPU_BOUND tasks from being moved out of the root 
cpuset.

quoted text
>  3) Could we call this PF_CPU_PINNED instead?  I tend to use "cpu
>     bound" to refer to tasks that consume alot of CPU cycles (which
>     these pinned tasks rarely do), and "pinned" to refer to what is
>     done to confine a task to a particular subset of all possible CPUs.
>     It looks to me like some code in kernel/sched.c already uses the
>     word pinned in this same way, so PF_CPU_PINNED would be more
>     consistent terminology.
> 

PF_CPU_BOUND follows the nomenclature of kthread_bind() really well, but 
it could probably be confused with a processor-bound task.  So perhaps 
PF_BOUND_CPU is even better?

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