Re: [RFC -v7 PATCH 3/7] sched: use a buddy to implement yield_task_fair

Peter Zijlstra
2011-01-31T11:46:45Z

On Wed, 2011-01-26 at 17:21 -0500, Rik van Riel wrote:

> +static struct sched_entity *__pick_second_entity(struct cfs_rq *cfs_rq)
> +{
> + struct rb_node *left = cfs_rq->rb_leftmost;
> + struct rb_node *second;
> +
> + if (!left)
> + return NULL;
> +
> + second = rb_next(left);
> +
> + if (!second)
> + second = left;
> +
> + return rb_entry(second, struct sched_entity, run_node);
> +}

I would still prefer:

sed -i 's/__pick_next_entity/__pick_first_entity/g' kernel/sched*.[ch]

and

static struct sched_entity *__pick_next_entity(struct sched_entity *se)
{
struct rb_node *next = rb_next(&se->run_node);

if (!next)
return NULL;

return rb_entry(next, struct sched_entity, run_node);
}

Its simpler and more versatile.

> static struct sched_entity *pick_next_entity(struct cfs_rq *cfs_rq)
> {
> struct sched_entity *se = __pick_next_entity(cfs_rq);
> struct sched_entity *left = se;
>
> - if (cfs_rq->next && wakeup_preempt_entity(cfs_rq->next, left) < 1)
> - se = cfs_rq->next;
> + /*
> + * Avoid running the skip buddy, if running something else can
> + * be done without getting too unfair.
> + */
> + if (cfs_rq->skip == se) {
> + struct sched_entity *second = __pick_second_entity(cfs_rq);

which then becomes *next = __pick_next_entity(se);

> + if (wakeup_preempt_entity(second, left) < 1)

oops when !second. (!next)

> + se = second;
> + }

> -/*
> - * sched_yield() support is very simple - we dequeue and enqueue.
> - *
> - * If compat_yield is turned on then we requeue to the end of the tree.
> - */
> -static void yield_task_fair(struct rq *rq)
> -{
> - struct task_struct *curr = rq->curr;
> - struct cfs_rq *cfs_rq = task_cfs_rq(curr);
> - struct sched_entity *rightmost, *se = &curr->se;
> -
> - /*
> - * Are we the only task in the tree?
> - */
> - if (unlikely(rq->nr_running == 1))
> - return;
> -
> - clear_buddies(cfs_rq, se);
> -
> - if (likely(!sysctl_sched_compat_yield) && curr->policy != SCHED_BATCH) {
> - update_rq_clock(rq);
> - /*
> - * Update run-time statistics of the 'current'.
> - */
> - update_curr(cfs_rq);
> -
> - return;
> - }
> - /*
> - * Find the rightmost entry in the rbtree:
> - */
> - rightmost = __pick_last_entity(cfs_rq);
> - /*
> - * Already in the rightmost position?
> - */
> - if (unlikely(!rightmost || entity_before(rightmost, se)))
> - return;
> -
> - /*
> - * Minimally necessary key value to be last in the tree:
> - * Upon rescheduling, sched_class::put_prev_task() will place
> - * 'current' within the tree based on its new key value.
> - */
> - se->vruntime = rightmost->vruntime + 1;
> -}

You seem to have lost the hunk removing sysctl_sched_compat_yield from
kernel/sysctl.c :-)