[PATCH/RFT 0/2] regression fix: "smpboot: do_boot_cpu failed(-1) to wakeup CPU#1"

[PATCH/RFT 0/2] regression fix: "smpboot: do_boot_cpu failed(-1) to wakeup CPU#1"

[Len Brown]

Shane, Donald,
Please test this pair of patches.

If the default kernel still has a problem booting CPU #1 on yoru Core2 system,
"cpu_init_udelay=10000" should get you back to the legacy delays.

thanks,
Len Brown, Intel Open Source Technology Center

[PATCH 1/2] x86 smpboot: fix cpu_init_udelay=10000

[Len Brown]

From: Len Brown <len.brown@intel.com>

For legacy machines cpu_init_udelay defaults to 10,000.
For modern machines it is set to 0.

The user should be able to set cpu_init_udelay to
any value on the cmdline, including 10,000.
Before this patch, that was seen as "unchanged from default"
and thus on a modern machine, the user request was ignored
and the delay was set to 0.

Signed-off-by: Len Brown <len.brown@intel.com>
---
 arch/x86/kernel/smpboot.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index e0c198e..32267cc 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -509,7 +509,7 @@ void __inquire_remote_apic(int apicid)
  */
 #define UDELAY_10MS_DEFAULT 10000
 
-static unsigned int init_udelay = UDELAY_10MS_DEFAULT;
+static unsigned int init_udelay = INT_MAX;
 
 static int __init cpu_init_udelay(char *str)
 {
@@ -522,13 +522,16 @@ early_param("cpu_init_udelay", cpu_init_udelay);
 static void __init smp_quirk_init_udelay(void)
 {
 	/* if cmdline changed it from default, leave it alone */
-	if (init_udelay != UDELAY_10MS_DEFAULT)
+	if (init_udelay != INT_MAX)
 		return;
 
 	/* if modern processor, use no delay */
 	if (((boot_cpu_data.x86_vendor == X86_VENDOR_INTEL) && (boot_cpu_data.x86 == 6)) ||
 	    ((boot_cpu_data.x86_vendor == X86_VENDOR_AMD) && (boot_cpu_data.x86 >= 0xF)))
 		init_udelay = 0;
+
+	/* else, use legacy delay */
+	init_udelay = UDELAY_10MS_DEFAULT;
 }
 
 /*
-- 
2.6.1.145.gb27dacc

[PATCH 2/2] x86 smpboot: fix CPU #1 boot timeout

[Len Brown]

From: Len Brown <len.brown@intel.com>

Commit a9bcaa02a5104ace6a9d9e4a9cd9192a9e7744d6
("x86/smpboot: Remove SIPI delays from cpu_up()")

Caused some Intel Core2 processors to time-out when bringing up CPU #1.

That patch reduced the SIPI delays from udelay() 300, 200
to udelay() 0, 0 on modern processors.

Several Intel(R) Core(TM)2 systems
failed to bring up CPU #1 10/10 times after that change.

Increasing either of the SIPI delays to udelay(1) results in success.
So here we increase both to udelay(10).  While this may be 20x slower
than the absolute minimum, it is still 20x to 30x faster
than the original code.

Signed-off-by: Len Brown <len.brown@intel.com>
---
 arch/x86/kernel/smpboot.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 32267cc..892ee2e5 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -660,7 +660,9 @@ wakeup_secondary_cpu_via_init(int phys_apicid, unsigned long start_eip)
 		/*
 		 * Give the other CPU some time to accept the IPI.
 		 */
-		if (init_udelay)
+		if (init_udelay == 0)
+			udelay(10);
+		else
 			udelay(300);
 
 		pr_debug("Startup point 1\n");
@@ -671,7 +673,9 @@ wakeup_secondary_cpu_via_init(int phys_apicid, unsigned long start_eip)
 		/*
 		 * Give the other CPU some time to accept the IPI.
 		 */
-		if (init_udelay)
+		if (init_udelay == 0)
+			udelay(10);
+		else
 			udelay(200);
 
 		if (maxlvt > 3)		/* Due to the Pentium erratum 3AP.  */
-- 
2.6.1.145.gb27dacc

Re: [PATCH 1/2] x86 smpboot: fix cpu_init_udelay=10000

[shrybman]

> From: Len Brown <len.brown@intel.com>
> 
> For legacy machines cpu_init_udelay defaults to 10,000.
> For modern machines it is set to 0.
> 
> The user should be able to set cpu_init_udelay to
> any value on the cmdline, including 10,000.
> Before this patch, that was seen as "unchanged from default"
> and thus on a modern machine, the user request was ignored
> and the delay was set to 0.
> 
> Signed-off-by: Len Brown <len.brown@intel.com>
> ---
>  arch/x86/kernel/smpboot.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
> index e0c198e..32267cc 100644
> --- a/arch/x86/kernel/smpboot.c
> +++ b/arch/x86/kernel/smpboot.c
> @@ -509,7 +509,7 @@ void __inquire_remote_apic(int apicid)
>   */
>  #define UDELAY_10MS_DEFAULT 10000
>  
> -static unsigned int init_udelay = UDELAY_10MS_DEFAULT;
> +static unsigned int init_udelay = INT_MAX;
>  
>  static int __init cpu_init_udelay(char *str)
>  {
> @@ -522,13 +522,16 @@ early_param("cpu_init_udelay", cpu_init_udelay);
>  static void __init smp_quirk_init_udelay(void)
>  {
>       /* if cmdline changed it from default, leave it alone */
> -     if (init_udelay != UDELAY_10MS_DEFAULT)
> +     if (init_udelay != INT_MAX)
>            return;
>  
>       /* if modern processor, use no delay */
>       if (((boot_cpu_data.x86_vendor == X86_VENDOR_INTEL) && (boot_cpu_data.x86 == 6)) ||
>           ((boot_cpu_data.x86_vendor == X86_VENDOR_AMD) && (boot_cpu_data.x86 >= 0xF)))
>            init_udelay = 0;
> +
> +     /* else, use legacy delay */
> +     init_udelay = UDELAY_10MS_DEFAULT;


Hi Len,


Is this really what you intended? The else is commented out so if init_udelay is quirked
to be 0 it will always be reset to UDELAY_10MS_DEFAULT. Also init_udelay is unsigned, so
would UINT_MAX be a better choice?


Shane

Re: [PATCH 1/2] x86 smpboot: fix cpu_init_udelay=10000

[Len Brown]

> Is this really what you intended? The else is commented out so if init_udelay is quirked
> to be 0 it will always be reset to UDELAY_10MS_DEFAULT. Also init_udelay is unsigned, so
> would UINT_MAX be a better choice?

Hi Shane,
Thanks for pointing out this flaw.
Seems it will make 4.3 10ms slower than 4.2 on these boxes, by default.
We'll send an incremental patch for 4.4.

Len Brown, Intel Open Source Technology Center

Re: [PATCH 1/2] x86 smpboot: fix cpu_init_udelay=10000

[shrybman]

> > Is this really what you intended? The else is commented out so if init_udelay is quirked
> > to be 0 it will always be reset to UDELAY_10MS_DEFAULT. Also init_udelay is unsigned, so
> > would UINT_MAX be a better choice?
> 
> Hi Shane,
> Thanks for pointing out this flaw.
> Seems it will make 4.3 10ms slower than 4.2 on these boxes, by default.
> We'll send an incremental patch for 4.4.


Sounds good. Thanks Len.

Re: [PATCH/RFT 0/2] regression fix: "smpboot: do_boot_cpu failed(-1) to wakeup CPU#1"

[Donald Parsons]

On Fri, 2015-10-16 at 00:14 -0400, Len Brown wrote:
> Shane, Donald,
> Please test this pair of patches.
> 
> If the default kernel still has a problem booting CPU #1 on your Core2 system,
> "cpu_init_udelay=10000" should get you back to the legacy delays.

Applied patches to Linus' 4.3-rc4 (with none of our recently discussed
changes.) 

Result missing CPU is back!

I also tried the "cpu_init_udelay=1" with rc5 and that also gave me back
the missing CPU?  Previously I had tried "cpu_init_udelay=10000" and it
did not get missing CPU back (with rc3 or rc4).

I can do more testing if you need it.

Thanks,
Don

Re: [PATCH/RFT 0/2] regression fix: "smpboot: do_boot_cpu failed(-1) to wakeup CPU#1"

[shrybman]

> Shane, Donald,
> Please test this pair of patches.
> 
> If the default kernel still has a problem booting CPU #1 on yoru Core2 system,
> "cpu_init_udelay=10000" should get you back to the legacy delays.


I tried with both patches and it worked fine without cpu_init_udelay.


Thanks,


Shane