Peter Zijlstra <pet...@infradead.org> writes:
> static struct task_struct *
> pick_next_task_fair(struct rq *rq, struct task_struct *prev)
> {
> + struct sched_entity *se, __maybe_unused *pse;
> struct task_struct *p;
> - struct cfs_rq *cfs_rq = &rq->cfs;
> - struct sched_entity *se;
> + struct cfs_rq *cfs_rq;
> +
> +again: __maybe_unused
> + cfs_rq = &rq->cfs;
> +
> + if (prev) {
> + if (!IS_ENABLED(CONFIG_FAIR_GROUP_SCHED) ||
> + (prev->sched_class != &fair_sched_class)) {
> + prev->sched_class->put_prev_task(rq, prev);
> + prev = NULL;
> + }
> + }
>
> if (!cfs_rq->nr_running)
> return NULL;
>
> - if (prev)
> - prev->sched_class->put_prev_task(rq, prev);
> -
> do {
> se = pick_next_entity(cfs_rq);
> - set_next_entity(cfs_rq, se);
> + if (!prev)
> + set_next_entity(cfs_rq, se);
> cfs_rq = group_cfs_rq(se);
> } while (cfs_rq);
>
> p = task_of(se);
> - if (hrtick_enabled(rq))
> - hrtick_start_fair(rq, p);
>
> - return p;
> -}
> +#ifdef CONFIG_FAIR_GROUP_SCHED
> + /*
> + * If we haven't yet done put_prev_entity and the selected task is
> + * a different task than we started out with, try and touch the least
> + * amount of cfs_rq trees.
> + */
> + if (prev) {
> + if (prev != p) {
> + pse = &prev->se;
> +
> + while (!(cfs_rq = is_same_group(se, pse))) {
> + int se_depth = se->depth;
> + int pse_depth = pse->depth;
> +
> + if (se_depth <= pse_depth) {
> + put_prev_entity(cfs_rq_of(pse), pse);
> + pse = parent_entity(pse);
> + }
> + if (se_depth >= pse_depth) {
> + set_next_entity(cfs_rq_of(se), se);
> + se = parent_entity(se);
> + }
> + }
>
> -/*
> - * Account for a descheduled task:
> - */
> -static void put_prev_task_fair(struct rq *rq, struct task_struct *prev)
> -{
> - struct sched_entity *se = &prev->se;
> - struct cfs_rq *cfs_rq;
> + put_prev_entity(cfs_rq, pse);
> + set_next_entity(cfs_rq, se);
> + }
>
> - for_each_sched_entity(se) {
> - cfs_rq = cfs_rq_of(se);
> - put_prev_entity(cfs_rq, se);
> + /*
> + * In case the common cfs_rq got throttled, just give up and
> + * put the stack and retry.
> + */
> + if (unlikely(check_cfs_rq_runtime(cfs_rq))) {
> + put_prev_task_fair(rq, p);
> + prev = NULL;
> + goto again;
> + }
This double-calls put_prev_entity on any non-common cfs_rqs and ses,
which means double __enqueue_entity, among other things. Just doing the
put_prev loop from se->parent should fix that.
However, any sort of abort means that we may have already done
set_next_entity on some children, which even with the changes to
pick_next_entity will cause problems, up to and including double
__dequeue_entity I think.
Also, this way we never do check_cfs_rq_runtime on any parents of the
common cfs_rq, which could even have been the reason for the resched to
begin with. I'm not sure if there would be any problem doing it on the
way down or not, I don't see any problems at a glance.
> }
> +#endif
> +
> + if (hrtick_enabled(rq))
> + hrtick_start_fair(rq, p);
> +
> + return p;
> }
>
> /*
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
