LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [patch 0/5] scheduler fixlets
@ 2011-01-22  4:44 Paul Turner
  2011-01-22  4:44 ` [patch 1/5] sched: fix sign under-flows in wake_affine Paul Turner
                   ` (5 more replies)
  0 siblings, 6 replies; 15+ messages in thread
From: Paul Turner @ 2011-01-22  4:44 UTC (permalink / raw)
  To: linux-kernel; +Cc: Peter Zijlstra, Ingo Molnar, Mike Galbraith

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



^ permalink raw reply	[flat|nested] 15+ messages in thread

* [patch 1/5] sched: fix sign under-flows in wake_affine
  2011-01-22  4:44 [patch 0/5] scheduler fixlets Paul Turner
@ 2011-01-22  4:44 ` Paul Turner
  2011-01-26 12:09   ` [tip:sched/core] sched: Fix " tip-bot for Paul Turner
  2011-01-22  4:45 ` [patch 2/5] sched: (cleanup) remove redundant cfs_rq checks Paul Turner
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 15+ messages in thread
From: Paul Turner @ 2011-01-22  4:44 UTC (permalink / raw)
  To: linux-kernel; +Cc: Peter Zijlstra, Ingo Molnar, Mike Galbraith

[-- Attachment #1: sched-signed_wake_affine.patch --]
[-- Type: text/plain, Size: 1246 bytes --]

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 <pjt@google.com>

---
 kernel/sched_fair.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Index: tip3/kernel/sched_fair.c
===================================================================
--- tip3.orig/kernel/sched_fair.c
+++ 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);



^ permalink raw reply	[flat|nested] 15+ messages in thread

* [patch 2/5] sched: (cleanup) remove redundant cfs_rq checks
  2011-01-22  4:44 [patch 0/5] scheduler fixlets Paul Turner
  2011-01-22  4:44 ` [patch 1/5] sched: fix sign under-flows in wake_affine Paul Turner
@ 2011-01-22  4:45 ` Paul Turner
  2011-01-26 12:10   ` [tip:sched/core] sched: Fix/remove " tip-bot for Paul Turner
  2011-01-22  4:45 ` [patch 3/5] sched: simplify update_cfs_shares parameters Paul Turner
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 15+ messages in thread
From: Paul Turner @ 2011-01-22  4:45 UTC (permalink / raw)
  To: linux-kernel; +Cc: Peter Zijlstra, Ingo Molnar, Mike Galbraith, Dan Carpenter

[-- Attachment #1: sched-remove_cfs_rq_checks.patch --]
[-- Type: text/plain, Size: 1252 bytes --]

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 <pjt@google.com>

---
 kernel/sched_fair.c |    5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

Index: tip3/kernel/sched_fair.c
===================================================================
--- tip3.orig/kernel/sched_fair.c
+++ 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)



^ permalink raw reply	[flat|nested] 15+ messages in thread

* [patch 3/5] sched: simplify update_cfs_shares parameters
  2011-01-22  4:44 [patch 0/5] scheduler fixlets Paul Turner
  2011-01-22  4:44 ` [patch 1/5] sched: fix sign under-flows in wake_affine Paul Turner
  2011-01-22  4:45 ` [patch 2/5] sched: (cleanup) remove redundant cfs_rq checks Paul Turner
@ 2011-01-22  4:45 ` Paul Turner
  2011-01-26 12:11   ` [tip:sched/core] sched: Simplify " tip-bot for Paul Turner
  2011-01-22  4:45 ` [patch 4/5] sched: use rq->clock_task instead of rq->clock for maintaining load averages Paul Turner
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 15+ messages in thread
From: Paul Turner @ 2011-01-22  4:45 UTC (permalink / raw)
  To: linux-kernel; +Cc: Peter Zijlstra, Ingo Molnar, Mike Galbraith

[-- Attachment #1: sched-clean_update_shares.patch --]
[-- Type: text/plain, Size: 3992 bytes --]

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 <pjt@google.com>

---
 kernel/sched.c      |    2 +-
 kernel/sched_fair.c |   22 +++++++++++-----------
 2 files changed, 12 insertions(+), 12 deletions(-)

Index: tip3/kernel/sched_fair.c
===================================================================
--- tip3.orig/kernel/sched_fair.c
+++ 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);
 
