lkml.org 
[lkml]   [2008]   [Jul]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] cpu hotplug, sched:Introduce cpu_active_map and redoscheddomainmanagment (take 2)
Gregory Haskins wrote:
> Max Krasnyansky wrote:
>> Greg, correct me if I'm wrong but we seem to have exact same issue
>> with the rq->rq->online map. Lets take "cpu going down" for
>> example. We're clearing rq->rd->online bit on DYING event, but
>> nothing AFAICS prevents another cpu calling
>> rebuild_sched_domains()->partition_sched_domains() in the middle of
>> the hotplug sequence. partition_sched_domains() will happily reset
>> rd->rq->online mask and things will fail. I'm talking about this
>> path
>>
>> __build_sched_domains() -> cpu_attach_domain() -> rq_attach_root()
>> ...
>> cpu_set(rq->cpu, rd->span);
>> if (cpu_isset(rq->cpu, cpu_online_map))
>> set_rq_online(rq);
>> ...
>>
>>
>
> I think you are right, but wouldn't s/online/active above fix that as
> well? The active_map didnt exist at the time that code went in
> initially ;)

Actually after a bit more thinking :) I realized that the scenario I
explained above cannot happen because partition_sched_domains() must be
called under get_online_cpus() and the set_rq_online() happens in the
hotplug writer's path (ie under cpu_hotplug.lock). Since I unified all
the other domain rebuild paths (arch_reinit_sched_domains, etc) we
should be safe. But it again means we'd rely on those intricate
dependencies that we wanted to avoid with the cpu_active_map. Also
cpusets might still need to rebuild the domains in the hotplug writer's
path.
So it's better to fix it once and for all :)

>> --
>>
>> btw Why didn't we convert sched*.c to use rq->rd->online when it was
>> introduced ? ie Instead of using cpu_online_map directly.
>>
> I think things were converted where they made sense to convert. But we
> also had a different goal at that time, so perhaps something was
> missed. If you think something else should be converted, please point
> it out.
Ok. I'll keep an eye on it.

> In the meantime, I would suggest we consider this patch on top of yours
> (applies to tip/sched/devel):
>
> ----------------------
>
> sched: Fully integrate cpus_active_map and root-domain code
> Signed-off-by: Gregory Haskins <ghaskins@novell.com>

Ack.
The only thing I'm a bit unsure of is the error scenarios in the cpu
hotplug event sequence. online_map is not cleared when something in the
notifier chain fails, but active_map is.

Max


\
 
 \ /
  Last update: 2008-07-22 21:35    [W:0.143 / U:2.424 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site