All of 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 related	[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 related	[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 related	[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 related	[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 related	[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 related	[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 related	[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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.