Index: tip3/kernel/sched.c
===================================================================
--- tip3.orig/kernel/sched.c
+++ 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);
 	}
 



^ permalink raw reply	[flat|nested] 15+ messages in thread

* [patch 4/5] sched: use rq->clock_task instead of rq->clock for maintaining load averages
  2011-01-22  4:44 [patch 0/5] scheduler fixlets Paul Turner
                   ` (2 preceding siblings ...)
  2011-01-22  4:45 ` [patch 3/5] sched: simplify update_cfs_shares parameters Paul Turner
@ 2011-01-22  4:45 ` Paul Turner
  2011-01-26 12:10   ` [tip:sched/core] sched: Use rq->clock_task instead of rq->clock for correctly " tip-bot for Paul Turner
  2011-01-22  4:45 ` [patch 5/5] sched: avoid expensive initial update_cfs_load() Paul Turner
  2011-01-24 10:17 ` [patch 0/5] scheduler fixlets Peter Zijlstra
  5 siblings, 1 reply; 15+ messages in thread
From: Paul Turner @ 2011-01-22  4:45 UTC (permalink / raw)
  To: linux-kernel; +Cc: Peter Zijlstra, Ingo Molnar, Mike Galbraith

[-- Attachment #1: sched-use_clock_task.patch --]
[-- Type: text/plain, Size: 836 bytes --]

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 <pjt@google.com>

---
 kernel/sched_fair.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: tip3/kernel/sched_fair.c
===================================================================
--- tip3.orig/kernel/sched_fair.c
+++ 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 */



^ permalink raw reply	[flat|nested] 15+ messages in thread

* [patch 5/5] sched: avoid expensive initial update_cfs_load()
  2011-01-22  4:44 [patch 0/5] scheduler fixlets Paul Turner
                   ` (3 preceding siblings ...)
  2011-01-22  4:45 ` [patch 4/5] sched: use rq->clock_task instead of rq->clock for maintaining load averages Paul Turner
@ 2011-01-22  4:45 ` Paul Turner
  2011-01-26 12:11   ` [tip:sched/core] sched: Avoid " tip-bot for Paul Turner
                     ` (2 more replies)
  2011-01-24 10:17 ` [patch 0/5] scheduler fixlets Peter Zijlstra
  5 siblings, 3 replies; 15+ messages in thread
From: Paul Turner @ 2011-01-22  4:45 UTC (permalink / raw)
  To: linux-kernel; +Cc: Peter Zijlstra, Ingo Molnar, Mike Galbraith

[-- Attachment #1: sched-fix_first_update_load.patch --]
[-- Type: text/plain, Size: 1648 bytes --]

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 <pjt@google.com>

---
 kernel/sched.c      |    2 ++
 kernel/sched_fair.c |    1 +
 2 files changed, 3 insertions(+)

Index: tip3/kernel/sched.c
===================================================================
--- tip3.orig/kernel/sched.c
+++ 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));
 }
Index: tip3/kernel/sched_fair.c
===================================================================
--- tip3.orig/kernel/sched_fair.c
+++ 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;



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [patch 0/5] scheduler fixlets
  2011-01-22  4:44 [patch 0/5] scheduler fixlets Paul Turner
                   ` (4 preceding siblings ...)
  2011-01-22  4:45 ` [patch 5/5] sched: avoid expensive initial update_cfs_load() Paul Turner
@ 2011-01-24 10:17 ` Peter Zijlstra
  5 siblings, 0 replies; 15+ messages in thread
From: Peter Zijlstra @ 2011-01-24 10:17 UTC (permalink / raw)
  To: Paul Turner; +Cc: linux-kernel, Ingo Molnar, Mike Galbraith

On Fri, 2011-01-21 at 20:44 -0800, Paul Turner wrote:
> 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!

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [tip:sched/core] sched: Fix sign under-flows in wake_affine
  2011-01-22  4:44 ` [patch 1/5] sched: fix sign under-flows in wake_affine Paul Turner
