Since commit 2f36825b1 ("sched: Next buddy hint on sleep and preempt
path") it is likely we pick a new task from the same cgroup, doing a put
and then set on all intermediate entities is a waste of time, so try to
avoid this.
Signed-off-by: Peter Zijlstra <[email protected]>
---
kernel/sched/fair.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 68 insertions(+), 7 deletions(-)
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -1285,15 +1285,46 @@ wakeup_preempt_entity(struct sched_entit
*/
static struct sched_entity *pick_next_entity(struct cfs_rq *cfs_rq)
{
- struct sched_entity *se = __pick_first_entity(cfs_rq);
- struct sched_entity *left = se;
+ struct sched_entity *left = __pick_first_entity(cfs_rq);
+ struct sched_entity *se, *curr = cfs_rq->curr;
+
+ /*
+ * Since its possible we got here without doing put_prev_entity() we
+ * also have to consider cfs_rq->curr. If it was set, and is still a
+ * runnable entity, update_curr() will update its vruntime, otherwise
+ * forget we've ever seen it.
+ */
+ if (curr) {
+ if (curr->on_rq)
+ update_curr(cfs_rq);
+ else
+ curr = NULL;
+ }
+
+ /*
+ * If curr is set we have to see if its left of the leftmost entity
+ * still in the tree, provided there was anything in the tree at all.
+ */
+ if (!left || (curr && entity_before(curr, left)))
+ left = curr;
+
+ se = left; /* ideally we run the leftmost entity */
/*
* 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_next_entity(se);
+ struct sched_entity *second;
+
+ if (se == curr) {
+ second = __pick_first_entity(cfs_rq);
+ } else {
+ second = __pick_next_entity(se);
+ if (!second || (curr && entity_before(curr, second)))
+ second = curr;
+ }
+
if (second && wakeup_preempt_entity(second, left) < 1)
se = second;
}
@@ -2993,23 +3024,53 @@ static void check_preempt_wakeup(struct
static struct task_struct *
pick_next_task_fair(struct rq *rq, struct task_struct *prev)
{
- struct task_struct *p;
struct cfs_rq *cfs_rq = &rq->cfs;
- struct sched_entity *se;
+ struct sched_entity *se, *pse;
+ struct task_struct *p;
if (!cfs_rq->nr_running)
return NULL;
- if (prev)
+ if (prev && (prev->sched_class != &fair_sched_class)) {
prev->sched_class->put_prev_task(rq, prev);
+ prev = NULL;
+ }
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 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 && 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);
+ }
+ }
+
+ put_prev_entity(cfs_rq, pse);
+ set_next_entity(cfs_rq, se);
+ }
+
if (hrtick_enabled(rq))
hrtick_start_fair(rq, p);
On Thu, 2012-06-14 at 15:29 +0200, Peter Zijlstra wrote:
> Since commit 2f36825b1 ("sched: Next buddy hint on sleep and preempt
> path") it is likely we pick a new task from the same cgroup, doing a put
> and then set on all intermediate entities is a waste of time, so try to
> avoid this.
I just noticed put_prev_entity() also does the bandwidth enforcement
stuff, I think I just broke that. Will have a peek at fixing that
tomorrow or so.
On Thu, 2012-06-14 at 23:19 +0200, Peter Zijlstra wrote:
> On Thu, 2012-06-14 at 15:29 +0200, Peter Zijlstra wrote:
> > Since commit 2f36825b1 ("sched: Next buddy hint on sleep and preempt
> > path") it is likely we pick a new task from the same cgroup, doing a put
> > and then set on all intermediate entities is a waste of time, so try to
> > avoid this.
>
> I just noticed put_prev_entity() also does the bandwidth enforcement
> stuff, I think I just broke that. Will have a peek at fixing that
> tomorrow or so.
Damn, that's annoying, that wants to be done bottom-up, while we're now
doing a top-down selection. pjt any sane ideas?
On Fri, Jun 15, 2012 at 7:03 AM, Peter Zijlstra <[email protected]> wrote:
> On Thu, 2012-06-14 at 23:19 +0200, Peter Zijlstra wrote:
>> On Thu, 2012-06-14 at 15:29 +0200, Peter Zijlstra wrote:
>> > Since commit 2f36825b1 ("sched: Next buddy hint on sleep and preempt
>> > path") it is likely we pick a new task from the same cgroup, doing a put
>> > and then set on all intermediate entities is a waste of time, so try to
>> > avoid this.
>>
>> I just noticed put_prev_entity() also does the bandwidth enforcement
>> stuff, I think I just broke that. Will have a peek at fixing that
>> tomorrow or so.
>
> Damn, that's annoying, that wants to be done bottom-up, while we're now
> doing a top-down selection. pjt any sane ideas?
I'll have a look at this tomorrow, but I think this is fairly
immediately resolvable -- I don't see any immediate reason we need to
do full enforcement here. The only interesting thing we really need
to do in the put_path is the voluntary return of quota, which -- if we
need to do it -- we will get to do since that occurs iff the entity is
no longe runnable and actually getting a put().