[PATCH] powerpc: inline ppc64_runlatch_off

[PATCH] powerpc: inline ppc64_runlatch_off

[Anton Blanchard]

I'm sick of seeing ppc64_runlatch_off in our profiles, so inline the
heavily used part of it into the callers. To avoid a mess of circular includes
I didn't add it as an inline function.

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

Index: powerpc.git/arch/powerpc/include/asm/reg.h
===================================================================
--- powerpc.git.orig/arch/powerpc/include/asm/reg.h	2010-08-04 19:55:38.910793475 +1000
+++ powerpc.git/arch/powerpc/include/asm/reg.h	2010-08-04 20:20:19.490751850 +1000
@@ -951,7 +951,14 @@
 #ifdef CONFIG_PPC64
 
 extern void ppc64_runlatch_on(void);
-extern void ppc64_runlatch_off(void);
+extern void __ppc64_runlatch_off(void);
+
+#define ppc64_runlatch_off()					\
+	do {							\
+		if (cpu_has_feature(CPU_FTR_CTRL) &&		\
+		    test_thread_flag(TIF_RUNLATCH))		\
+			__ppc64_runlatch_off();			\
+	} while (0);
 
 extern unsigned long scom970_read(unsigned int address);
 extern void scom970_write(unsigned int address, unsigned long value);
Index: powerpc.git/arch/powerpc/kernel/process.c
===================================================================
--- powerpc.git.orig/arch/powerpc/kernel/process.c	2010-08-04 19:55:38.890747120 +1000
+++ powerpc.git/arch/powerpc/kernel/process.c	2010-08-04 20:15:27.573241044 +1000
@@ -1198,19 +1198,17 @@ void ppc64_runlatch_on(void)
 	}
 }
 
-void ppc64_runlatch_off(void)
+void __ppc64_runlatch_off(void)
 {
 	unsigned long ctrl;
 
-	if (cpu_has_feature(CPU_FTR_CTRL) && test_thread_flag(TIF_RUNLATCH)) {
-		HMT_medium();
+	HMT_medium();
 
-		clear_thread_flag(TIF_RUNLATCH);
+	clear_thread_flag(TIF_RUNLATCH);
 
-		ctrl = mfspr(SPRN_CTRLF);
-		ctrl &= ~CTRL_RUNLATCH;
-		mtspr(SPRN_CTRLT, ctrl);
-	}
+	ctrl = mfspr(SPRN_CTRLF);
+	ctrl &= ~CTRL_RUNLATCH;
+	mtspr(SPRN_CTRLT, ctrl);
 }
 #endif
 

Re: [PATCH] powerpc: inline ppc64_runlatch_off

[Benjamin Herrenschmidt]

On Fri, 2010-08-06 at 14:53 +1000, Anton Blanchard wrote:
> I'm sick of seeing ppc64_runlatch_off in our profiles, so inline the
> heavily used part of it into the callers. To avoid a mess of circular includes
> I didn't add it as an inline function.

Considering that it's just an asm instruction or two, should we make it
inline asm and have it NOPed out instead using the feature sections ?

Cheers,
Ben.

