Skip to contentSkip to conversation

scheduler fixlets

PATCH In review Paul Turner 2 participants 15 messages Jan 22, 2011 – Jan 27, 2011

lore
Share & tools
lore

Paul Turner

author
#0 lore raw

cover scheduler fixlets

A small set of scheduler patches for the CFS side of things.

Mostly buglet fixes with one or two associated clean-ups. Each patch should be (modulo any merge fuzz) independent.

Thanks,

- Paul

author
#1 lore raw

PATCH 1/5 sched: fix sign under-flows in wake_affine

While care is taken around the zero-point in effective_load to not exceed the instantaneous rq->weight, it's still possible (e.g. using wake_idx != 0) for (load + effective_load) to underflow.

In this case the comparing the unsigned values can result in incorrect balanced decisions.

Signed-off-by Paul Turner <[email protected]>
---
 kernel/sched_fair.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
kernel/sched_fair.c +3 -3
Index: tip3/kernel/sched_fair.c
===================================================================
@@ -1404,7 +1404,7 @@ static inline unsigned long effective_lo
 
 static int wake_affine(struct sched_domain *sd, struct task_struct *p, int sync)
 {
-	unsigned long this_load, load;
+	s64 this_load, load;
 	int idx, this_cpu, prev_cpu;
 	unsigned long tl_per_task;
 	struct task_group *tg;

@@ -1443,8 +1443,8 @@ static int wake_affine(struct sched_doma
 	 * Otherwise check if either cpus are near enough in load to allow this
 	 * task to be woken on this_cpu.
 	 */
-	if (this_load) {
-		unsigned long this_eff_load, prev_eff_load;
+	if (this_load > 0) {
+		s64 this_eff_load, prev_eff_load;
 
 		this_eff_load = 100;
 		this_eff_load *= power_of(prev_cpu);
author
#2 lore raw

[tip:sched/core] sched: Fix sign under-flows in wake_affine

Commit-ID:  e37b6a7b27b400c3aa488db8c6629a05095bc79c
Gitweb:     http://git.kernel.org/tip/e37b6a7b27b400c3aa488db8c6629a05095bc79c
Author:     Paul Turner <[email protected]>
AuthorDate: Fri, 21 Jan 2011 20:44:59 -0800
Committer:  Ingo Molnar <[email protected]>
CommitDate: Wed, 26 Jan 2011 12:31:01 +0100

sched: Fix sign under-flows in wake_affine

While care is taken around the zero-point in effective_load to not exceed the instantaneous rq->weight, it's still possible (e.g. using wake_idx != 0) for (load + effective_load) to underflow.

In this case the comparing the unsigned values can result in incorrect balanced decisions.

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

LKML-Reference: <[email protected]>

Signed-off-by Ingo Molnar <[email protected]>
---
 kernel/sched_fair.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
kernel/sched_fair.c +3 -3
index 3547699..ccecfec 100644
@@ -1432,7 +1432,7 @@ static inline unsigned long effective_load(struct task_group *tg, int cpu,
 
 static int wake_affine(struct sched_domain *sd, struct task_struct *p, int sync)
 {
-	unsigned long this_load, load;
+	s64 this_load, load;
 	int idx, this_cpu, prev_cpu;
 	unsigned long tl_per_task;
 	struct task_group *tg;

@@ -1471,8 +1471,8 @@ static int wake_affine(struct sched_domain *sd, struct task_struct *p, int sync)
 	 * Otherwise check if either cpus are near enough in load to allow this
 	 * task to be woken on this_cpu.
 	 */
-	if (this_load) {
-		unsigned long this_eff_load, prev_eff_load;
+	if (this_load > 0) {
+		s64 this_eff_load, prev_eff_load;
 
 		this_eff_load = 100;
 		this_eff_load *= power_of(prev_cpu);
author
#3 lore raw

PATCH 2/5 sched: (cleanup) remove redundant cfs_rq checks

In reply to Paul Turner

Since updates are against an entity's queuing cfs_rq it's not possible to enter update_cfs_{shares,load} with a NULL cfs_rq. (Indeed, update_cfs_load would crash prior to the check if we did anyway since we load is examined during the initializers). Also, in the update_cfs_load case there's no point in maintaining averages for rq->cfs_rq since we don't perform shares distribution at that level -- NULL check is replaced accordingly.

Thanks to Dan Carpenter for pointing out the deference before NULL check.

Signed-off-by Paul Turner <[email protected]>
---
 kernel/sched_fair.c |    5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)
kernel/sched_fair.c +1 -4
Index: tip3/kernel/sched_fair.c
===================================================================
@@ -721,7 +721,7 @@ static void update_cfs_load(struct cfs_r
 	u64 now, delta;
 	unsigned long load = cfs_rq->load.weight;
 
-	if (!cfs_rq)
+	if (cfs_rq->tg == &root_task_group)
 		return;
 
 	now = rq_of(cfs_rq)->clock;

@@ -784,9 +784,6 @@ static void update_cfs_shares(struct cfs
 	struct sched_entity *se;
 	long load_weight, load, shares;
 
-	if (!cfs_rq)
-		return;
-
 	tg = cfs_rq->tg;
 	se = tg->se[cpu_of(rq_of(cfs_rq))];
 	if (!se)
author
#4 lore raw

[tip:sched/core] sched: Fix/remove redundant cfs_rq checks

Commit-ID:  b815f1963e47b9b69bb17e0588bd5af5b1114ae0
Gitweb:     http://git.kernel.org/tip/b815f1963e47b9b69bb17e0588bd5af5b1114ae0
Author:     Paul Turner <[email protected]>
AuthorDate: Fri, 21 Jan 2011 20:45:00 -0800
Committer:  Ingo Molnar <[email protected]>
CommitDate: Wed, 26 Jan 2011 12:31:02 +0100

sched: Fix/remove redundant cfs_rq checks

Since updates are against an entity's queuing cfs_rq it's not possible to enter update_cfs_{shares,load} with a NULL cfs_rq. (Indeed, update_cfs_load would crash prior to the check if we did anyway since we load is examined during the initializers).

Also, in the update_cfs_load case there's no point
in maintaining averages for rq->cfs_rq since we don't perform shares distribution at that level -- NULL check is replaced accordingly.

Thanks to Dan Carpenter for pointing out the deference before NULL check.

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

LKML-Reference: <[email protected]>

Signed-off-by Ingo Molnar <[email protected]>
---
 kernel/sched_fair.c |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)
kernel/sched_fair.c +1 -4
index ccecfec..1997383 100644
@@ -722,7 +722,7 @@ static void update_cfs_load(struct cfs_rq *cfs_rq, int global_update)
 	u64 now, delta;
 	unsigned long load = cfs_rq->load.weight;
 
-	if (!cfs_rq)
+	if (cfs_rq->tg == &root_task_group)
 		return;
 
 	now = rq_of(cfs_rq)->clock;

@@ -830,9 +830,6 @@ static void update_cfs_shares(struct cfs_rq *cfs_rq, long weight_delta)
 	struct sched_entity *se;
 	long shares;
 
-	if (!cfs_rq)
-		return;
-
 	tg = cfs_rq->tg;
 	se = tg->se[cpu_of(rq_of(cfs_rq))];
 	if (!se)
author
#5 lore raw

PATCH 3/5 sched: simplify update_cfs_shares parameters

In reply to Paul Turner

Re-visiting this: Since update_cfs_shares will now only ever re-weight an entity that is a relative parent of the current entity in enqueue_entity; we can safely issue the account_entity_enqueue relative to that cfs_rq and avoid the requirement for special handling of the enqueue case in update_cfs_shares.

Signed-off-by Paul Turner <[email protected]>
---
 kernel/sched.c      |    2 +-
 kernel/sched_fair.c |   22 +++++++++++-----------
 2 files changed, 12 insertions(+), 12 deletions(-)
kernel/sched_fair.c +11 -11
Index: tip3/kernel/sched_fair.c
===================================================================
@@ -540,7 +540,7 @@ static u64 sched_vslice(struct cfs_rq *c
 }
 
 static void update_cfs_load(struct cfs_rq *cfs_rq, int global_update);
-static void update_cfs_shares(struct cfs_rq *cfs_rq, long weight_delta);
+static void update_cfs_shares(struct cfs_rq *cfs_rq);
 
 /*
  * Update the current task's runtime statistics. Skip current tasks that

@@ -778,7 +778,7 @@ static void reweight_entity(struct cfs_r
 		account_entity_enqueue(cfs_rq, se);
 }
 
-static void update_cfs_shares(struct cfs_rq *cfs_rq, long weight_delta)
+static void update_cfs_shares(struct cfs_rq *cfs_rq)
 {
 	struct task_group *tg;
 	struct sched_entity *se;

@@ -789,11 +789,11 @@ static void update_cfs_shares(struct cfs
 	if (!se)
 		return;
 
-	load = cfs_rq->load.weight + weight_delta;
+	load = cfs_rq->load.weight;
 
 	load_weight = atomic_read(&tg->load_weight);
-	load_weight -= cfs_rq->load_contribution;
 	load_weight += load;
+	load_weight -= cfs_rq->load_contribution;
 
 	shares = (tg->shares * load);
 	if (load_weight)

@@ -811,7 +811,7 @@ static void update_entity_shares_tick(st
 {
 	if (cfs_rq->load_unacc_exec_time > sysctl_sched_shares_window) {
 		update_cfs_load(cfs_rq, 0);
-		update_cfs_shares(cfs_rq, 0);
+		update_cfs_shares(cfs_rq);
 	}
 }
 #else /* CONFIG_FAIR_GROUP_SCHED */

@@ -819,7 +819,7 @@ static void update_cfs_load(struct cfs_r
 {
 }
 
-static inline void update_cfs_shares(struct cfs_rq *cfs_rq, long weight_delta)
+static inline void update_cfs_shares(struct cfs_rq *cfs_rq)
 {
 }
 

@@ -950,8 +950,8 @@ enqueue_entity(struct cfs_rq *cfs_rq, st
 	 */
 	update_curr(cfs_rq);
 	update_cfs_load(cfs_rq, 0);
-	update_cfs_shares(cfs_rq, se->load.weight);
 	account_entity_enqueue(cfs_rq, se);
+	update_cfs_shares(cfs_rq);
 
 	if (flags & ENQUEUE_WAKEUP) {
 		place_entity(cfs_rq, se, 0);

@@ -1013,7 +1013,7 @@ dequeue_entity(struct cfs_rq *cfs_rq, st
 	update_cfs_load(cfs_rq, 0);
 	account_entity_dequeue(cfs_rq, se);
 	update_min_vruntime(cfs_rq);
-	update_cfs_shares(cfs_rq, 0);
+	update_cfs_shares(cfs_rq);
 
 	/*
 	 * Normalize the entity after updating the min_vruntime because the

@@ -1254,7 +1254,7 @@ enqueue_task_fair(struct rq *rq, struct 
 		struct cfs_rq *cfs_rq = cfs_rq_of(se);
 
 		update_cfs_load(cfs_rq, 0);
-		update_cfs_shares(cfs_rq, 0);
+		update_cfs_shares(cfs_rq);
 	}
 
 	hrtick_update(rq);

@@ -1284,7 +1284,7 @@ static void dequeue_task_fair(struct rq 
 		struct cfs_rq *cfs_rq = cfs_rq_of(se);
 
 		update_cfs_load(cfs_rq, 0);
-		update_cfs_shares(cfs_rq, 0);
+		update_cfs_shares(cfs_rq);
 	}
 
 	hrtick_update(rq);

@@ -2095,7 +2095,7 @@ static int update_shares_cpu(struct task
 	 * We need to update shares after updating tg->load_weight in
 	 * order to adjust the weight of groups with long running tasks.
 	 */
-	update_cfs_shares(cfs_rq, 0);
+	update_cfs_shares(cfs_rq);
 
 	raw_spin_unlock_irqrestore(&rq->lock, flags);
kernel/sched.c +1 -1
Index: tip3/kernel/sched.c
===================================================================
@@ -8510,7 +8510,7 @@ int sched_group_set_shares(struct task_g
 		/* Propagate contribution to hierarchy */
 		raw_spin_lock_irqsave(&rq->lock, flags);
 		for_each_sched_entity(se)
-			update_cfs_shares(group_cfs_rq(se), 0);
+			update_cfs_shares(group_cfs_rq(se));
 		raw_spin_unlock_irqrestore(&rq->lock, flags);
 	}
author
#6 lore raw

[tip:sched/core] sched: Simplify update_cfs_shares parameters

Commit-ID:  6d5ab2932a21ea54406ab95c43ecff90a3eddfda
Gitweb:     http://git.kernel.org/tip/6d5ab2932a21ea54406ab95c43ecff90a3eddfda
Author:     Paul Turner <[email protected]>
AuthorDate: Fri, 21 Jan 2011 20:45:01 -0800
Committer:  Ingo Molnar <[email protected]>
CommitDate: Wed, 26 Jan 2011 12:33:19 +0100

sched: Simplify update_cfs_shares parameters

Re-visiting this: Since update_cfs_shares will now only ever re-weight an entity that is a relative parent of the current entity in enqueue_entity; we can safely issue the account_entity_enqueue relative to that cfs_rq and avoid the requirement for special handling of the enqueue case in update_cfs_shares.

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

LKML-Reference: <[email protected]>

Signed-off-by Ingo Molnar <[email protected]>
---
 kernel/sched.c      |    2 +-
 kernel/sched_fair.c |   30 ++++++++++++++----------------
 2 files changed, 15 insertions(+), 17 deletions(-)
kernel/sched.c +1 -1
index 18d38e4..e0fa3ff 100644
@@ -8510,7 +8510,7 @@ int sched_group_set_shares(struct task_group *tg, unsigned long shares)
 		/* Propagate contribution to hierarchy */
 		raw_spin_lock_irqsave(&rq->lock, flags);
 		for_each_sched_entity(se)
-			update_cfs_shares(group_cfs_rq(se), 0);
+			update_cfs_shares(group_cfs_rq(se));
 		raw_spin_unlock_irqrestore(&rq->lock, flags);
 	}
kernel/sched_fair.c +14 -16
index 0c26e2d..0c550c8 100644
@@ -540,7 +540,7 @@ static u64 sched_vslice(struct cfs_rq *cfs_rq, struct sched_entity *se)
 }
 
 static void update_cfs_load(struct cfs_rq *cfs_rq, int global_update);
-static void update_cfs_shares(struct cfs_rq *cfs_rq, long weight_delta);
+static void update_cfs_shares(struct cfs_rq *cfs_rq);
 
 /*
  * Update the current task's runtime statistics. Skip current tasks that

@@ -763,16 +763,15 @@ static void update_cfs_load(struct cfs_rq *cfs_rq, int global_update)
 		list_del_leaf_cfs_rq(cfs_rq);
 }
 
-static long calc_cfs_shares(struct cfs_rq *cfs_rq, struct task_group *tg,
-				long weight_delta)
+static long calc_cfs_shares(struct cfs_rq *cfs_rq, struct task_group *tg)
 {
 	long load_weight, load, shares;
 
-	load = cfs_rq->load.weight + weight_delta;
+	load = cfs_rq->load.weight;
 
 	load_weight = atomic_read(&tg->load_weight);
-	load_weight -= cfs_rq->load_contribution;
 	load_weight += load;
+	load_weight -= cfs_rq->load_contribution;
 
 	shares = (tg->shares * load);
 	if (load_weight)

@@ -790,7 +789,7 @@ static void update_entity_shares_tick(struct cfs_rq *cfs_rq)
 {
 	if (cfs_rq->load_unacc_exec_time > sysctl_sched_shares_window) {
 		update_cfs_load(cfs_rq, 0);
-		update_cfs_shares(cfs_rq, 0);
+		update_cfs_shares(cfs_rq);
 	}
 }
 # else /* CONFIG_SMP */

@@ -798,8 +797,7 @@ static void update_cfs_load(struct cfs_rq *cfs_rq, int global_update)
 {
 }
 
-static inline long calc_cfs_shares(struct cfs_rq *cfs_rq, struct task_group *tg,
-				long weight_delta)
+static inline long calc_cfs_shares(struct cfs_rq *cfs_rq, struct task_group *tg)
 {
 	return tg->shares;
 }

@@ -824,7 +822,7 @@ static void reweight_entity(struct cfs_rq *cfs_rq, struct sched_entity *se,
 		account_entity_enqueue(cfs_rq, se);
 }
 
-static void update_cfs_shares(struct cfs_rq *cfs_rq, long weight_delta)
+static void update_cfs_shares(struct cfs_rq *cfs_rq)
 {
 	struct task_group *tg;
 	struct sched_entity *se;

@@ -838,7 +836,7 @@ static void update_cfs_shares(struct cfs_rq *cfs_rq, long weight_delta)
 	if (likely(se->load.weight == tg->shares))
 		return;
 #endif
-	shares = calc_cfs_shares(cfs_rq, tg, weight_delta);
+	shares = calc_cfs_shares(cfs_rq, tg);
 
 	reweight_entity(cfs_rq_of(se), se, shares);
 }

@@ -847,7 +845,7 @@ static void update_cfs_load(struct cfs_rq *cfs_rq, int global_update)
 {
 }
 
-static inline void update_cfs_shares(struct cfs_rq *cfs_rq, long weight_delta)
+static inline void update_cfs_shares(struct cfs_rq *cfs_rq)
 {
 }
 

@@ -978,8 +976,8 @@ enqueue_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
 	 */
 	update_curr(cfs_rq);
 	update_cfs_load(cfs_rq, 0);
-	update_cfs_shares(cfs_rq, se->load.weight);
 	account_entity_enqueue(cfs_rq, se);
+	update_cfs_shares(cfs_rq);
 
 	if (flags & ENQUEUE_WAKEUP) {
 		place_entity(cfs_rq, se, 0);

@@ -1041,7 +1039,7 @@ dequeue_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
 	update_cfs_load(cfs_rq, 0);
 	account_entity_dequeue(cfs_rq, se);
 	update_min_vruntime(cfs_rq);
-	update_cfs_shares(cfs_rq, 0);
+	update_cfs_shares(cfs_rq);
 
 	/*
 	 * Normalize the entity after updating the min_vruntime because the

@@ -1282,7 +1280,7 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
 		struct cfs_rq *cfs_rq = cfs_rq_of(se);
 
 		update_cfs_load(cfs_rq, 0);
-		update_cfs_shares(cfs_rq, 0);
+		update_cfs_shares(cfs_rq);
 	}
 
 	hrtick_update(rq);

@@ -1312,7 +1310,7 @@ static void dequeue_task_fair(struct rq *rq, struct task_struct *p, int flags)
 		struct cfs_rq *cfs_rq = cfs_rq_of(se);
 
 		update_cfs_load(cfs_rq, 0);
-		update_cfs_shares(cfs_rq, 0);
+		update_cfs_shares(cfs_rq);
 	}
 
 	hrtick_update(rq);

@@ -2123,7 +2121,7 @@ static int update_shares_cpu(struct task_group *tg, int cpu)
 	 * We need to update shares after updating tg->load_weight in
 	 * order to adjust the weight of groups with long running tasks.
 	 */
-	update_cfs_shares(cfs_rq, 0);
+	update_cfs_shares(cfs_rq);
 
 	raw_spin_unlock_irqrestore(&rq->lock, flags);
author
#7 lore raw

PATCH 4/5 sched: use rq->clock_task instead of rq->clock for maintaining load averages

In reply to Paul Turner

The delta in clock_task is a more fair attribution of how much time a tg has been contributing load to the current cpu.

While not really important it also means we're more in sync (by magnitude) with respect to periodic updates (since __update_curr deltas are clock_task based).

Signed-off-by Paul Turner <[email protected]>
---
 kernel/sched_fair.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/sched_fair.c +1 -1
Index: tip3/kernel/sched_fair.c
===================================================================
@@ -724,7 +724,7 @@ static void update_cfs_load(struct cfs_r
 	if (cfs_rq->tg == root_task_group)
 		return;
 
-	now = rq_of(cfs_rq)->clock;
+	now = rq_of(cfs_rq)->clock_task;
 	delta = now - cfs_rq->load_stamp;
 
 	/* truncate load history at 4 idle periods */
author
#8 lore raw

[tip:sched/core] sched: Use rq->clock_task instead of rq->clock for correctly maintaining load averages

Commit-ID:  05ca62c6ca17f39b88fa956d5ebc1fa6e93ad5e3
Gitweb:     http://git.kernel.org/tip/05ca62c6ca17f39b88fa956d5ebc1fa6e93ad5e3
Author:     Paul Turner <[email protected]>
AuthorDate: Fri, 21 Jan 2011 20:45:02 -0800
Committer:  Ingo Molnar <[email protected]>
CommitDate: Wed, 26 Jan 2011 12:31:03 +0100

sched: Use rq->clock_task instead of rq->clock for correctly maintaining load averages

The delta in clock_task is a more fair attribution of how much time a tg has been contributing load to the current cpu.

While not really important it also means we're more in sync (by magnitude) with respect to periodic updates (since __update_curr deltas are clock_task based).

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

LKML-Reference: <[email protected]>

Signed-off-by Ingo Molnar <[email protected]>
---
 kernel/sched_fair.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
kernel/sched_fair.c +1 -1
index 1997383..0c26e2d 100644
@@ -725,7 +725,7 @@ static void update_cfs_load(struct cfs_rq *cfs_rq, int global_update)
 	if (cfs_rq->tg == &root_task_group)
 		return;
 
-	now = rq_of(cfs_rq)->clock;
+	now = rq_of(cfs_rq)->clock_task;
 	delta = now - cfs_rq->load_stamp;
 
 	/* truncate load history at 4 idle periods */
author
#9 lore raw

PATCH 5/5 sched: avoid expensive initial update_cfs_load()

In reply to Paul Turner

Since cfs->{load_stamp,load_last} are zero-initalized the initial load update will consider the delta to be 'since the beginning of time'.

This results in a lot of pointless divisions to bring this large period to be within the sysctl_sched_shares_window.

Fix this by initializing load_stamp to be 1 at cfs_rq initialization, this allows for an initial load_stamp > load_last which then lets standard idle truncation proceed.

We avoid spinning (and slightly improve consistency) by fixing delta to be [period - 1] in this path resulting in a slightly more predictable shares ramp. (Previously the amount of idle time preserved by the overflow would range between [period/2,period-1].)

Signed-off-by Paul Turner <[email protected]>
---
 kernel/sched.c      |    2 ++
 kernel/sched_fair.c |    1 +
 2 files changed, 3 insertions(+)
kernel/sched.c +2 -0
Index: tip3/kernel/sched.c
===================================================================
@@ -7796,6 +7796,8 @@ static void init_cfs_rq(struct cfs_rq *c
 	INIT_LIST_HEAD(&cfs_rq->tasks);
 #ifdef CONFIG_FAIR_GROUP_SCHED
 	cfs_rq->rq = rq;
+	/* allow initial update_cfs_load() to truncate */
+	cfs_rq->load_stamp = 1;
 #endif
 	cfs_rq->min_vruntime = (u64)(-(1LL << 20));
 }
kernel/sched_fair.c +1 -0
Index: tip3/kernel/sched_fair.c
===================================================================
@@ -732,6 +732,7 @@ static void update_cfs_load(struct cfs_r
 	    now - cfs_rq->load_last > 4 * period) {
 		cfs_rq->load_period = 0;
 		cfs_rq->load_avg = 0;
+		delta = period - 1;
 	}
 
 	cfs_rq->load_stamp = now;
author
#10 lore raw

[tip:sched/core] sched: Avoid expensive initial update_cfs_load()

Commit-ID:  f07333bf6ee66d9b49286cec4371cf375e745b7a
Gitweb:     http://git.kernel.org/tip/f07333bf6ee66d9b49286cec4371cf375e745b7a
Author:     Paul Turner <[email protected]>
AuthorDate: Fri, 21 Jan 2011 20:45:03 -0800
Committer:  Ingo Molnar <[email protected]>
CommitDate: Wed, 26 Jan 2011 12:33:19 +0100

sched: Avoid expensive initial update_cfs_load()

Since cfs->{load_stamp,load_last} are zero-initalized the initial load update will consider the delta to be 'since the beginning of time'.

This results in a lot of pointless divisions to bring this large period to be within the sysctl_sched_shares_window.

Fix this by initializing load_stamp to be 1 at cfs_rq initialization, this allows for an initial load_stamp > load_last which then lets standard idle truncation proceed.

We avoid spinning (and slightly improve consistency) by fixing delta to be [period - 1] in this path resulting in a slightly more predictable shares ramp. (Previously the amount of idle time preserved by the overflow would range between [period/2,period-1].)

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

LKML-Reference: <[email protected]>

Signed-off-by Ingo Molnar <[email protected]>
---
 kernel/sched.c      |    2 ++
 kernel/sched_fair.c |    1 +
 2 files changed, 3 insertions(+), 0 deletions(-)
kernel/sched.c +2 -0
index e0fa3ff..6820b5b 100644
@@ -7796,6 +7796,8 @@ static void init_cfs_rq(struct cfs_rq *cfs_rq, struct rq *rq)
 	INIT_LIST_HEAD(&cfs_rq->tasks);
 #ifdef CONFIG_FAIR_GROUP_SCHED
 	cfs_rq->rq = rq;
+	/* allow initial update_cfs_load() to truncate */
+	cfs_rq->load_stamp = 1;
 #endif
 	cfs_rq->min_vruntime = (u64)(-(1LL << 20));
 }
kernel/sched_fair.c +1 -0
index 0c550c8..4cbc912 100644
@@ -733,6 +733,7 @@ static void update_cfs_load(struct cfs_rq *cfs_rq, int global_update)
 	    now - cfs_rq->load_last > 4 * period) {
 		cfs_rq->load_period = 0;
 		cfs_rq->load_avg = 0;
+		delta = period - 1;
 	}
 
 	cfs_rq->load_stamp = now;
#11 lore raw

Re: [tip:sched/core] sched: Avoid expensive initial update_cfs_load()

Quoted 12 lines · tip-bot for Paul Turner

index e0fa3ff..6820b5b 100644

kernel/sched.c +2 -0
@@ -7796,6 +7796,8 @@ static void init_cfs_rq(struct cfs_rq *cfs_rq,
 struct rq *rq)
        INIT_LIST_HEAD(&cfs_rq->tasks);
 #ifdef CONFIG_FAIR_GROUP_SCHED
        cfs_rq->rq = rq;
+       /* allow initial update_cfs_load() to truncate */
+       cfs_rq->load_stamp = 1;
 #endif
        cfs_rq->min_vruntime = (u64)(-(1LL << 20));
Quoted 1 line

}

That wants a fix to build on UP,

---

kernel/sched.c +2 -0
Index: linux-2.6/kernel/sched.c
===================================================================
@@ -7797,8 +7797,10 @@ static void init_cfs_rq(struct cfs_rq *c
 #ifdef CONFIG_FAIR_GROUP_SCHED
 	cfs_rq->rq = rq;
 	/* allow initial update_cfs_load() to truncate */
+#ifdef CONFIG_SMP
 	cfs_rq->load_stamp = 1;
 #endif
+#endif
 	cfs_rq->min_vruntime = (u64)(-(1LL << 20));
 }
#12 lore raw

[tip:sched/core] sched: Avoid expensive initial update_cfs_load(), on UP too

In reply to Paul Turner
Commit-ID:  2d65333ab09b9a11722d822231c85b9f251cfe9d
Gitweb:     http://git.kernel.org/tip/2d65333ab09b9a11722d822231c85b9f251cfe9d
Author:     Peter Zijlstra <[email protected]>
AuthorDate: Wed, 26 Jan 2011 13:36:03 +0100
Committer:  Ingo Molnar <[email protected]>
CommitDate: Wed, 26 Jan 2011 13:43:55 +0100

sched: Avoid expensive initial update_cfs_load(), on UP too

Fix the build on UP.

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

LKML-Reference: <[email protected]>

Signed-off-by Ingo Molnar <[email protected]>
---
 kernel/sched.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
kernel/sched.c +2 -0
index 3fc5749..1f6ca89 100644
@@ -7931,8 +7931,10 @@ static void init_cfs_rq(struct cfs_rq *cfs_rq, struct rq *rq)
 #ifdef CONFIG_FAIR_GROUP_SCHED
 	cfs_rq->rq = rq;
 	/* allow initial update_cfs_load() to truncate */
+#ifdef CONFIG_SMP
 	cfs_rq->load_stamp = 1;
 #endif
+#endif
 	cfs_rq->min_vruntime = (u64)(-(1LL << 20));
 }
#13 lore raw

[tip:sched/core] sched: Avoid expensive initial update_cfs_load(), on UP too

In reply to Paul Turner
Commit-ID:  6ea72f12069306b235151c5b05ac0cca7e1dedfa
Gitweb:     http://git.kernel.org/tip/6ea72f12069306b235151c5b05ac0cca7e1dedfa
Author:     Peter Zijlstra <[email protected]>
AuthorDate: Wed, 26 Jan 2011 13:36:03 +0100
Committer:  Ingo Molnar <[email protected]>
CommitDate: Thu, 27 Jan 2011 12:48:14 +0100

sched: Avoid expensive initial update_cfs_load(), on UP too

Fix the build on UP.

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

LKML-Reference: <[email protected]>

Signed-off-by Ingo Molnar <[email protected]>
---
 kernel/sched.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
kernel/sched.c +2 -0
index 78fa753..477e1bc 100644
@@ -7922,8 +7922,10 @@ static void init_cfs_rq(struct cfs_rq *cfs_rq, struct rq *rq)
 #ifdef CONFIG_FAIR_GROUP_SCHED
 	cfs_rq->rq = rq;
 	/* allow initial update_cfs_load() to truncate */
+#ifdef CONFIG_SMP
 	cfs_rq->load_stamp = 1;
 #endif
+#endif
 	cfs_rq->min_vruntime = (u64)(-(1LL << 20));
 }
#14 lore raw
In reply to Paul Turner
Quoted 4 lines · Paul Turner

A small set of scheduler patches for the CFS side of things.

Mostly buglet fixes with one or two associated clean-ups. Each patch should be (modulo any merge fuzz) independent.

Thanks Paul, these all look good, applied!