Messages in this thread | | | Subject | Re: [RFC][PATCH 16/17] sched: Move the second half of ttwu() to the remote cpu | From | Peter Zijlstra <> | Date | Tue, 04 Jan 2011 15:47:21 +0100 |
| |
On Tue, 2011-01-04 at 15:28 +0100, Oleg Nesterov wrote: > 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().
I don't think so, nobody should be migrating a TASK_WAKING task.
> 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?
Ah, it appears I've already fixed that, let me clean up my current series and repost.
| |