@ 2011-01-26 12:09   ` tip-bot for Paul Turner
  0 siblings, 0 replies; 15+ messages in thread
From: tip-bot for Paul Turner @ 2011-01-26 12:09 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, mingo, a.p.zijlstra, pjt, tglx, mingo

Commit-ID:  e37b6a7b27b400c3aa488db8c6629a05095bc79c
Gitweb:     http://git.kernel.org/tip/e37b6a7b27b400c3aa488db8c6629a05095bc79c
Author:     Paul Turner <pjt@google.com>
AuthorDate: Fri, 21 Jan 2011 20:44:59 -0800
Committer:  Ingo Molnar <mingo@elte.hu>
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 <pjt@google.com>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
LKML-Reference: <20110122044851.734245014@google.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 kernel/sched_fair.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/sched_fair.c b/kernel/sched_fair.c
index 3547699..ccecfec 100644
--- a/kernel/sched_fair.c
+++ b/kernel/sched_fair.c
@@ -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);

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [tip:sched/core] sched: Fix/remove redundant cfs_rq checks
  2011-01-22  4:45 ` [patch 2/5] sched: (cleanup) remove redundant cfs_rq checks Paul Turner
@ 2011-01-26 12:10   ` tip-bot for Paul Turner
  0 siblings, 0 replies; 15+ messages in thread
From: tip-bot for Paul Turner @ 2011-01-26 12:10 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, mingo, a.p.zijlstra, pjt, tglx, mingo

Commit-ID:  b815f1963e47b9b69bb17e0588bd5af5b1114ae0
Gitweb:     http://git.kernel.org/tip/b815f1963e47b9b69bb17e0588bd5af5b1114ae0
Author:     Paul Turner <pjt@google.com>
AuthorDate: Fri, 21 Jan 2011 20:45:00 -0800
Committer:  Ingo Molnar <mingo@elte.hu>
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 <pjt@google.com>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
LKML-Reference: <20110122044851.825284940@google.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 kernel/sched_fair.c |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/kernel/sched_fair.c b/kernel/sched_fair.c
index ccecfec..1997383 100644
--- a/kernel/sched_fair.c
+++ b/kernel/sched_fair.c
@@ -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)

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [tip:sched/core] sched: Use rq->clock_task instead of rq->clock for correctly maintaining load averages
  2011-01-22  4:45 ` [patch 4/5] sched: use rq->clock_task instead of rq->clock for maintaining load averages Paul Turner
@ 2011-01-26 12:10   ` tip-bot for Paul Turner
  0 siblings, 0 replies; 15+ messages in thread
From: tip-bot for Paul Turner @ 2011-01-26 12:10 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, mingo, a.p.zijlstra, pjt, tglx, mingo

Commit-ID:  05ca62c6ca17f39b88fa956d5ebc1fa6e93ad5e3
Gitweb:     http://git.kernel.org/tip/05ca62c6ca17f39b88fa956d5ebc1fa6e93ad5e3
Author:     Paul Turner <pjt@google.com>
AuthorDate: Fri, 21 Jan 2011 20:45:02 -0800
Committer:  Ingo Molnar <mingo@elte.hu>
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 <pjt@google.com>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
LKML-Reference: <20110122044852.007092349@google.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 kernel/sched_fair.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/kernel/sched_fair.c b/kernel/sched_fair.c
index 1997383..0c26e2d 100644
--- a/kernel/sched_fair.c
+++ b/kernel/sched_fair.c
@@ -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 */

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [tip:sched/core] sched: Simplify update_cfs_shares parameters
  2011-01-22  4:45 ` [patch 3/5] sched: simplify update_cfs_shares parameters Paul Turner
