Messages in this thread | | | Date | Tue, 22 Jul 2014 08:25:10 -0400 | From | Steven Rostedt <> | Subject | Re: [PATCH 2/5] sched: Teach scheduler to understand ONRQ_MIGRATING state |
| |
On Tue, 22 Jul 2014 13:45:42 +0200 Peter Zijlstra <peterz@infradead.org> wrote:
> > @@ -1491,10 +1491,14 @@ static void ttwu_activate(struct rq *rq, struct task_struct *p, int en_flags) > > static void > > ttwu_do_wakeup(struct rq *rq, struct task_struct *p, int wake_flags) > > { > > - check_preempt_curr(rq, p, wake_flags); > > trace_sched_wakeup(p, true); > > > > p->state = TASK_RUNNING; > > + > > + if (!task_queued(p)) > > + return; > > How can this happen? we're in the middle of a wakeup, we're just added > the task to the rq and are still holding the appropriate rq->lock.
I believe it can be in the migrating state. A comment would be useful here.
> > > @@ -4623,9 +4629,14 @@ int set_cpus_allowed_ptr(struct task_struct *p, const struct cpumask *new_mask) > > struct rq *rq; > > unsigned int dest_cpu; > > int ret = 0; > > - > > +again: > > rq = task_rq_lock(p, &flags); > > > > + if (unlikely(p->on_rq) == ONRQ_MIGRATING) { > > + task_rq_unlock(rq, p, &flags); > > + goto again; > > + } > > + > > if (cpumask_equal(&p->cpus_allowed, new_mask)) > > goto out; > > > > That looks like a non-deterministic spin loop, 'waiting' for the > migration to finish. Not particularly nice and something I think we > should avoid for it has bad (TM) worst case behaviour.
As this patch doesn't introduce the MIGRATING getting set yet, I'd be interested in this too. I'm assuming that the MIGRATING flag is only set and then cleared within an interrupts disabled section, such that the time is no more than a spinlock being taken.
I would also add a cpu_relax() there too.
> > Also, why only this site and not all task_rq_lock() sites?
I'm assuming that its because set_cpus_allowed_ptr() is suppose to return with the task already migrated to the CPUs it is allowed on, and not before.
-- Steve
| |