Messages in this thread | | | Date | Tue, 4 Jan 2011 15:28:05 +0100 | From | Oleg Nesterov <> | Subject | Re: [RFC][PATCH 16/17] sched: Move the second half of ttwu() to the remote cpu |
| |
On 12/24, Peter Zijlstra wrote: > > +static void > +ttwu_do_activate(struct rq *rq, struct task_struct *p, int wake_flags) > +{ > +#ifdef CONFIG_SMP > + if (task_cpu(p) != cpu_of(rq)) > + set_task_cpu(p, cpu_of(rq)); > +#endif
This looks a bit suspicious.
If this is called by sched_ttwu_pending() we are holding rq->lock, not task_rq_lock(). It seems, we can race with, say, migration thread running on task_cpu().
OK, p->state = TASK_WAKING protects us against, say, set_cpus_allowed_ptr() which does task_rq_lock(p) and thus checks task_is_waking().
But, at the same time,
> +#ifdef CONFIG_SMP > +static void ttwu_queue_remote(struct task_struct *p, int cpu) > +{ > + struct task_struct *next = NULL; > + struct rq *rq = cpu_rq(cpu); > + > + for (;;) { > + struct task_struct *old = next; > + > + p->wake_entry = next; > + next = cmpxchg(&rq->wake_list, old, p); > + if (next == old) > + break; > + } > + > + if (!next) > + smp_send_reschedule(cpu);
what if that cpu does set_cpus_allowed_ptr(p) ?
It spins with irq disabled. Once the caller, try_to_wake_up(), drops ->pi_lock it will wait for !task_is_waking() forever, no?
Oleg.
| |