Re: [Lse-tech] Re: [PATCH] cpusets+hotplug+preepmt broken

Srivatsa Vaddagiri
2005-05-14T17:23:48Z

On Sat, May 14, 2005 at 11:30:12AM -0500, Nathan Lynch wrote:
> I suspect that the lock_cpu_hotplug is no longer necessary in
> sched_setaffinity.

Digging harder into my memory, I think the primary reason why lock_cpu_hotplug
was added in sched_setaffinity was this: some code wants to temporarily
override a (user-space) task's cpu mask. Before stop_machine came along, module
code was doing that (again if my memory serves me right). With stop_machine,
temporary changing of user-space (rmmod's) cpus_allowed is not reqd.
However I still see other code (like acpi_processor_set_performance) which
does something like this:

saved_mask = current->cpus_allowed;

set_cpus_allowed(current, new_mask);

/* Do something ..Could block */

set_cpus_allowed(current, saved_mask);

How do you ensure that saved_mask has not changed since the time
we took a snapshot of it (bcoz of sched_setaffinity having
changed it)? lock_cpu_hotplug could serialize such pieces of code

> I found the original changeset which introduced
> it, and it's short enough that I'll just duplicate it here:

[snip]

> The lock/unlock_cpu_hotplug is no longer there in sched_migrate_task.

Essentialy, if I remember correct, sched_migrate_task was also doing
something like acpi_processor_set_performance to migrate a task to a new cpu.
Thats why we discussed that sched_migrate_task should take lock_cpu_hotplug.
But I don't think it ever went in. There were concerns of cache-line bounces
on NUMA m/c bcoz of lock_cpu_hotplug in a frequently executed code like
sched_migrate_task. Moreover sched_migrate_task no longer behaves like
acpi_processor_set_performance when it wants to migrate a user-space task to a
different CPU. It now seems to take the help of migration thread (which is
much neater compared to what was there earlier i guess).

> The changelog leads me to believe that it was intended that the same
> change should have been made to sched_setaffinity by now. I think
> it's safe to remove it; I can't see why it would be necessary any
> more.

I recommend that we keep the lock_cpu_hotplug in sched_affinity
unless we figure out a different way of solving the race I highlighted above
or we ban code like that in acpi_processor_set_performance :)

--

Thanks and Regards,
Srivatsa Vaddagiri,
Linux Technology Center,
IBM Software Labs,
Bangalore, INDIA - 560017