> Signed-off-by: Anton Blanchard <anton@samba.org>
> ---
> 
> Index: powerpc.git/arch/powerpc/include/asm/reg.h
> ===================================================================
> --- powerpc.git.orig/arch/powerpc/include/asm/reg.h	2010-08-04 19:55:38.910793475 +1000
> +++ powerpc.git/arch/powerpc/include/asm/reg.h	2010-08-04 20:20:19.490751850 +1000
> @@ -951,7 +951,14 @@
>  #ifdef CONFIG_PPC64
>  
>  extern void ppc64_runlatch_on(void);
> -extern void ppc64_runlatch_off(void);
> +extern void __ppc64_runlatch_off(void);
> +
> +#define ppc64_runlatch_off()					\
> +	do {							\
> +		if (cpu_has_feature(CPU_FTR_CTRL) &&		\
> +		    test_thread_flag(TIF_RUNLATCH))		\
> +			__ppc64_runlatch_off();			\
> +	} while (0);
>  
>  extern unsigned long scom970_read(unsigned int address);
>  extern void scom970_write(unsigned int address, unsigned long value);
> Index: powerpc.git/arch/powerpc/kernel/process.c
> ===================================================================
> --- powerpc.git.orig/arch/powerpc/kernel/process.c	2010-08-04 19:55:38.890747120 +1000
> +++ powerpc.git/arch/powerpc/kernel/process.c	2010-08-04 20:15:27.573241044 +1000
> @@ -1198,19 +1198,17 @@ void ppc64_runlatch_on(void)
>  	}
>  }
>  
> -void ppc64_runlatch_off(void)
> +void __ppc64_runlatch_off(void)
>  {
>  	unsigned long ctrl;
>  
> -	if (cpu_has_feature(CPU_FTR_CTRL) && test_thread_flag(TIF_RUNLATCH)) {
> -		HMT_medium();
> +	HMT_medium();
>  
> -		clear_thread_flag(TIF_RUNLATCH);
> +	clear_thread_flag(TIF_RUNLATCH);
>  
> -		ctrl = mfspr(SPRN_CTRLF);
> -		ctrl &= ~CTRL_RUNLATCH;
> -		mtspr(SPRN_CTRLT, ctrl);
> -	}
> +	ctrl = mfspr(SPRN_CTRLF);
> +	ctrl &= ~CTRL_RUNLATCH;
> +	mtspr(SPRN_CTRLT, ctrl);
>  }
>  #endif
>  

Re: [PATCH] powerpc: inline ppc64_runlatch_off

[Anton Blanchard]

Hi Ben,

> > I'm sick of seeing ppc64_runlatch_off in our profiles, so inline the
> > heavily used part of it into the callers. To avoid a mess of circular
> > includes I didn't add it as an inline function.
> 
> Considering that it's just an asm instruction or two, should we make it
> inline asm and have it NOPed out instead using the feature sections ?

Unfortunately we still need to prevent continual writes to it with a per thread
flag because on some CPUs a write to the SPR in low priority mode will stall
another SMT thread. So we could get rid of the cpu feature comparison but
we would still need the thread bit check (or perhaps replace it with a per cpu
variable).

Anton

Re: [PATCH] powerpc: inline ppc64_runlatch_off

[Benjamin Herrenschmidt]

On Fri, 2010-08-06 at 15:56 +1000, Anton Blanchard wrote:
> Unfortunately we still need to prevent continual writes to it with a per thread
> flag because on some CPUs a write to the SPR in low priority mode will stall
> another SMT thread. So we could get rid of the cpu feature comparison but
> we would still need the thread bit check (or perhaps replace it with a per cpu
> variable). 

remind me why we need to do that runlatch thing on these CPUs at all ?

Cheers,
Ben.

Re: [PATCH] powerpc: inline ppc64_runlatch_off

[Anton Blanchard]

Hi,

> remind me why we need to do that runlatch thing on these CPUs at all ?

The PMU uses it so events can be constructed that count only non idle cycles.
I think the power management hardware on POWER6 and POWER7 also use the
runlatch state to determine how busy a CPU is.

Anton

Re: [PATCH] powerpc: inline ppc64_runlatch_off

[Olof Johansson]

