Re: [patch] sched: fix b5d9d734 blunder in task_new_fair()

Peter Zijlstra
2009-11-25T09:51:58Z

On Wed, 2009-11-25 at 07:57 +0100, Mike Galbraith wrote:

> update_curr(cfs_rq);
> +
> + if (is_same_group(se, pse)) {
> + se->vruntime = pse->vruntime;
> +
> + /*
> + * If we're not sharing a runqueue, redo the child's vruntime
> + * offset after accounting for any yet to be booked vruntime.
> + */
> + if (this_cpu != task_cpu(p)) {
> + struct cfs_rq *old_cfs_rq = cfs_rq_of(pse);
> + u64 now = cpu_rq(this_cpu)->clock;
> + unsigned long delta_exec = now - pse->exec_start;
> +
> + delta_exec = calc_delta_fair(delta_exec, se);
> + se->vruntime += delta_exec;
> + se->vruntime -= old_cfs_rq->min_vruntime -
> + cfs_rq->min_vruntime;
> + }
> + }
> +
> place_entity(cfs_rq, se, 1);

/me got his head in a twist..

- is_same_group() assumes both things are on the same cpu and will
fail (for the group configs) when this is not so.

- if we're not on the same cpu, update_curr() will have updated current
on the target cpu, but the parent vruntime will still be stale.

- when all is said and done, place_entity(.initial=1) will do:
->vruntime = max(min_vruntime + debit, ->vruntime)
which ought to place a new task far enough ahead in any case...