Re: [RFC/PATCH 0/4] CPUSET driven CPU isolation

David Rientjes
2008-02-28T21:25:48Z

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