sched: Optimize cgroup muck

RFC PATCH In review Peter Zijlstra 6 participants 16 messages Jun 14, 2012 – Jun 21, 2012 Scheduler

lore

Share & tools

lore

Peter Zijlstra

author

2012-06-14 13:33 UTC #0 lore raw

cover sched: Optimize cgroup muck

Hi, I finally got some time to make this stuff work (it boots!).

I haven't actually done any benchmarks etc.. but various people expressed interest in the fold put_prev_task into pick_next_task thing.

Please give it a spin or so..

Peter Zijlstra

author

2012-06-14 13:33 UTC #1 lore raw

RFC PATCH 1/4 sched/fair: track cgroup depth

Track depth in cgroup tree, this is useful for things like find_matching_se() where you need to get to a common parent of two sched entities.

Keeping the depth avoids having to calculate it on the spot, which saves a number of possible cache-misses.

Signed-off-by Peter Zijlstra <[email protected]>

---
 include/linux/sched.h |    1 +
 kernel/sched/fair.c   |   42 +++++++++++++++++++-----------------------
 2 files changed, 20 insertions(+), 23 deletions(-)

include/linux/sched.h +1 -0

@@ -1186,6 +1186,7 @@ struct sched_entity {
 #endif
 
 #ifdef CONFIG_FAIR_GROUP_SCHED
+	int			depth;
 	struct sched_entity	*parent;
 	/* rq on which this entity is (to be) queued: */
 	struct cfs_rq		*cfs_rq;

kernel/sched/fair.c +19 ~~-23~~

97 lines not on this page · raw message

Peter Zijlstra

author

2012-06-14 13:33 UTC #2 lore raw

RFC PATCH 2/4 sched: Push put_prev_task() into pick_next_task()

In reply to Peter Zijlstra

In order to avoid having to do put/set on a whole cgroup hierarchy when we context switch, push the put into pick_next_task() so that both operations are in the same function. Further changes then allow us to possibly optimize away redundant work.

Signed-off-by Peter Zijlstra <[email protected]>

---
 include/linux/sched.h    |    3 ++-
 kernel/sched/core.c      |   13 +++++++------
 kernel/sched/fair.c      |    6 +++++-
 kernel/sched/idle_task.c |    6 +++++-
 kernel/sched/rt.c        |   27 ++++++++++++++++-----------
 kernel/sched/stop_task.c |    9 +++++++--
 6 files changed, 42 insertions(+), 22 deletions(-)

include/linux/sched.h +2 -1

@@ -1093,7 +1093,8 @@ struct sched_class {
 
 	void (*check_preempt_curr) (struct rq *rq, struct task_struct *p, int flags);
 
-	struct task_struct * (*pick_next_task) (struct rq *rq);
+	struct task_struct * (*pick_next_task) (struct rq *rq,
+						struct task_struct *prev);
 	void (*put_prev_task) (struct rq *rq, struct task_struct *p);
 
 #ifdef CONFIG_SMP

kernel/sched/core.c +7 -6

@@ -3176,7 +3176,7 @@ static void put_prev_task(struct rq *rq,
  * Pick up the highest-prio task:
  */
 static inline struct task_struct *
-pick_next_task(struct rq *rq)
+pick_next_task(struct rq *rq, struct task_struct *prev)
 {
 	const struct sched_class *class;
 	struct task_struct *p;

@@ -3186,13 +3186,13 @@ pick_next_task(struct rq *rq)
 	 * the fair class we can call that function directly:
 	 */
 	if (likely(rq->nr_running == rq->cfs.h_nr_running)) {
-		p = fair_sched_class.pick_next_task(rq);
+		p = fair_sched_class.pick_next_task(rq, prev);
 		if (likely(p))
 			return p;
 	}
 
 	for_each_class(class) {
-		p = class->pick_next_task(rq);
+		p = class->pick_next_task(rq, prev);
 		if (p)
 			return p;
 	}

@@ -3253,8 +3253,9 @@ static void __sched __schedule(void)
 	if (unlikely(!rq->nr_running))
 		idle_balance(cpu, rq);
 
-	put_prev_task(rq, prev);
-	next = pick_next_task(rq);
+	if (prev->on_rq || rq->skip_clock_update < 0)
+		update_rq_clock(rq);
+	next = pick_next_task(rq, prev);
 	clear_tsk_need_resched(prev);
 	rq->skip_clock_update = 0;

@@ -5200,7 +5201,7 @@ static void migrate_tasks(unsigned int d
 		if (rq->nr_running == 1)
 			break;
 
-		next = pick_next_task(rq);
+		next = pick_next_task(rq, NULL);
 		BUG_ON(!next);
 		next->sched_class->put_prev_task(rq, next);

kernel/sched/fair.c +5 -1

@@ -2990,7 +2990,8 @@ static void check_preempt_wakeup(struct 
 		set_last_buddy(se);
 }
 
-static struct task_struct *pick_next_task_fair(struct rq *rq)
+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;

@@ -2999,6 +3000,9 @@ static struct task_struct *pick_next_tas
 	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);

kernel/sched/idle_task.c +5 -1

@@ -22,8 +22,12 @@ static void check_preempt_curr_idle(stru
 	resched_task(rq->idle);
 }
 
-static struct task_struct *pick_next_task_idle(struct rq *rq)
+static struct task_struct *
+pick_next_task_idle(struct rq *rq, struct task_struct *prev)
 {
+	if (prev)
+		prev->sched_class->put_prev_task(rq, prev);
+
 	schedstat_inc(rq, sched_goidle);
 	calc_load_account_idle(rq);
 	return rq->idle;

kernel/sched/rt.c +16 ~~-11~~

@@ -1346,15 +1346,7 @@ static struct task_struct *_pick_next_ta
 {
 	struct sched_rt_entity *rt_se;
 	struct task_struct *p;
-	struct rt_rq *rt_rq;
-
-	rt_rq = &rq->rt;
-
-	if (!rt_rq->rt_nr_running)
-		return NULL;
-
-	if (rt_rq_throttled(rt_rq))
-		return NULL;
+	struct rt_rq *rt_rq  = &rq->rt;
 
 	do {
 		rt_se = pick_next_rt_entity(rq, rt_rq);

@@ -1368,9 +1360,22 @@ static struct task_struct *_pick_next_ta
 	return p;
 }
 
-static struct task_struct *pick_next_task_rt(struct rq *rq)
+static struct task_struct *
+pick_next_task_rt(struct rq *rq, struct task_struct *prev)
 {
-	struct task_struct *p = _pick_next_task_rt(rq);
+	struct task_struct *p;
+	struct rt_rq *rt_rq = &rq->rt;
+
+	if (!rt_rq->rt_nr_running)
+		return NULL;
+
+	if (rt_rq_throttled(rt_rq))
+		return NULL;
+
+	if (prev)
+		prev->sched_class->put_prev_task(rq, prev);
+
+	p = _pick_next_task_rt(rq);
 
 	/* The running task is never eligible for pushing */
 	if (p)

kernel/sched/stop_task.c +7 -2

@@ -23,12 +23,17 @@ check_preempt_curr_stop(struct rq *rq, s
 	/* we're never preempted */
 }
 
-static struct task_struct *pick_next_task_stop(struct rq *rq)
+static struct task_struct *
+pick_next_task_stop(struct rq *rq, struct task_struct *prev)
 {
 	struct task_struct *stop = rq->stop;
 
-	if (stop && stop->on_rq)
+	if (stop && stop->on_rq) {
+		if (prev)
+			prev->sched_class->put_prev_task(rq, prev);
+
 		return stop;
+	}
 
 	return NULL;
 }

Steven Rostedt

reviewer

2012-06-14 14:32 UTC #3 lore raw

Re: [RFC][PATCH 2/4] sched: Push put_prev_task() into pick_next_task()

Quoted 28 lines · Peter Zijlstra

kernel/sched/core.c +3 -3

@@ -3176,7 +3176,7 @@ static void put_prev_task(struct rq *rq,
  * Pick up the highest-prio task:
  */
 static inline struct task_struct *
-pick_next_task(struct rq *rq)
+pick_next_task(struct rq *rq, struct task_struct *prev)
 {
 	const struct sched_class *class;
 	struct task_struct *p;

@@ -3186,13 +3186,13 @@ pick_next_task(struct rq *rq)
 	 * the fair class we can call that function directly:
 	 */
 	if (likely(rq->nr_running == rq->cfs.h_nr_running)) {
-		p = fair_sched_class.pick_next_task(rq);
+		p = fair_sched_class.pick_next_task(rq, prev);
 		if (likely(p))
 			return p;
 	}
 
 	for_each_class(class) {
-		p = class->pick_next_task(rq);
+		p = class->pick_next_task(rq, prev);
 		if (p)
 			return p;
 	}

kernel/sched/core.c static void __sched __schedule(void) · line 3256

Quoted 4 lines

kernel/sched/core.c +0 -1

@@ -3253,8 +3253,9 @@ static void __sched __schedule(void)
 	if (unlikely(!rq->nr_running))
 		idle_balance(cpu, rq);
 
-	put_prev_task(rq, prev);

You remove the call to put_prev_task() which looks to be the only user of this static function. But I don't see the deletion of this function.

-- Steve

Quoted 16 lines

kernel/sched/core.c +4 -2

-	next = pick_next_task(rq);
+	if (prev->on_rq || rq->skip_clock_update < 0)
+		update_rq_clock(rq);
+	next = pick_next_task(rq, prev);
 	clear_tsk_need_resched(prev);
 	rq->skip_clock_update = 0;

@@ -5200,7 +5201,7 @@ static void migrate_tasks(unsigned int d
 		if (rq->nr_running == 1)
 			break;
 
-		next = pick_next_task(rq);
+		next = pick_next_task(rq, NULL);
 		BUG_ON(!next);
 		next->sched_class->put_prev_task(rq, next);

Peter Zijlstra

author

2012-06-14 14:58 UTC #4 lore raw

Re: [RFC][PATCH 2/4] sched: Push put_prev_task() into pick_next_task()

Quoted 6 lines · Steven Rostedt

You remove the call to put_prev_task() which looks to be the only user of this static function. But I don't see the deletion of this function.

Right you are.. must've overlooked my compiler complaining about this.

Glauber Costa

2012-06-15 09:51 UTC #5 lore raw

Re: [RFC][PATCH 2/4] sched: Push put_prev_task() into pick_next_task()

In reply to Peter Zijlstra

Quoted 6 lines · Peter Zijlstra

kernel/sched/rt.c +4 -1

+static struct task_struct *
+pick_next_task_rt(struct rq *rq, struct task_struct *prev)
  {
-	struct task_struct *p = _pick_next_task_rt(rq);
+	struct task_struct *p;
+	struct rt_rq *rt_rq =&rq->rt;

kernel/sched/rt.c static struct task_struct *_pick_next_ta · line 1377

Quoted 10 lines

kernel/sched/rt.c +10 -0

+
+	if (!rt_rq->rt_nr_running)
+		return NULL;
+
+	if (rt_rq_throttled(rt_rq))
+		return NULL;
+
+	if (prev)
+		prev->sched_class->put_prev_task(rq, prev);
+

it might be me, but this one sounds strange. If the responsibility of putting the task now lays with pic_next_task, you can't return NULL without doing this first.

Peter Zijlstra

author

2012-06-15 10:03 UTC #6 lore raw

Re: [RFC][PATCH 2/4] sched: Push put_prev_task() into pick_next_task()

Quoted 12 lines · Glauber Costa

Quoted 11 lines · Peter Zijlstra

(quoted diff) +9 -1

+static struct task_struct *
+pick_next_task_rt(struct rq *rq, struct task_struct *prev)
  {
-	struct task_struct *p = _pick_next_task_rt(rq);
+	struct task_struct *p;
+	struct rt_rq *rt_rq =&rq->rt;
+
+	if (!rt_rq->rt_nr_running)
+		return NULL;
+
+	if (rt_rq_throttled(rt_rq))

Quoted 8 lines

Review
Quoted 5 lines
(quoted diff) +5 -0
+		return NULL;
+
+	if (prev)
+		prev->sched_class->put_prev_task(rq, prev);
+
it might be me, but this one sounds strange. If the responsibility of putting the task now lays with pic_next_task, you can't return NULL without doing this first.

Its the responsibility of the sched_class::pick_next_task implementation that will return the next task. Not of the first one called.

Clearly this needs a comment.. /me adds.

---

include/linux/sched.h +5 -0

@@ -1096,6 +1096,11 @@ struct sched_class {
 
 	void (*check_preempt_curr) (struct rq *rq, struct task_struct *p, int flags);
 
+	/*
+	 * It is the responsibility of the pick_next_task() method that will
+	 * return the next task to call put_prev_task() on the @prev task or
+	 * something equivalent.
+	 */
 	struct task_struct * (*pick_next_task) (struct rq *rq,
 						struct task_struct *prev);
 	void (*put_prev_task) (struct rq *rq, struct task_struct *p);

Glauber Costa

2012-06-15 10:04 UTC #7 lore raw

Re: [RFC][PATCH 2/4] sched: Push put_prev_task() into pick_next_task()

Quoted 22 lines · Peter Zijlstra

Quoted 12 lines · Glauber Costa
Quoted 11 lines · Peter Zijlstra
(quoted diff) +9 -1
+static struct task_struct *
+pick_next_task_rt(struct rq *rq, struct task_struct *prev)
   {
-	struct task_struct *p = _pick_next_task_rt(rq);
+	struct task_struct *p;
+	struct rt_rq *rt_rq =&rq->rt;
+
+	if (!rt_rq->rt_nr_running)
+		return NULL;
+
+	if (rt_rq_throttled(rt_rq))
Quoted 8 lines
Review
Quoted 5 lines
(quoted diff) +5 -0
+		return NULL;
+
+	if (prev)
+		prev->sched_class->put_prev_task(rq, prev);
+
it might be me, but this one sounds strange. If the responsibility of putting the task now lays with pic_next_task, you can't return NULL without doing this first.

Quoted 4 lines

Its the responsibility of the sched_class::pick_next_task implementation that will return the next task. Not of the first one called.
Clearly this needs a comment.. /me adds.

hummmm, yeah, you are right. The comment does make it clearer.

Michael Wang

2012-06-21 07:35 UTC #8 lore raw

Re: [RFC][PATCH 2/4] sched: Push put_prev_task() into pick_next_task()

In reply to Peter Zijlstra

Quoted 18 lines

kernel/sched/fair.c +4 -0

+pick_next_task_fair(struct rq *rq, struct task_struct *prev)
 {
 	struct task_struct *p;
 	struct cfs_rq *cfs_rq = &rq->cfs;

@@ -2999,6 +3000,9 @@ static struct task_struct *pick_next_tas
 	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);

kernel/sched/idle_task.c static void check_preempt_curr_idle(stru · line 23

Quoted 2 lines

kernel/sched/idle_task.c +0 -0

@@ -22,8 +22,12 @@ static void check_preempt_curr_idle(stru
 	resched_task(rq->idle);
 }

Will it cause trouble when the last task on cfs_rq was dequeued before been putted?

The cfs_rq->nr_running will be 0 and we will miss the chance to put prev, will we?

Regards,
Michael Wang

Peter Zijlstra

author

2012-06-21 08:41 UTC #9 lore raw

Re: [RFC][PATCH 2/4] sched: Push put_prev_task() into pick_next_task()

Quoted 23 lines · Michael Wang

Quoted 17 lines

kernel/sched/fair.c +4 -0

+pick_next_task_fair(struct rq *rq, struct task_struct *prev)
 {
 	struct task_struct *p;
 	struct cfs_rq *cfs_rq = &rq->cfs;

@@ -2999,6 +3000,9 @@ static struct task_struct *pick_next_tas
 	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);

kernel/sched/idle_task.c static void check_preempt_curr_idle(stru · line 23

Quoted 2 lines

kernel/sched/idle_task.c +0 -0

@@ -22,8 +22,12 @@ static void check_preempt_curr_idle(stru
 	resched_task(rq->idle);
 }

Will it cause trouble when the last task on cfs_rq was dequeued before been putted?

Quoted 2 lines

The cfs_rq->nr_running will be 0 and we will miss the chance to put prev, will we?

pick_next_task_idle() will then do the put, no?

Michael Wang

2012-06-21 09:23 UTC #10 lore raw

Re: [RFC][PATCH 2/4] sched: Push put_prev_task() into pick_next_task()

Quoted 24 lines · Peter Zijlstra

Quoted 23 lines · Michael Wang
Quoted 17 lines
kernel/sched/fair.c +4 -0
+pick_next_task_fair(struct rq *rq, struct task_struct *prev)
 {
 	struct task_struct *p;
 	struct cfs_rq *cfs_rq = &rq->cfs;
@@ -2999,6 +3000,9 @@ static struct task_struct *pick_next_tas
 	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);
kernel/sched/idle_task.c static void check_preempt_curr_idle(stru · line 23
Quoted 2 lines
kernel/sched/idle_task.c +0 -0
@@ -22,8 +22,12 @@ static void check_preempt_curr_idle(stru
 	resched_task(rq->idle);
 }
Will it cause trouble when the last task on cfs_rq was dequeued before been putted?

Quoted 4 lines

Quoted 2 lines
The cfs_rq->nr_running will be 0 and we will miss the chance to put prev, will we?
pick_next_task_idle() will then do the put, no?

Oh, that's right, I see.

Quoted 1 line

Peter Zijlstra

author

2012-06-14 13:33 UTC #11 lore raw

RFC PATCH 3/4 sched/fair: clean up __clear_buddies_*

In reply to Peter Zijlstra

Slightly easier code flow, no functional changes.

Signed-off-by Peter Zijlstra <[email protected]>

---
 kernel/sched/fair.c |   18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

kernel/sched/fair.c +9 -9

@@ -1115,10 +1115,10 @@ static void __clear_buddies_last(struct 
 {
 	for_each_sched_entity(se) {
 		struct cfs_rq *cfs_rq = cfs_rq_of(se);
-		if (cfs_rq->last == se)
-			cfs_rq->last = NULL;
-		else
+		if (cfs_rq->last != se)
 			break;
+
+		cfs_rq->last = NULL;
 	}
 }

@@ -1126,10 +1126,10 @@ static void __clear_buddies_next(struct 
 {
 	for_each_sched_entity(se) {
 		struct cfs_rq *cfs_rq = cfs_rq_of(se);
-		if (cfs_rq->next == se)
-			cfs_rq->next = NULL;
-		else
+		if (cfs_rq->next != se)
 			break;
+
+		cfs_rq->next = NULL;
 	}
 }

@@ -1137,10 +1137,10 @@ static void __clear_buddies_skip(struct 
 {
 	for_each_sched_entity(se) {
 		struct cfs_rq *cfs_rq = cfs_rq_of(se);
-		if (cfs_rq->skip == se)
-			cfs_rq->skip = NULL;
-		else
+		if (cfs_rq->skip != se)
 			break;
+
+		cfs_rq->skip = NULL;
 	}
 }

Peter Zijlstra

author

2012-06-14 13:33 UTC #12 lore raw

RFC PATCH 4/4 sched/fair: Optimize cgroup pick_next_task_fair

In reply to Peter Zijlstra

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(-)

kernel/sched/fair.c +68 -7

@@ -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);

Peter Zijlstra

author

2012-06-14 21:19 UTC #13 lore raw

Re: [RFC][PATCH 4/4] sched/fair: Optimize cgroup pick_next_task_fair

Quoted 4 lines · Peter Zijlstra

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.

Peter Zijlstra

maintainer

2012-06-15 14:03 UTC #14 lore raw

Re: [RFC][PATCH 4/4] sched/fair: Optimize cgroup pick_next_task_fair

Quoted 9 lines · Peter Zijlstra

Quoted 4 lines · Peter Zijlstra
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?

Paul Turner

2012-06-19 08:45 UTC #15 lore raw

Re: [RFC][PATCH 4/4] sched/fair: Optimize cgroup pick_next_task_fair

Quoted 7 lines · Peter Zijlstra

Quoted 6 lines · Peter Zijlstra
Quoted 4 lines · Peter Zijlstra
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.

Quoted 6 lines

Quoted 3 lines
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().