@ 2011-01-26 12:11   ` tip-bot for Paul Turner
  0 siblings, 0 replies; 15+ messages in thread
From: tip-bot for Paul Turner @ 2011-01-26 12:11 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, mingo, a.p.zijlstra, pjt, tglx, mingo

Commit-ID:  6d5ab2932a21ea54406ab95c43ecff90a3eddfda
Gitweb:     http://git.kernel.org/tip/6d5ab2932a21ea54406ab95c43ecff90a3eddfda
Author:     Paul Turner <pjt@google.com>
AuthorDate: Fri, 21 Jan 2011 20:45:01 -0800
Committer:  Ingo Molnar <mingo@elte.hu>
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 <pjt@google.com>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
LKML-Reference: <20110122044851.915214637@google.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 kernel/sched.c      |    2 +-
 kernel/sched_fair.c |   30 ++++++++++++++----------------
 2 files changed, 15 insertions(+), 17 deletions(-)

diff --git a/kernel/sched.c b/kernel/sched.c
index 18d38e4..e0fa3ff 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -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);
 	}
 
diff --git a/kernel/sched_fair.c b/kernel/sched_fair.c
index 0c26e2d..0c550c8 100644
--- a/kernel/sched_fair.c
+++ b/kernel/sched_fair.c
@@ -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);
 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [tip:sched/core] sched: Avoid expensive initial update_cfs_load()
  2011-01-22  4:45 ` [patch 5/5] sched: avoid expensive initial update_cfs_load() Paul Turner
@ 2011-01-26 12:11   ` tip-bot for Paul Turner
  2011-01-26 12:36     ` Peter Zijlstra
  2011-01-26 12:45   ` [tip:sched/core] sched: Avoid expensive initial update_cfs_load(), on UP too tip-bot for Peter Zijlstra
  2011-01-27 11:58   ` tip-bot for Peter Zijlstra
  2 siblings, 1 reply; 15+ messages in thread
From: tip-bot for Paul Turner @ 2011-01-26 12:11 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, mingo, a.p.zijlstra, pjt, tglx, mingo

Commit-ID:  f07333bf6ee66d9b49286cec4371cf375e745b7a
Gitweb:     http://git.kernel.org/tip/f07333bf6ee66d9b49286cec4371cf375e745b7a
Author:     Paul Turner <pjt@google.com>
AuthorDate: Fri, 21 Jan 2011 20:45:03 -0800
Committer:  Ingo Molnar <mingo@elte.hu>
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 <pjt@google.com>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
LKML-Reference: <20110122044852.102126037@google.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 kernel/sched.c      |    2 ++
 kernel/sched_fair.c |    1 +
 2 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/kernel/sched.c b/kernel/sched.c
index e0fa3ff..6820b5b 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -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));
 }
diff --git a/kernel/sched_fair.c b/kernel/sched_fair.c
index 0c550c8..4cbc912 100644
--- a/kernel/sched_fair.c
+++ b/kernel/sched_fair.c
@@ -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;

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [tip:sched/core] sched: Avoid expensive initial update_cfs_load()
  2011-01-26 12:11   ` [tip:sched/core] sched: Avoid " tip-bot for Paul Turner
@ 2011-01-26 12:36     ` Peter Zijlstra
  0 siblings, 0 replies; 15+ messages in thread
From: Peter Zijlstra @ 2011-01-26 12:36 UTC (permalink / raw)
  To: mingo, hpa, linux-kernel, pjt, tglx, mingo; +Cc: linux-tip-commits

On Wed, 2011-01-26 at 12:11 +0000, tip-bot for Paul Turner wrote:
> index e0fa3ff..6820b5b 100644
> --- a/kernel/sched.c
> +++ b/kernel/sched.c
> @@ -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));
>  } 


That wants a fix to build on UP,

---
Index: linux-2.6/kernel/sched.c
===================================================================
--- linux-2.6.orig/kernel/sched.c
+++ 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));
 }
 


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [tip:sched/core] sched: Avoid expensive initial update_cfs_load(), on UP too
  2011-01-22  4:45 ` [patch 5/5] sched: avoid expensive initial update_cfs_load() Paul Turner
  2011-01-26 12:11   ` [tip:sched/core] sched: Avoid " tip-bot for Paul Turner
@ 2011-01-26 12:45   ` tip-bot for Peter Zijlstra
  2011-01-27 11:58   ` tip-bot for Peter Zijlstra
  2 siblings, 0 replies; 15+ messages in thread
