Re: [PATCH 1/1] hotplug cpu: migrate a task within its cpuset

[Nathan Lynch <ntl@pobox.com>]

Robin Holt wrote:
> On Fri, Mar 09, 2007 at 05:58:59PM -0600, Nathan Lynch wrote:
> > Hello-
> > 
> > Cliff Wickman wrote:
> > > This patch would insert a preference to migrate such a task to a cpu within
> > > its cpuset (and set its cpus_allowed to its cpuset).
> > > 
> > > With this patch, migrate the task to:
> > >  1) to any cpu on the same node as the disabled cpu, which is both online
> > >     and among that task's cpus_allowed
> > >  2) to any online cpu within the task's cpuset
> > >  3) to any cpu which is both online and among that task's cpus_allowed
> > 
> > I think I disagree with this change.
> > 
> > The kernel shouldn't have to be any smarter than it already is about
> > moving tasks off an offlined cpu.  The only way case 2) can be reached
> > is if the user has changed a task's cpu affinity.  If the user is
> > sophisticated enough to manipulate tasks' cpu affinity then they can
> > arrange to migrate tasks as they see fit before offlining a cpu.
> 
> You are assuming some sort of interlock between the admin and the user.

Oh, the horror!  ;-)

> While this may be true on your own personal desktop, I don't think you
> can expect this to be true on a development machine shared by hundreds
> of users and admin'd by a group of people.

Actually, I would hope that a large development environment where
users are given fine-grained control over their resource usage would
1) allow interested tasks to register for notification before a
resource is taken away, and 2) that this system of registration and
notification is implemented in userspace.  Use d-bus or something.

> Additionally, ia64 is gaining support for offlining a cpu which is giving
> cache errors.

Okay.  Consider the case where a task's cpuset consists only of threads
or cores that share cache.  Keeping the task in its cpuset when
offlining due to cache errors is exactly the wrong thing to do.

Anyway, it looks to me like task->cpus_allowed should already reflect
task's cpuset (cpuset.c:attach_task calls sched.c:set_cpus_allowed),
so now I'm missing the point of the patch.