On Fri, Aug 06, 2010 at 02:53:15PM +1000, Anton Blanchard wrote:
> 
> I'm sick of seeing ppc64_runlatch_off in our profiles, so inline the
> heavily used part of it into the callers. To avoid a mess of circular includes
> I didn't add it as an inline function.
> 
> Signed-off-by: Anton Blanchard <anton@samba.org>
> ---
> 
> Index: powerpc.git/arch/powerpc/include/asm/reg.h
> ===================================================================
> --- powerpc.git.orig/arch/powerpc/include/asm/reg.h	2010-08-04 19:55:38.910793475 +1000
> +++ powerpc.git/arch/powerpc/include/asm/reg.h	2010-08-04 20:20:19.490751850 +1000
> @@ -951,7 +951,14 @@
>  #ifdef CONFIG_PPC64
>  
>  extern void ppc64_runlatch_on(void);
> -extern void ppc64_runlatch_off(void);
> +extern void __ppc64_runlatch_off(void);
> +
> +#define ppc64_runlatch_off()					\
> +	do {							\
> +		if (cpu_has_feature(CPU_FTR_CTRL) &&		\
> +		    test_thread_flag(TIF_RUNLATCH))		\
> +			__ppc64_runlatch_off();			\
> +	} while (0);

No semicolon here.


-Olof

Re: [PATCH] powerpc: inline ppc64_runlatch_off

[Anton Blanchard]

 
Hi Olof,

> No semicolon here.

Nice catch!

Anton

I'm sick of seeing ppc64_runlatch_off in our profiles, so inline it
into the callers. To avoid a mess of circular includes I didn't add
it as an inline function.

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

Index: powerpc.git/arch/powerpc/include/asm/reg.h
===================================================================
--- powerpc.git.orig/arch/powerpc/include/asm/reg.h	2010-08-04 19:55:38.910793475 +1000
+++ powerpc.git/arch/powerpc/include/asm/reg.h	2010-08-04 20:20:19.490751850 +1000
@@ -951,7 +951,14 @@
 #ifdef CONFIG_PPC64
 
 extern void ppc64_runlatch_on(void);
-extern void ppc64_runlatch_off(void);
+extern void __ppc64_runlatch_off(void);
+
+#define ppc64_runlatch_off()					\
+	do {							\
+		if (cpu_has_feature(CPU_FTR_CTRL) &&		\
+		    test_thread_flag(TIF_RUNLATCH))		\
+			__ppc64_runlatch_off();			\
+	} while (0)
 
 extern unsigned long scom970_read(unsigned int address);
 extern void scom970_write(unsigned int address, unsigned long value);
Index: powerpc.git/arch/powerpc/kernel/process.c
===================================================================
--- powerpc.git.orig/arch/powerpc/kernel/process.c	2010-08-04 19:55:38.890747120 +1000
+++ powerpc.git/arch/powerpc/kernel/process.c	2010-08-04 20:15:27.573241044 +1000
@@ -1198,19 +1198,17 @@ void ppc64_runlatch_on(void)
 	}
 }
 
-void ppc64_runlatch_off(void)
+void __ppc64_runlatch_off(void)
 {
 	unsigned long ctrl;
 
-	if (cpu_has_feature(CPU_FTR_CTRL) && test_thread_flag(TIF_RUNLATCH)) {
-		HMT_medium();
+	HMT_medium();
 
-		clear_thread_flag(TIF_RUNLATCH);
+	clear_thread_flag(TIF_RUNLATCH);
 
-		ctrl = mfspr(SPRN_CTRLF);
-		ctrl &= ~CTRL_RUNLATCH;
-		mtspr(SPRN_CTRLT, ctrl);
-	}
+	ctrl = mfspr(SPRN_CTRLF);
+	ctrl &= ~CTRL_RUNLATCH;
+	mtspr(SPRN_CTRLT, ctrl);
 }
 #endif
 

Re: [PATCH] powerpc: inline ppc64_runlatch_off

[Olof Johansson]

On Fri, Aug 06, 2010 at 11:28:19PM +1000, Anton Blanchard wrote:
>  
> Hi Olof,
> 
> > No semicolon here.
> 
> Nice catch!
> 
> Anton
> 
> I'm sick of seeing ppc64_runlatch_off in our profiles, so inline it
> into the callers. To avoid a mess of circular includes I didn't add
> it as an inline function.
> 
> Signed-off-by: Anton Blanchard <anton@samba.org>

Acked-by: Olof Johansson <olof@lixom.net>


-Olof