[PATCH 0/5] ppc64 scheduler fixes

[PATCH 0/5] ppc64 scheduler fixes

[Anton Blanchard]

Here are a set of ppc64 scheduler fixes that help with some
multi node performance issues.

[PATCH 1/5] powerpc/numa: Enable SD_WAKE_AFFINE in node definition

[Anton Blanchard]

[-- Attachment #1: powerpc_numa_wake_affine --]
[-- Type: text/plain, Size: 1349 bytes --]

When chasing a performance issue on ppc64, I noticed tasks
communicating via a pipe would often end up on different nodes.

It turns out SD_WAKE_AFFINE is not set in our node defition. Commit
9fcd18c9e63e (sched: re-tune balancing) enabled SD_WAKE_AFFINE
in the node definition for x86 and we need a similar change for
ppc64.

I used lmbench lat_ctx and perf bench pipe to verify this fix. Each
benchmark was run 10 times and the average taken.


lmbench lat_ctx:

before:  66565 ops/sec
after:  204700 ops/sec

3.1x faster


perf bench pipe:

before: 5.6570 usecs
after:  1.3470 usecs

4.2x faster


Signed-off-by: Anton Blanchard <anton@samba.org>
---

Cc-ing arch maintainers who might need to look at their SD_NODE_INIT
definitions

Index: linux-2.6-work/arch/powerpc/include/asm/topology.h
===================================================================
--- linux-2.6-work.orig/arch/powerpc/include/asm/topology.h	2011-07-18 16:24:55.639949552 +1000
+++ linux-2.6-work/arch/powerpc/include/asm/topology.h	2011-07-18 16:25:02.630074557 +1000
@@ -73,7 +73,7 @@ static inline int pcibus_to_node(struct
 				| 1*SD_BALANCE_EXEC			\
 				| 1*SD_BALANCE_FORK			\
 				| 0*SD_BALANCE_WAKE			\
-				| 0*SD_WAKE_AFFINE			\
+				| 1*SD_WAKE_AFFINE			\
 				| 0*SD_PREFER_LOCAL			\
 				| 0*SD_SHARE_CPUPOWER			\
 				| 0*SD_POWERSAVINGS_BALANCE		\

[PATCH 1/5] powerpc/numa: Enable SD_WAKE_AFFINE in node definition

[Anton Blanchard]

When chasing a performance issue on ppc64, I noticed tasks
communicating via a pipe would often end up on different nodes.

It turns out SD_WAKE_AFFINE is not set in our node defition. Commit
9fcd18c9e63e (sched: re-tune balancing) enabled SD_WAKE_AFFINE
in the node definition for x86 and we need a similar change for
ppc64.

I used lmbench lat_ctx and perf bench pipe to verify this fix. Each
benchmark was run 10 times and the average taken.


lmbench lat_ctx:

before:  66565 ops/sec
after:  204700 ops/sec

3.1x faster


perf bench pipe:

before: 5.6570 usecs
after:  1.3470 usecs

4.2x faster


Signed-off-by: Anton Blanchard <anton@samba.org>
---

Cc-ing arch maintainers who might need to look at their SD_NODE_INIT
definitions

Index: linux-2.6-work/arch/powerpc/include/asm/topology.h
===================================================================
--- linux-2.6-work.orig/arch/powerpc/include/asm/topology.h	2011-07-18 16:24:55.639949552 +1000
+++ linux-2.6-work/arch/powerpc/include/asm/topology.h	2011-07-18 16:25:02.630074557 +1000
@@ -73,7 +73,7 @@ static inline int pcibus_to_node(struct
 				| 1*SD_BALANCE_EXEC			\
 				| 1*SD_BALANCE_FORK			\
 				| 0*SD_BALANCE_WAKE			\
-				| 0*SD_WAKE_AFFINE			\
+				| 1*SD_WAKE_AFFINE			\
 				| 0*SD_PREFER_LOCAL			\
 				| 0*SD_SHARE_CPUPOWER			\
 				| 0*SD_POWERSAVINGS_BALANCE		\

[PATCH 2/5] sched: Allow SD_NODES_PER_DOMAIN to be overridden

[-- Attachment #1: move_SD_NODES_PER_DOMAIN --]
[-- Type: text/plain, Size: 1156 bytes --]

We want to override the default value of SD_NODES_PER_DOMAIN on ppc64,
so move it into linux/topology.h.

Signed-off-by: Anton Blanchard <anton@samba.org> 
---

Index: linux-2.6-work/include/linux/topology.h
===================================================================
--- linux-2.6-work.orig/include/linux/topology.h	2011-07-25 11:20:02.588717796 +1000
+++ linux-2.6-work/include/linux/topology.h	2011-07-25 11:26:50.616468376 +1000
@@ -201,6 +201,10 @@ int arch_update_cpu_topology(void);
 	.balance_interval	= 64,					\
 }
 
+#ifndef SD_NODES_PER_DOMAIN
+#define SD_NODES_PER_DOMAIN 16
+#endif
+
 #ifdef CONFIG_SCHED_BOOK
 #ifndef SD_BOOK_INIT
 #error Please define an appropriate SD_BOOK_INIT in include/asm/topology.h!!!
Index: linux-2.6-work/kernel/sched.c
===================================================================
--- linux-2.6-work.orig/kernel/sched.c	2011-07-25 11:20:09.538850173 +1000
+++ linux-2.6-work/kernel/sched.c	2011-07-25 11:26:50.626468565 +1000
@@ -6938,8 +6938,6 @@ static int __init isolated_cpu_setup(cha
 
 __setup("isolcpus=", isolated_cpu_setup);
 
-#define SD_NODES_PER_DOMAIN 16
-
 #ifdef CONFIG_NUMA
 
 /**

[PATCH 2/5] sched: Allow SD_NODES_PER_DOMAIN to be overridden

[Anton Blanchard]

We want to override the default value of SD_NODES_PER_DOMAIN on ppc64,
so move it into linux/topology.h.

Signed-off-by: Anton Blanchard <anton@samba.org> 
---

Index: linux-2.6-work/include/linux/topology.h
===================================================================
--- linux-2.6-work.orig/include/linux/topology.h	2011-07-25 11:20:02.588717796 +1000
+++ linux-2.6-work/include/linux/topology.h	2011-07-25 11:26:50.616468376 +1000
@@ -201,6 +201,10 @@ int arch_update_cpu_topology(void);
 	.balance_interval	= 64,					\
 }
 
+#ifndef SD_NODES_PER_DOMAIN
+#define SD_NODES_PER_DOMAIN 16
+#endif
+
 #ifdef CONFIG_SCHED_BOOK
 #ifndef SD_BOOK_INIT
 #error Please define an appropriate SD_BOOK_INIT in include/asm/topology.h!!!
Index: linux-2.6-work/kernel/sched.c
===================================================================
--- linux-2.6-work.orig/kernel/sched.c	2011-07-25 11:20:09.538850173 +1000
+++ linux-2.6-work/kernel/sched.c	2011-07-25 11:26:50.626468565 +1000
@@ -6938,8 +6938,6 @@ static int __init isolated_cpu_setup(cha
 
 __setup("isolcpus=", isolated_cpu_setup);
 
-#define SD_NODES_PER_DOMAIN 16
-
 #ifdef CONFIG_NUMA
 
 /**

[PATCH 3/5] powerpc/numa: Increase SD_NODES_PER_DOMAIN to 32.

[Anton Blanchard]

[-- Attachment #1: override_SD_NODES_PER_DOMAIN --]
[-- Type: text/plain, Size: 938 bytes --]

The largest POWER7 boxes have 32 nodes. SD_NODES_PER_DOMAIN groups
nodes into chunks of 16 and adds a global balancing domain
(SD_ALLNODES) above it.

If we bump SD_NODES_PER_DOMAIN to 32, then we avoid this extra
level of balancing on our largest boxes.

Signed-off-by: Anton Blanchard <anton@samba.org> 
---

Index: linux-2.6-work/arch/powerpc/include/asm/topology.h
===================================================================
--- linux-2.6-work.orig/arch/powerpc/include/asm/topology.h	2011-07-25 11:43:24.954093179 +1000
+++ linux-2.6-work/arch/powerpc/include/asm/topology.h	2011-07-25 11:43:31.274205122 +1000
@@ -28,6 +28,12 @@ struct device_node;
  */
 #define RECLAIM_DISTANCE 10
 
+/*
+ * Avoid creating an extra level of balancing (SD_ALLNODES) on the largest
+ * POWER7 boxes which have a maximum of 32 nodes.
+ */
+#define SD_NODES_PER_DOMAIN 32
+
 #include <asm/mmzone.h>
 
 static inline int cpu_to_node(int cpu)

[PATCH 3/5] powerpc/numa: Increase SD_NODES_PER_DOMAIN to 32.

[Anton Blanchard]

The largest POWER7 boxes have 32 nodes. SD_NODES_PER_DOMAIN groups
nodes into chunks of 16 and adds a global balancing domain
(SD_ALLNODES) above it.

If we bump SD_NODES_PER_DOMAIN to 32, then we avoid this extra
level of balancing on our largest boxes.

Signed-off-by: Anton Blanchard <anton@samba.org> 
---

Index: linux-2.6-work/arch/powerpc/include/asm/topology.h
===================================================================
--- linux-2.6-work.orig/arch/powerpc/include/asm/topology.h	2011-07-25 11:43:24.954093179 +1000
+++ linux-2.6-work/arch/powerpc/include/asm/topology.h	2011-07-25 11:43:31.274205122 +1000
@@ -28,6 +28,12 @@ struct device_node;
  */
 #define RECLAIM_DISTANCE 10
 
+/*
+ * Avoid creating an extra level of balancing (SD_ALLNODES) on the largest
+ * POWER7 boxes which have a maximum of 32 nodes.
+ */
+#define SD_NODES_PER_DOMAIN 32
+
 #include <asm/mmzone.h>
 
 static inline int cpu_to_node(int cpu)

[PATCH 4/5] powerpc/numa: Disable NEWIDLE balancing at node level

[Anton Blanchard]

[-- Attachment #1: nodes_no_newidle --]
[-- Type: text/plain, Size: 989 bytes --]

On big POWER7 boxes we see large amounts of CPU time in system
processes like workqueue and watchdog kernel threads.

We currently rebalance the entire machine each time a task goes
idle and this is very expensive on large machines. Disable newidle
balancing at the node level and rely on the scheduler tick to
rebalance across nodes.

Signed-off-by: Anton Blanchard <anton@samba.org>
---

Index: linux-2.6-work/arch/powerpc/include/asm/topology.h
===================================================================
--- linux-2.6-work.orig/arch/powerpc/include/asm/topology.h	2011-07-25 12:14:25.448671947 +1000
+++ linux-2.6-work/arch/powerpc/include/asm/topology.h	2011-07-25 12:14:26.568692651 +1000
@@ -75,7 +75,7 @@ static inline int pcibus_to_node(struct
 	.forkexec_idx		= 0,					\
 									\
 	.flags			= 1*SD_LOAD_BALANCE			\
-				| 1*SD_BALANCE_NEWIDLE			\
+				| 0*SD_BALANCE_NEWIDLE			\
 				| 1*SD_BALANCE_EXEC			\
 				| 1*SD_BALANCE_FORK			\
 				| 0*SD_BALANCE_WAKE			\

[PATCH 4/5] powerpc/numa: Disable NEWIDLE balancing at node level

[Anton Blanchard]

On big POWER7 boxes we see large amounts of CPU time in system
processes like workqueue and watchdog kernel threads.

We currently rebalance the entire machine each time a task goes
idle and this is very expensive on large machines. Disable newidle
balancing at the node level and rely on the scheduler tick to
rebalance across nodes.

Signed-off-by: Anton Blanchard <anton@samba.org>
---

Index: linux-2.6-work/arch/powerpc/include/asm/topology.h
===================================================================
--- linux-2.6-work.orig/arch/powerpc/include/asm/topology.h	2011-07-25 12:14:25.448671947 +1000
+++ linux-2.6-work/arch/powerpc/include/asm/topology.h	2011-07-25 12:14:26.568692651 +1000
@@ -75,7 +75,7 @@ static inline int pcibus_to_node(struct
 	.forkexec_idx		= 0,					\
 									\
 	.flags			= 1*SD_LOAD_BALANCE			\
-				| 1*SD_BALANCE_NEWIDLE			\
+				| 0*SD_BALANCE_NEWIDLE			\
 				| 1*SD_BALANCE_EXEC			\
 				| 1*SD_BALANCE_FORK			\
 				| 0*SD_BALANCE_WAKE			\

[PATCH 5/5] powerpc/numa: Remove duplicate RECLAIM_DISTANCE definition

[Anton Blanchard]

[-- Attachment #1: move_duplicate_distance --]
[-- Type: text/plain, Size: 1080 bytes --]

We have two identical definitions of RECLAIM_DISTANCE, looks like
the patch got applied twice. Remove one.

Signed-off-by: Anton Blanchard <anton@samba.org> 
---

Index: linux-2.6-work/arch/powerpc/include/asm/topology.h
===================================================================
--- linux-2.6-work.orig/arch/powerpc/include/asm/topology.h	2011-07-25 12:15:33.059921510 +1000
+++ linux-2.6-work/arch/powerpc/include/asm/topology.h	2011-07-25 12:15:46.750174446 +1000
@@ -19,16 +19,6 @@ struct device_node;
 #define RECLAIM_DISTANCE 10
 
 /*
- * Before going off node we want the VM to try and reclaim from the local
- * node. It does this if the remote distance is larger than RECLAIM_DISTANCE.
- * With the default REMOTE_DISTANCE of 20 and the default RECLAIM_DISTANCE of
- * 20, we never reclaim and go off node straight away.
- *
- * To fix this we choose a smaller value of RECLAIM_DISTANCE.
- */
-#define RECLAIM_DISTANCE 10
-
-/*
  * Avoid creating an extra level of balancing (SD_ALLNODES) on the largest
  * POWER7 boxes which have a maximum of 32 nodes.
  */

[PATCH 5/5] powerpc/numa: Remove duplicate RECLAIM_DISTANCE definition

[Anton Blanchard]

We have two identical definitions of RECLAIM_DISTANCE, looks like
the patch got applied twice. Remove one.

Signed-off-by: Anton Blanchard <anton@samba.org> 
---

Index: linux-2.6-work/arch/powerpc/include/asm/topology.h
===================================================================
--- linux-2.6-work.orig/arch/powerpc/include/asm/topology.h	2011-07-25 12:15:33.059921510 +1000
+++ linux-2.6-work/arch/powerpc/include/asm/topology.h	2011-07-25 12:15:46.750174446 +1000
@@ -19,16 +19,6 @@ struct device_node;
 #define RECLAIM_DISTANCE 10
 
 /*
- * Before going off node we want the VM to try and reclaim from the local
- * node. It does this if the remote distance is larger than RECLAIM_DISTANCE.
- * With the default REMOTE_DISTANCE of 20 and the default RECLAIM_DISTANCE of
- * 20, we never reclaim and go off node straight away.
- *
- * To fix this we choose a smaller value of RECLAIM_DISTANCE.
- */
-#define RECLAIM_DISTANCE 10
-
-/*
  * Avoid creating an extra level of balancing (SD_ALLNODES) on the largest
  * POWER7 boxes which have a maximum of 32 nodes.
  */

Re: [PATCH 0/5] ppc64 scheduler fixes

[Peter Zijlstra]

On Mon, 2011-07-25 at 12:33 +1000, Anton Blanchard wrote:
> Here are a set of ppc64 scheduler fixes that help with some
> multi node performance issues.

They look fine to me. I'll probably ping you when I'll rip out all that
SD_NODES_PER_DOMAIN crap for good, but until then I'm fine with you
fiddling it for ppc64.

Acked-by: Peter Zijlstra <a.p.zijlstra@chello.nl>

Re: [PATCH 0/5] ppc64 scheduler fixes

[Peter Zijlstra]

On Mon, 2011-07-25 at 12:33 +1000, Anton Blanchard wrote:
> Here are a set of ppc64 scheduler fixes that help with some
> multi node performance issues.

They look fine to me. I'll probably ping you when I'll rip out all that
SD_NODES_PER_DOMAIN crap for good, but until then I'm fine with you
fiddling it for ppc64.

Acked-by: Peter Zijlstra <a.p.zijlstra@chello.nl>

Re: [PATCH 0/5] ppc64 scheduler fixes

[Anton Blanchard]

Hi Peter,

> On Mon, 2011-07-25 at 12:33 +1000, Anton Blanchard wrote:
> > Here are a set of ppc64 scheduler fixes that help with some
> > multi node performance issues.
> 
> They look fine to me. I'll probably ping you when I'll rip out all
> that SD_NODES_PER_DOMAIN crap for good, but until then I'm fine with
> you fiddling it for ppc64.
> 
> Acked-by: Peter Zijlstra <a.p.zijlstra@chello.nl>

Are you OK if Ben takes this, or would you prefer to send it via the
scheduler tree?

Anton
--

[2/5] sched: Allow SD_NODES_PER_DOMAIN to be overridden

We want to override the default value of SD_NODES_PER_DOMAIN on ppc64,
so move it into linux/topology.h.

Signed-off-by: Anton Blanchard <anton@samba.org>

---

Index: linux-2.6-work/include/linux/topology.h
===================================================================
--- linux-2.6-work.orig/include/linux/topology.h	2011-07-25 11:20:02.588717796 +1000
+++ linux-2.6-work/include/linux/topology.h	2011-07-25 11:26:50.616468376 +1000
@@ -201,6 +201,10 @@ int arch_update_cpu_topology(void);
 	.balance_interval	= 64,					\
 }
 
+#ifndef SD_NODES_PER_DOMAIN
+#define SD_NODES_PER_DOMAIN 16
+#endif
+
 #ifdef CONFIG_SCHED_BOOK
 #ifndef SD_BOOK_INIT
 #error Please define an appropriate SD_BOOK_INIT in include/asm/topology.h!!!
Index: linux-2.6-work/kernel/sched.c
===================================================================
--- linux-2.6-work.orig/kernel/sched.c	2011-07-25 11:20:09.538850173 +1000
+++ linux-2.6-work/kernel/sched.c	2011-07-25 11:26:50.626468565 +1000
@@ -6938,8 +6938,6 @@ static int __init isolated_cpu_setup(cha
 
 __setup("isolcpus=", isolated_cpu_setup);
 
-#define SD_NODES_PER_DOMAIN 16
-
 #ifdef CONFIG_NUMA
 
 /**

Re: [PATCH 0/5] ppc64 scheduler fixes

[Anton Blanchard]

Hi Peter,

> On Mon, 2011-07-25 at 12:33 +1000, Anton Blanchard wrote:
> > Here are a set of ppc64 scheduler fixes that help with some
> > multi node performance issues.
> 
> They look fine to me. I'll probably ping you when I'll rip out all
> that SD_NODES_PER_DOMAIN crap for good, but until then I'm fine with
> you fiddling it for ppc64.
> 
> Acked-by: Peter Zijlstra <a.p.zijlstra@chello.nl>

Are you OK if Ben takes this, or would you prefer to send it via the
scheduler tree?

Anton
--

[2/5] sched: Allow SD_NODES_PER_DOMAIN to be overridden

We want to override the default value of SD_NODES_PER_DOMAIN on ppc64,
so move it into linux/topology.h.

Signed-off-by: Anton Blanchard <anton@samba.org>

---

Index: linux-2.6-work/include/linux/topology.h
===================================================================
--- linux-2.6-work.orig/include/linux/topology.h	2011-07-25 11:20:02.588717796 +1000
+++ linux-2.6-work/include/linux/topology.h	2011-07-25 11:26:50.616468376 +1000
@@ -201,6 +201,10 @@ int arch_update_cpu_topology(void);
 	.balance_interval	= 64,					\
 }
 
+#ifndef SD_NODES_PER_DOMAIN
+#define SD_NODES_PER_DOMAIN 16
+#endif
+
 #ifdef CONFIG_SCHED_BOOK
 #ifndef SD_BOOK_INIT
 #error Please define an appropriate SD_BOOK_INIT in include/asm/topology.h!!!
Index: linux-2.6-work/kernel/sched.c
===================================================================
--- linux-2.6-work.orig/kernel/sched.c	2011-07-25 11:20:09.538850173 +1000
+++ linux-2.6-work/kernel/sched.c	2011-07-25 11:26:50.626468565 +1000
@@ -6938,8 +6938,6 @@ static int __init isolated_cpu_setup(cha
 
 __setup("isolcpus=", isolated_cpu_setup);
 
-#define SD_NODES_PER_DOMAIN 16
-
 #ifdef CONFIG_NUMA
 
 /**

Re: [PATCH 0/5] ppc64 scheduler fixes

[Benjamin Herrenschmidt]

On Tue, 2011-09-20 at 10:19 +1000, Anton Blanchard wrote:
> Hi Peter,
> 
> > On Mon, 2011-07-25 at 12:33 +1000, Anton Blanchard wrote:
> > > Here are a set of ppc64 scheduler fixes that help with some
> > > multi node performance issues.
> > 
> > They look fine to me. I'll probably ping you when I'll rip out all
> > that SD_NODES_PER_DOMAIN crap for good, but until then I'm fine with
> > you fiddling it for ppc64.
> > 
> > Acked-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
> 
> Are you OK if Ben takes this, or would you prefer to send it via the
> scheduler tree?

I've already put it in my next branch that I'll stick on github later
today hopefully :-)

Cheers,
Ben.
 
> Anton
> --
> 
> [2/5] sched: Allow SD_NODES_PER_DOMAIN to be overridden
> 
> We want to override the default value of SD_NODES_PER_DOMAIN on ppc64,
> so move it into linux/topology.h.
> 
> Signed-off-by: Anton Blanchard <anton@samba.org>
> 
> ---
> 
> Index: linux-2.6-work/include/linux/topology.h
> ===================================================================
> --- linux-2.6-work.orig/include/linux/topology.h	2011-07-25 11:20:02.588717796 +1000
> +++ linux-2.6-work/include/linux/topology.h	2011-07-25 11:26:50.616468376 +1000
> @@ -201,6 +201,10 @@ int arch_update_cpu_topology(void);
>  	.balance_interval	= 64,					\
>  }
>  
> +#ifndef SD_NODES_PER_DOMAIN
> +#define SD_NODES_PER_DOMAIN 16
> +#endif
> +
>  #ifdef CONFIG_SCHED_BOOK
>  #ifndef SD_BOOK_INIT
>  #error Please define an appropriate SD_BOOK_INIT in include/asm/topology.h!!!
> Index: linux-2.6-work/kernel/sched.c
> ===================================================================
> --- linux-2.6-work.orig/kernel/sched.c	2011-07-25 11:20:09.538850173 +1000
> +++ linux-2.6-work/kernel/sched.c	2011-07-25 11:26:50.626468565 +1000
> @@ -6938,8 +6938,6 @@ static int __init isolated_cpu_setup(cha
>  
>  __setup("isolcpus=", isolated_cpu_setup);
>  
> -#define SD_NODES_PER_DOMAIN 16
> -
>  #ifdef CONFIG_NUMA
>  
>  /**
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 0/5] ppc64 scheduler fixes

[Peter Zijlstra]

On Tue, 2011-09-20 at 10:19 +1000, Anton Blanchard wrote:
> > Acked-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
> 
> Are you OK if Ben takes this, or would you prefer to send it via the
> scheduler tree? 

I was expecting Ben to pick it up, and I see he already did so.

Thanks!

Re: [PATCH 0/5] ppc64 scheduler fixes

[Peter Zijlstra]

On Tue, 2011-09-20 at 10:19 +1000, Anton Blanchard wrote:
> > Acked-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
>=20
> Are you OK if Ben takes this, or would you prefer to send it via the
> scheduler tree?=20

I was expecting Ben to pick it up, and I see he already did so.

Thanks!