On Thu, 28 Feb 2008, Paul Jackson wrote:

> 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?

> 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?

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

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