Messages in this thread | | | Date | Tue, 1 Sep 2015 17:03:43 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH v4 5/5] sched: add two functions for att(det)aching a task to(from) a cfs_rq |
| |
On Tue, Sep 01, 2015 at 09:28:49AM +0900, Byungchul Park wrote:
> check the condition "!(flags & DEQUEUE_SLEEP)" for doing normalizing in > dequeue_entity(). i think you have to keep my original comment, or > modify your comment to something like below. > > before - If it's !queued, sleeping tasks have a normalized vruntime, > after - If it's !queued, sleeping tasks have a non-normalize vruntime, > > but.. i think it would be better that you keep my original comment..
The comment we can talk about later, but I think the condition:
> > - if (p->state == TASK_RUNNING) > > + if (!p->se.on_rq)
is important now. Both are broken in different ways.
p->state == TASK_RUNNING
is broken in this scenario:
CPU0 CPU1
set_current_state(TASK_UNINTERRUPTIBLE);
sched_move_task() task_move_group_fair() vruntime_normalized() == true if (!cond) schedule(); __set_current_state(TASK_RUNNING);
Now the proposed replacement:
!p->se.on_rq
is equally broken, because (as you point out) clearing it isn't conditional on DEQUEUE_SLEEP.
And the problem with tracking the vruntime state is that while it helps detach_task_cfs_rq(), attach_task_cfs_rq() is still left wondering what it should return to.
So we do indeed need something to determine, based on the current state, if vruntime should be normalized.
/me ponders moar
| |