pseries: cpu: Reduce the polling interval in __cpu_up()

Submitted by Gautham R Shenoy on June 24, 2009, 9:26 a.m.

Details

Message ID 20090624092637.32078.88042.stgit@sofia.in.ibm.com
State Accepted, archived
Commit 6776426320e151051a16bc7bf86f12d310c9e8ca
Delegated to: Benjamin Herrenschmidt
Headers show

Commit Message

Gautham R Shenoy June 24, 2009, 9:26 a.m.
Time time taken for a single cpu online operation on a pseries machine
is as follows:
Dedicated LPAR (POWER6): ~220ms.
Shared LPAR (POWER5)   : ~240ms.

Of this time, approximately 200ms is taken up by __cpu_up(). This is because
we poll every 200ms to check if the new cpu has notified it's presence
through the cpu_callin_map. We repeat this operation until the new cpu sets
the value in cpu_callin_map or 5 seconds elapse, whichever comes earlier.

However, using completion_structs instead of polling loops,
the time taken by the new processor to indicate it's presence has
found to be less than 1ms on pseries. This method however may not
work on all powerpc platforms due to the time-base synchronization code.

Keeping this in mind, we could reduce msleep polling interval from
200ms to 1ms while retaining the 5 second timeout.

With this, the time taken for a cpu online operation changes as follows:
Dedicated LPAR (POWER6): 20-25ms.
Shared LPAR (POWER5)   : 60-80ms.

In both these cases, it was found that the code polls through the loop
only once indicating that 1ms is a reasonable value, atleast on pseries.

The code needs testing on other powerpc platforms.

Signed-off-by: Gautham R Shenoy <ego@in.ibm.com>
---
 arch/powerpc/kernel/smp.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

Comments

jschopp@austin.ibm.com June 24, 2009, 4:39 p.m.
>
> The code needs testing on other powerpc platforms.
>   
I think given the numbers you showed this is a good improvement, and it 
clearly can't do any harm on platforms that implement msleep correctly.

For what it's worth:
Acked-by: Joel Schopp <jschopp@austin.ibm.com>
> Signed-off-by: Gautham R Shenoy <ego@in.ibm.com>
> ---
>  arch/powerpc/kernel/smp.c |    5 ++---
>  1 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> index 65484b2..00c13a1 100644
> --- a/arch/powerpc/kernel/smp.c
> +++ b/arch/powerpc/kernel/smp.c
> @@ -411,9 +411,8 @@ int __cpuinit __cpu_up(unsigned int cpu)
>  		 * CPUs can take much longer to come up in the
>  		 * hotplug case.  Wait five seconds.
>  		 */
> -		for (c = 25; c && !cpu_callin_map[cpu]; c--) {
> -			msleep(200);
> -		}
> +		for (c = 5000; c && !cpu_callin_map[cpu]; c--)
> +			msleep(1);
>  #endif
>  
>  	if (!cpu_callin_map[cpu]) {
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>

Patch hide | download patch | download mbox

diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 65484b2..00c13a1 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -411,9 +411,8 @@  int __cpuinit __cpu_up(unsigned int cpu)
 		 * CPUs can take much longer to come up in the
 		 * hotplug case.  Wait five seconds.
 		 */
-		for (c = 25; c && !cpu_callin_map[cpu]; c--) {
-			msleep(200);
-		}
+		for (c = 5000; c && !cpu_callin_map[cpu]; c--)
+			msleep(1);
 #endif
 
 	if (!cpu_callin_map[cpu]) {