Messages in this thread | | | Date | Fri, 29 May 2015 15:16:26 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH v3 1/4] sched/rt: Check to push the task away after its affinity was changed |
| |
On Tue, May 12, 2015 at 10:46:41PM +0800, Xunlei Pang wrote: > @@ -2278,6 +2279,20 @@ static void set_cpus_allowed_rt(struct task_struct *p, > } > > update_rt_migration(&rq->rt); > + > +check_push: > + if (weight > 1 && > + !task_running(rq, p) && > + !test_tsk_need_resched(rq->curr) && > + !cpumask_subset(new_mask, &p->cpus_allowed)) { > + /* Update new affinity and try to push. */ > + cpumask_copy(&p->cpus_allowed, new_mask); > + p->nr_cpus_allowed = weight; > + push_rt_tasks(rq); > + return true; > + } > + > + return false; > }
I think this is broken; push_rt_tasks() will do double_rq_lock() which will drop rq->lock.
This means load-balancing can come in and move our task p; in fact, push_rt_task() can do exactly that -- after all that was the point of this patch.
_However_ this means that after calling ->set_cpus_allowed() we must not assume @p is on @rt, yet we do. Look at __set_cpus_allowed_ptr(), we'll call move_queued_task() if (!running || waking) && on_rq, and move_queued_task() happily calls dequeue_task(rq, p), which will go *boom*.
I currently do not have a better idea than to repurpose the PUSH_IPI stuff, that is, send a self IPI to go do the push or somesuch. Lemme stare at this a little more.
| |