From: tip-bot for Peter Zijlstra @ 2011-01-26 12:45 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, mingo, a.p.zijlstra, pjt, tglx, mingo

Commit-ID:  2d65333ab09b9a11722d822231c85b9f251cfe9d
Gitweb:     http://git.kernel.org/tip/2d65333ab09b9a11722d822231c85b9f251cfe9d
Author:     Peter Zijlstra <a.p.zijlstra@chello.nl>
AuthorDate: Wed, 26 Jan 2011 13:36:03 +0100
Committer:  Ingo Molnar <mingo@elte.hu>
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 <a.p.zijlstra@chello.nl>
Cc: Paul Turner <pjt@google.com>
LKML-Reference: <20110122044852.102126037@google.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 kernel/sched.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/kernel/sched.c b/kernel/sched.c
index 3fc5749..1f6ca89 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -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));
 }
 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [tip:sched/core] sched: Avoid expensive initial update_cfs_load(), on UP too
  2011-01-22  4:45 ` [patch 5/5] sched: avoid expensive initial update_cfs_load() Paul Turner
  2011-01-26 12:11   ` [tip:sched/core] sched: Avoid " tip-bot for Paul Turner
  2011-01-26 12:45   ` [tip:sched/core] sched: Avoid expensive initial update_cfs_load(), on UP too tip-bot for Peter Zijlstra
@ 2011-01-27 11:58   ` tip-bot for Peter Zijlstra
  2 siblings, 0 replies; 15+ messages in thread
From: tip-bot for Peter Zijlstra @ 2011-01-27 11:58 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, mingo, a.p.zijlstra, pjt, tglx, mingo

Commit-ID:  6ea72f12069306b235151c5b05ac0cca7e1dedfa
Gitweb:     http://git.kernel.org/tip/6ea72f12069306b235151c5b05ac0cca7e1dedfa
Author:     Peter Zijlstra <a.p.zijlstra@chello.nl>
AuthorDate: Wed, 26 Jan 2011 13:36:03 +0100
Committer:  Ingo Molnar <mingo@elte.hu>
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 <a.p.zijlstra@chello.nl>
Cc: Paul Turner <pjt@google.com>
LKML-Reference: <20110122044852.102126037@google.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 kernel/sched.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/kernel/sched.c b/kernel/sched.c
index 78fa753..477e1bc 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -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));
 }
 

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2011-01-27 11:59 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-01-22  4:44 [patch 0/5] scheduler fixlets Paul Turner
2011-01-22  4:44 ` [patch 1/5] sched: fix sign under-flows in wake_affine Paul Turner
2011-01-26 12:09   ` [tip:sched/core] sched: Fix " tip-bot for Paul Turner
2011-01-22  4:45 ` [patch 2/5] sched: (cleanup) remove redundant cfs_rq checks Paul Turner
2011-01-26 12:10   ` [tip:sched/core] sched: Fix/remove " tip-bot for Paul Turner
2011-01-22  4:45 ` [patch 3/5] sched: simplify update_cfs_shares parameters Paul Turner
2011-01-26 12:11   ` [tip:sched/core] sched: Simplify " tip-bot for Paul Turner
2011-01-22  4:45 ` [patch 4/5] sched: use rq->clock_task instead of rq->clock for maintaining load averages Paul Turner
2011-01-26 12:10   ` [tip:sched/core] sched: Use rq->clock_task instead of rq->clock for correctly " tip-bot for Paul Turner
2011-01-22  4:45 ` [patch 5/5] sched: avoid expensive initial update_cfs_load() Paul Turner
2011-01-26 12:11   ` [tip:sched/core] sched: Avoid " tip-bot for Paul Turner
2011-01-26 12:36     ` Peter Zijlstra
2011-01-26 12:45   ` [tip:sched/core] sched: Avoid expensive initial update_cfs_load(), on UP too tip-bot for Peter Zijlstra
2011-01-27 11:58   ` tip-bot for Peter Zijlstra
2011-01-24 10:17 ` [patch 0/5] scheduler fixlets Peter Zijlstra

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).