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

[Peter Zijlstra]

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...



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/