Messages in this thread | | | Subject | Re: [PATCH] cpu hotplug, sched:Introduce cpu_active_map and redoscheddomainmanagment (take 2) | From | Peter Zijlstra <> | Date | Tue, 22 Jul 2008 16:16:00 +0200 |
| |
On Tue, 2008-07-22 at 10:06 -0400, 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 ;) > > > -- > > > > 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. > > 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>
Right, makes sense.
ACK
> diff --git a/kernel/sched.c b/kernel/sched.c > index 62b1b8e..99ba70d 100644 > --- a/kernel/sched.c > +++ b/kernel/sched.c > @@ -6611,7 +6611,7 @@ static void rq_attach_root(struct rq *rq, struct > root_domain *rd) > rq->rd = rd; > > cpu_set(rq->cpu, rd->span); > - if (cpu_isset(rq->cpu, cpu_online_map)) > + if (cpu_isset(rq->cpu, cpu_active_map)) > set_rq_online(rq); > > spin_unlock_irqrestore(&rq->lock, flags); > diff --git a/kernel/sched_fair.c b/kernel/sched_fair.c > index 7f70026..2bae8de 100644 > --- a/kernel/sched_fair.c > +++ b/kernel/sched_fair.c > @@ -1004,7 +1004,7 @@ static void yield_task_fair(struct rq *rq) > * search starts with cpus closest then further out as needed, > * so we always favor a closer, idle cpu. > * Domains may include CPUs that are not usable for migration, > - * hence we need to mask them out (cpu_active_map) > + * hence we need to mask them out (rq->rd->online) > * > * Returns the CPU we should wake onto. > */ > @@ -1032,7 +1032,7 @@ static int wake_idle(int cpu, struct task_struct *p) > || ((sd->flags & SD_WAKE_IDLE_FAR) > && !task_hot(p, task_rq(p)->clock, sd))) { > cpus_and(tmp, sd->span, p->cpus_allowed); > - cpus_and(tmp, tmp, cpu_active_map); > + cpus_and(tmp, tmp, task_rq(p)->rd->online); > for_each_cpu_mask(i, tmp) { > if (idle_cpu(i)) { > if (i != task_cpu(p)) { > diff --git a/kernel/sched_rt.c b/kernel/sched_rt.c > index 24621ce..d93169d 100644 > --- a/kernel/sched_rt.c > +++ b/kernel/sched_rt.c > @@ -936,13 +936,6 @@ static int find_lowest_rq(struct task_struct *task) > return -1; /* No targets found */ > > /* > - * Only consider CPUs that are usable for migration. > - * I guess we might want to change cpupri_find() to ignore those > - * in the first place. > - */ > - cpus_and(*lowest_mask, *lowest_mask, cpu_active_map); > - > - /* > * At this point we have built a mask of cpus representing the > * lowest priority tasks in the system. Now we want to elect > * the best one based on our affinity and topology. > > -------------- > > Regards, > -Greg >
| |