[PATCH] [0/4] SCHED: Trivial scheduler fixes

[PATCH] [0/4] SCHED: Trivial scheduler fixes

[Andi Kleen]

This patch series fixes a couple of minor issues I noticed
while reading the scheduler code.

Patches against 2.6.28-rc4

-Andi

[PATCH] [1/4] SCHED: cache task_hot result

[Andi Kleen]

Minor scheduler optimization: cache the result of task_hot() in 
can migrate task instead of computing it three times.

Signed-off-by: Andi Kleen <ak@linux.intel.com>

---
 kernel/sched.c |   11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

Index: linux-2.6.28-rc4-test/kernel/sched.c
===================================================================
--- linux-2.6.28-rc4-test.orig/kernel/sched.c	2008-11-10 08:50:24.000000000 +0100
+++ linux-2.6.28-rc4-test/kernel/sched.c	2008-11-12 12:32:52.000000000 +0100
@@ -2907,6 +2907,8 @@
 		     struct sched_domain *sd, enum cpu_idle_type idle,
 		     int *all_pinned)
 {
+	int hot;
+
 	/*
 	 * We do not migrate tasks that are:
 	 * 1) running (obviously), or
@@ -2930,10 +2932,11 @@
 	 * 2) too many balance attempts have failed.
 	 */
 
-	if (!task_hot(p, rq->clock, sd) ||
-			sd->nr_balance_failed > sd->cache_nice_tries) {
+	hot = task_hot(p, rq->clock, sd);
+
+	if (!hot || sd->nr_balance_failed > sd->cache_nice_tries) {
 #ifdef CONFIG_SCHEDSTATS
-		if (task_hot(p, rq->clock, sd)) {
+		if (hot) {
 			schedstat_inc(sd, lb_hot_gained[idle]);
 			schedstat_inc(p, se.nr_forced_migrations);
 		}
@@ -2941,7 +2944,7 @@
 		return 1;
 	}
 
-	if (task_hot(p, rq->clock, sd)) {
+	if (hot) {
 		schedstat_inc(p, se.nr_failed_migrations_hot);
 		return 0;
 	}

[PATCH] [2/4] SCHED: Mark all frequently used sysctls __read_mostly

[Andi Kleen]

There are more optimizations possible here, but that's for another
patch.

Signed-off-by: Andi Kleen <ak@linux.intel.com>

---
 kernel/sched.c |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Index: linux-2.6.28-rc4-test/kernel/sched.c
===================================================================
--- linux-2.6.28-rc4-test.orig/kernel/sched.c	2008-11-12 12:32:52.000000000 +0100
+++ linux-2.6.28-rc4-test/kernel/sched.c	2008-11-12 12:52:52.000000000 +0100
@@ -686,7 +686,7 @@
 #define SCHED_FEAT(name, enabled)	\
 	(1UL << __SCHED_FEAT_##name) * enabled |
 
-const_debug unsigned int sysctl_sched_features =
+const_debug unsigned int sysctl_sched_features __read_mostly =
 #include "sched_features.h"
 	0;
 
@@ -809,26 +809,26 @@
  * Number of tasks to iterate in a single balance run.
  * Limited because this is done with IRQs disabled.
  */
-const_debug unsigned int sysctl_sched_nr_migrate = 32;
+const_debug unsigned int sysctl_sched_nr_migrate __read_mostly = 32;
 
 /*
  * ratelimit for updating the group shares.
  * default: 0.25ms
  */
-unsigned int sysctl_sched_shares_ratelimit = 250000;
+unsigned int sysctl_sched_shares_ratelimit __read_mostly = 250000;
 
 /*
  * Inject some fuzzyness into changing the per-cpu group shares
  * this avoids remote rq-locks at the expense of fairness.
  * default: 4
  */
-unsigned int sysctl_sched_shares_thresh = 4;
+unsigned int sysctl_sched_shares_thresh __read_mostly = 4;
 
 /*
  * period over which we measure -rt task cpu usage in us.
  * default: 1s
  */
-unsigned int sysctl_sched_rt_period = 1000000;
+unsigned int sysctl_sched_rt_period __read_mostly = 1000000;
 
 static __read_mostly int scheduler_running;
 

[PATCH] [3/4] SCHED: don't inline idle_balance into schedule()

[Andi Kleen]

This function is not called under load, so it makes sense to out of line
it to get better register allocation in schedule() and make the assembler
easier to read.

Signed-off-by: Andi Kleen <ak@linux.intel.com>

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

Index: linux-2.6.28-rc4-test/kernel/sched.c
===================================================================
--- linux-2.6.28-rc4-test.orig/kernel/sched.c	2008-11-12 12:52:52.000000000 +0100
+++ linux-2.6.28-rc4-test/kernel/sched.c	2008-11-12 13:07:53.000000000 +0100
@@ -3696,7 +3696,7 @@
  * idle_balance is called by schedule() if this_cpu is about to become
  * idle. Attempts to pull tasks from other CPUs.
  */
-static void idle_balance(int this_cpu, struct rq *this_rq)
+static void noinline idle_balance(int this_cpu, struct rq *this_rq)
 {
 	struct sched_domain *sd;
 	int pulled_task = -1;

[PATCH] [4/4] SCHED: Use a lookup array in sd_level_to_string

[Andi Kleen]

Generates better/shorter code. Doesn't matter much because it's only
a SCHED_DEBUG function. 

Signed-off-by: Andi Kleen <ak@linux.intel.com>

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

Index: linux-2.6.28-rc4-test/kernel/sched.c
===================================================================
--- linux-2.6.28-rc4-test.orig/kernel/sched.c	2008-11-12 12:36:23.000000000 +0100
+++ linux-2.6.28-rc4-test/kernel/sched.c	2008-11-12 12:43:58.000000000 +0100
@@ -6629,24 +6629,17 @@
 
 static inline const char *sd_level_to_string(enum sched_domain_level lvl)
 {
-	switch (lvl) {
-	case SD_LV_NONE:
-			return "NONE";
-	case SD_LV_SIBLING:
-			return "SIBLING";
-	case SD_LV_MC:
-			return "MC";
-	case SD_LV_CPU:
-			return "CPU";
-	case SD_LV_NODE:
-			return "NODE";
-	case SD_LV_ALLNODES:
-			return "ALLNODES";
-	case SD_LV_MAX:
-			return "MAX";
-
-	}
-	return "MAX";
+	static const char *lva[] = {
+#define T(x) [SD_LV_ ## x ] = #T
+		T(NONE),
+		T(SIBLING),
+		T(MC),
+		T(CPU),
+		T(NODE),
+		T(ALLNODES),
+#undef T
+	};
+	return (unsigned)lvl < ARRAY_SIZE(lva) ? lva[lvl] : "MAX";
 }
 
 static int sched_domain_debug_one(struct sched_domain *sd, int cpu, int level,

Re: [PATCH] [4/4] SCHED: Use a lookup array in sd_level_to_string

[Johannes Weiner]

On Wed, Nov 12, 2008 at 01:45:12PM +0100, Andi Kleen wrote:
> 
> Generates better/shorter code. Doesn't matter much because it's only
> a SCHED_DEBUG function. 
> 
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> 
> ---
>  kernel/sched.c |   29 +++++++++++------------------
>  1 file changed, 11 insertions(+), 18 deletions(-)
> 
> Index: linux-2.6.28-rc4-test/kernel/sched.c
> ===================================================================
> --- linux-2.6.28-rc4-test.orig/kernel/sched.c	2008-11-12 12:36:23.000000000 +0100
> +++ linux-2.6.28-rc4-test/kernel/sched.c	2008-11-12 12:43:58.000000000 +0100
> @@ -6629,24 +6629,17 @@
>  
>  static inline const char *sd_level_to_string(enum sched_domain_level lvl)
>  {
> -	switch (lvl) {
> -	case SD_LV_NONE:
> -			return "NONE";
> -	case SD_LV_SIBLING:
> -			return "SIBLING";
> -	case SD_LV_MC:
> -			return "MC";
> -	case SD_LV_CPU:
> -			return "CPU";
> -	case SD_LV_NODE:
> -			return "NODE";
> -	case SD_LV_ALLNODES:
> -			return "ALLNODES";
> -	case SD_LV_MAX:
> -			return "MAX";
> -
> -	}
> -	return "MAX";
> +	static const char *lva[] = {
> +#define T(x) [SD_LV_ ## x ] = #T

Shouldn't                         ^ that be x?

	Hannes

Re: [PATCH] [4/4] SCHED: Use a lookup array in sd_level_to_string

[Andi Kleen]

On Wed, Nov 12, 2008 at 03:47:09PM +0100, Johannes Weiner wrote:
> > +#define T(x) [SD_LV_ ## x ] = #T
> 
> Shouldn't                         ^ that be x?

Yes good point. Thanks. Revised patch appended.

-Andi

---

SCHED: Use a lookup array in sd_level_to_string v2

Generates better/shorter code. Doesn't matter much because it's only
a SCHED_DEBUG function. 

v2: Fix stupid typo thanks to Johannes Weiner

Signed-off-by: Andi Kleen <ak@linux.intel.com>

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

Index: linux-2.6.28-rc4-test/kernel/sched.c
===================================================================
--- linux-2.6.28-rc4-test.orig/kernel/sched.c	2008-11-12 16:35:02.000000000 +0100
+++ linux-2.6.28-rc4-test/kernel/sched.c	2008-11-12 16:35:48.000000000 +0100
@@ -6629,24 +6629,17 @@
 
 static inline const char *sd_level_to_string(enum sched_domain_level lvl)
 {
-	switch (lvl) {
-	case SD_LV_NONE:
-			return "NONE";
-	case SD_LV_SIBLING:
-			return "SIBLING";
-	case SD_LV_MC:
-			return "MC";
-	case SD_LV_CPU:
-			return "CPU";
-	case SD_LV_NODE:
-			return "NODE";
-	case SD_LV_ALLNODES:
-			return "ALLNODES";
-	case SD_LV_MAX:
-			return "MAX";
-
-	}
-	return "MAX";
+	static const char *lva[] = {
+#define T(x) [SD_LV_ ## x ] = #x
+		T(NONE),
+		T(SIBLING),
+		T(MC),
+		T(CPU),
+		T(NODE),
+		T(ALLNODES),
+#undef T
+	};
+	return (unsigned)lvl < ARRAY_SIZE(lva) ? lva[lvl] : "MAX";
 }
 
 static int sched_domain_debug_one(struct sched_domain *sd, int cpu, int level,

Re: [PATCH] [4/4] SCHED: Use a lookup array in sd_level_to_string

[Ingo Molnar]

* Johannes Weiner <hannes@cmpxchg.org> wrote:

> On Wed, Nov 12, 2008 at 01:45:12PM +0100, Andi Kleen wrote:
> > 
> > Generates better/shorter code. Doesn't matter much because it's only
> > a SCHED_DEBUG function. 
> > 
> > Signed-off-by: Andi Kleen <ak@linux.intel.com>
> > 
> > ---
> >  kernel/sched.c |   29 +++++++++++------------------
> >  1 file changed, 11 insertions(+), 18 deletions(-)
> > 
> > Index: linux-2.6.28-rc4-test/kernel/sched.c
> > ===================================================================
> > --- linux-2.6.28-rc4-test.orig/kernel/sched.c	2008-11-12 12:36:23.000000000 +0100
> > +++ linux-2.6.28-rc4-test/kernel/sched.c	2008-11-12 12:43:58.000000000 +0100
> > @@ -6629,24 +6629,17 @@
> >  
> >  static inline const char *sd_level_to_string(enum sched_domain_level lvl)
> >  {
> > -	switch (lvl) {
> > -	case SD_LV_NONE:
> > -			return "NONE";
> > -	case SD_LV_SIBLING:
> > -			return "SIBLING";
> > -	case SD_LV_MC:
> > -			return "MC";
> > -	case SD_LV_CPU:
> > -			return "CPU";
> > -	case SD_LV_NODE:
> > -			return "NODE";
> > -	case SD_LV_ALLNODES:
> > -			return "ALLNODES";
> > -	case SD_LV_MAX:
> > -			return "MAX";
> > -
> > -	}
> > -	return "MAX";
> > +	static const char *lva[] = {
> > +#define T(x) [SD_LV_ ## x ] = #T
> 
> Shouldn't                         ^ that be x?

hasnt been tested i guess. Note that the patch is moot: an equivalent 
change has been done in the scheduler tree already, in a cleaner way. 
It's all in linux-next.

	Ingo