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.
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;
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.
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.
--
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <pj@sgi.com> 1.940.382.4214
--
