Skip to content

Commit

Permalink
sched/fair: Fix fairness issue on migration
Browse files Browse the repository at this point in the history
Pavan reported that in the presence of very light tasks (or cgroups)
the placement of migrated tasks can cause severe fairness issues.

The problem is that enqueue_entity() places the task before it updates
time, thereby it can place the task far in the past (remember that
light tasks will shoot virtual time forward at a high speed, so in
relation to the pre-existing light task, we can land far in the past).

This is done because update_curr() needs the current task, and we
might be placing the current task.

The obvious solution is to differentiate between the current and any
other task; placing the current before we update time, and placing any
other task after, such that !curr tasks end up at the current moment
in time, and not in the past.

Reported-by: Pavan Kondeti <pkondeti@codeaurora.org>
Tested-by: Pavan Kondeti <pkondeti@codeaurora.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Ben Segall <bsegall@google.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Mike Galbraith <umgwanakikbuti@gmail.com>
Cc: Morten Rasmussen <morten.rasmussen@arm.com>
Cc: Paul Turner <pjt@google.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: byungchul.park@lge.com
Link: http://lkml.kernel.org/r/20160309120403.GK6344@twins.programming.kicks-ass.net
Signed-off-by: Ingo Molnar <mingo@kernel.org>
  • Loading branch information
Peter Zijlstra authored and Ingo Molnar committed Mar 21, 2016
1 parent 2f5177f commit 3a47d51
Showing 1 changed file with 14 additions and 6 deletions.
20 changes: 14 additions & 6 deletions kernel/sched/fair.c
Original file line number Diff line number Diff line change
Expand Up @@ -3157,17 +3157,25 @@ static inline void check_schedstat_required(void)
static void
enqueue_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
{
bool renorm = !(flags & ENQUEUE_WAKEUP) || (flags & ENQUEUE_WAKING);
bool curr = cfs_rq->curr == se;

/*
* Update the normalized vruntime before updating min_vruntime
* through calling update_curr().
* If we're the current task, we must renormalise before calling
* update_curr().
*/
if (!(flags & ENQUEUE_WAKEUP) || (flags & ENQUEUE_WAKING))
if (renorm && curr)
se->vruntime += cfs_rq->min_vruntime;

update_curr(cfs_rq);

/*
* Update run-time statistics of the 'current'.
* Otherwise, renormalise after, such that we're placed at the current
* moment in time, instead of some random moment in the past.
*/
update_curr(cfs_rq);
if (renorm && !curr)
se->vruntime += cfs_rq->min_vruntime;

enqueue_entity_load_avg(cfs_rq, se);
account_entity_enqueue(cfs_rq, se);
update_cfs_shares(cfs_rq);
Expand All @@ -3183,7 +3191,7 @@ enqueue_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
update_stats_enqueue(cfs_rq, se);
check_spread(cfs_rq, se);
}
if (se != cfs_rq->curr)
if (!curr)
__enqueue_entity(cfs_rq, se);
se->on_rq = 1;

Expand Down

0 comments on commit 3a47d51

Please sign in to comment.