On Wed, Jan 30, 2019 at 02:06:20PM +0100, Peter Zijlstra wrote: > On Wed, Jan 30, 2019 at 02:04:10PM +0100, Peter Zijlstra wrote:
> > So I don't much like this; at all. But maybe I misunderstand, this is > > somewhat tricky stuff and I've not looked at it in a while. > > > > So per normal we do: > > > > enqueue_task_fair() > > for_each_sched_entity() { > > if (se->on_rq) > > break; > > enqueue_entity() > > list_add_leaf_cfs_rq(); > > } > > > > This ensures that all parents are already enqueued, right? because this > > is what enqueues those parents. > > > > And in this case you add an unconditional second > > for_each_sched_entity(); even though it is completely redundant, afaict. > > Ah, it doesn't do a second iteration; it continues where the previous > two left off. > > Still, why isn't this in unthrottle? Aah, I see, because we need: rq->tmp_alone_branch == &rq->lead_cfs_rq_list at the end of enqueue_task_fair(); having had that assertion would've saved some pain I suppose.