x86/topology: Create logical package id

v4.6-rc1 Regression Joseph Salisbury 5 participants 11 messages May 6, 2016 – May 7, 2016 x86

Share & tools

Joseph Salisbury

author

2016-05-06 18:16 UTC #0 lore raw

Hi Thomas,

A kernel bug report was opened against Ubuntu [0]. After a kernel bisect, it was found that reverting the following commit resolved this bug:

commit 1f12e32f4cd5243ae46d8b933181be0d022c6793

Author: Thomas Gleixner <[email protected]>
Date:   Mon Feb 22 22:19:15 2016 +0000

x86/topology: Create logical package id

To build successfully with this commit reverted, I also had to revert
commits: e7ee3e8,2d4de83,87f01cc and 33c3cc7.

The regression was introduced as of v4.6-rc1.

I was hoping to get your feedback, since you are the patch author. Do you think gathering any additional data will help diagnose this issue, or would it be best to submit a revert request?

Thanks,

Joe

[0] http://pad.lv/1573231

Thomas Gleixner

2016-05-06 18:50 UTC #1 lore raw

Quoted 9 lines · Joseph Salisbury

A kernel bug report was opened against Ubuntu [0]. After a kernel bisect, it was found that reverting the following commit resolved this bug:
commit 1f12e32f4cd5243ae46d8b933181be0d022c6793
Author: Thomas Gleixner <[email protected]>
Date:   Mon Feb 22 22:19:15 2016 +0000
x86/topology: Create logical package id

Quoted 8 lines

To build successfully with this commit reverted, I also had to revert
commits: e7ee3e8,2d4de83,87f01cc and 33c3cc7.
The regression was introduced as of v4.6-rc1.
I was hoping to get your feedback, since you are the patch author. Do you think gathering any additional data will help diagnose this issue, or would it be best to submit a revert request?

Yuck. That dies with a divide error. And that looks like XEN is supplying crap data in the CPUID.

Does the patch below cure the issue?

Thanks,

tglx

8<---------------

arch/x86/kernel/smpboot.c +5 -0

@@ -332,6 +332,11 @@ static void __init smp_init_package_map(
 	 * primary cores.
 	 */
 	ncpus = boot_cpu_data.x86_max_cores;
+	if (!ncpus) {
+		pr_warn("x86_max_cores == zero !?!?");
+		ncpus = 1;
+	}
+
 	__max_logical_packages = DIV_ROUND_UP(total_cpus, ncpus);
 
 	/*

Boris Ostrovsky

2016-05-06 19:15 UTC #2 lore raw

Quoted 13 lines · Thomas Gleixner

Quoted 12 lines · Joseph Salisbury
A kernel bug report was opened against Ubuntu [0]. After a kernel bisect, it was found that reverting the following commit resolved this bug:
commit 1f12e32f4cd5243ae46d8b933181be0d022c6793
Author: Thomas Gleixner <[email protected]>
Date:   Mon Feb 22 22:19:15 2016 +0000
x86/topology: Create logical package id
To build successfully with this commit reverted, I also had to revert
commits: e7ee3e8,2d4de83,87f01cc and 33c3cc7.

Quoted 7 lines

Quoted 5 lines
The regression was introduced as of v4.6-rc1.
I was hoping to get your feedback, since you are the patch author. Do you think gathering any additional data will help diagnose this issue, or would it be best to submit a revert request?
Yuck. That dies with a divide error. And that looks like XEN is supplying crap data in the CPUID.

Joe, do you have

ed6069b xen/apic: Provide Xen-specific version of cpu_present_to_apicid APIC op

-boris

Quoted 23 lines

Does the patch below cure the issue?

Thanks,

tglx

8<---------------

arch/x86/kernel/smpboot.c +5 -0

@@ -332,6 +332,11 @@ static void __init smp_init_package_map(
 	 * primary cores.
 	 */
 	ncpus = boot_cpu_data.x86_max_cores;
+	if (!ncpus) {
+		pr_warn("x86_max_cores == zero !?!?");
+		ncpus = 1;
+	}
+
 	__max_logical_packages = DIV_ROUND_UP(total_cpus, ncpus);
 
 	/*

Joseph Salisbury

author

2016-05-06 19:38 UTC #3 lore raw

Quoted 19 lines · Boris Ostrovsky

Quoted 18 lines · Thomas Gleixner
Quoted 17 lines · Joseph Salisbury
A kernel bug report was opened against Ubuntu [0]. After a kernel bisect, it was found that reverting the following commit resolved this bug:
commit 1f12e32f4cd5243ae46d8b933181be0d022c6793
Author: Thomas Gleixner <[email protected]>
Date:   Mon Feb 22 22:19:15 2016 +0000
x86/topology: Create logical package id
To build successfully with this commit reverted, I also had to revert
commits: e7ee3e8,2d4de83,87f01cc and 33c3cc7.
The regression was introduced as of v4.6-rc1.
I was hoping to get your feedback, since you are the patch author. Do you think gathering any additional data will help diagnose this issue, or would it be best to submit a revert request?

Quoted 8 lines

Quoted 2 lines
Yuck. That dies with a divide error. And that looks like XEN is supplying crap data in the CPUID.
Joe, do you have
ed6069b xen/apic: Provide Xen-specific version of cpu_present_to_apicid APIC op
-boris

Yes the commit is in the 4.4 based Ubuntu kernel. This bug also happens with the vanilla 4.6-rc5 kernel, which also has that commit.

Quoted 25 lines

Quoted 24 lines

Does the patch below cure the issue?

Thanks,

tglx

8<---------------

arch/x86/kernel/smpboot.c +5 -0

@@ -332,6 +332,11 @@ static void __init smp_init_package_map(
 	 * primary cores.
 	 */
 	ncpus = boot_cpu_data.x86_max_cores;
+	if (!ncpus) {
+		pr_warn("x86_max_cores == zero !?!?");
+		ncpus = 1;
+	}
+
 	__max_logical_packages = DIV_ROUND_UP(total_cpus, ncpus);
 
 	/*

Boris Ostrovsky

2016-05-06 20:49 UTC #4 lore raw

Quoted 5 lines · Joseph Salisbury

Quoted 4 lines · Boris Ostrovsky
Quoted 3 lines · Thomas Gleixner
Yuck. That dies with a divide error. And that looks like XEN is supplying crap data in the CPUID.

Quoted 8 lines

Quoted 6 lines
Joe, do you have
ed6069b xen/apic: Provide Xen-specific version of cpu_present_to_apicid APIC op
-boris
Yes the commit is in the 4.4 based Ubuntu kernel. This bug also happens with the vanilla 4.6-rc5 kernel, which also has that commit.

Can you post guest's cpuid -1 -r ? (I guess after you verify Thomas' patch)

Thanks.
-boris

Joseph Salisbury

author

2016-05-06 20:51 UTC #5 lore raw

Quoted 10 lines · Boris Ostrovsky

Quoted 9 lines · Joseph Salisbury
Quoted 8 lines · Boris Ostrovsky
Quoted 2 lines · Thomas Gleixner
Yuck. That dies with a divide error. And that looks like XEN is supplying crap data in the CPUID.
Joe, do you have
ed6069b xen/apic: Provide Xen-specific version of cpu_present_to_apicid APIC op

Quoted 11 lines

Quoted 3 lines
Quoted 1 line
-boris
Yes the commit is in the 4.4 based Ubuntu kernel. This bug also happens with the vanilla 4.6-rc5 kernel, which also has that commit.
Can you post guest's cpuid -1 -r ? (I guess after you verify Thomas' patch)
Thanks.
-boris

Thomas' patch does resolve the bug. The cpuid info can be seen here:
https://launchpadlibrarian.net/258234267/cpuid_full.txt

Thanks,

Joe

Boris Ostrovsky

2016-05-06 21:27 UTC #6 lore raw

Quoted 2 lines · Joseph Salisbury

Thomas' patch does resolve the bug. The cpuid info can be seen here:
https://launchpadlibrarian.net/258234267/cpuid_full.txt

Any chance you could post it raw (cpuid -1 -r)?

Thanks.
-boris

Joseph Salisbury

author

2016-05-06 19:38 UTC #7 lore raw

In reply to Thomas Gleixner

Quoted 33 lines · Thomas Gleixner

Quoted 9 lines · Joseph Salisbury
A kernel bug report was opened against Ubuntu [0]. After a kernel bisect, it was found that reverting the following commit resolved this bug:
commit 1f12e32f4cd5243ae46d8b933181be0d022c6793
Author: Thomas Gleixner <[email protected]>
Date:   Mon Feb 22 22:19:15 2016 +0000
x86/topology: Create logical package id
Quoted 8 lines
To build successfully with this commit reverted, I also had to revert
commits: e7ee3e8,2d4de83,87f01cc and 33c3cc7.
The regression was introduced as of v4.6-rc1.
I was hoping to get your feedback, since you are the patch author. Do you think gathering any additional data will help diagnose this issue, or would it be best to submit a revert request?
Yuck. That dies with a divide error. And that looks like XEN is supplying crap data in the CPUID.
Does the patch below cure the issue?
Thanks,
tglx
8<---------------
arch/x86/kernel/smpboot.c +0 -0
@@ -332,6 +332,11 @@ static void __init smp_init_package_map(
 	 * primary cores.

arch/x86/kernel/smpboot.c static void __init smp_init_package_map( · line 342

Quoted 10 lines

arch/x86/kernel/smpboot.c +5 -0

 	 */
 	ncpus = boot_cpu_data.x86_max_cores;
+	if (!ncpus) {
+		pr_warn("x86_max_cores == zero !?!?");
+		ncpus = 1;
+	}
+
 	__max_logical_packages = DIV_ROUND_UP(total_cpus, ncpus);
 
 	/*

I'll have this patch tested and report back.

Thanks,

Joe

Joseph Salisbury

author

2016-05-06 20:50 UTC #8 lore raw

Quoted 43 lines · Joseph Salisbury

Quoted 42 lines · Thomas Gleixner
Quoted 9 lines · Joseph Salisbury
A kernel bug report was opened against Ubuntu [0]. After a kernel bisect, it was found that reverting the following commit resolved this bug:
commit 1f12e32f4cd5243ae46d8b933181be0d022c6793
Author: Thomas Gleixner <[email protected]>
Date:   Mon Feb 22 22:19:15 2016 +0000
x86/topology: Create logical package id
Quoted 8 lines
To build successfully with this commit reverted, I also had to revert
commits: e7ee3e8,2d4de83,87f01cc and 33c3cc7.
The regression was introduced as of v4.6-rc1.
I was hoping to get your feedback, since you are the patch author. Do you think gathering any additional data will help diagnose this issue, or would it be best to submit a revert request?
Yuck. That dies with a divide error. And that looks like XEN is supplying crap data in the CPUID.
Does the patch below cure the issue?
Thanks,
tglx
8<---------------
arch/x86/kernel/smpboot.c +5 -0
@@ -332,6 +332,11 @@ static void __init smp_init_package_map(
 	 * primary cores.
 	 */
 	ncpus = boot_cpu_data.x86_max_cores;
+	if (!ncpus) {
+		pr_warn("x86_max_cores == zero !?!?");
+		ncpus = 1;
+	}
+
 	__max_logical_packages = DIV_ROUND_UP(total_cpus, ncpus);
 

Quoted 6 lines

arch/x86/kernel/smpboot.c static void __init smp_init_package_map( · line 342
Quoted 1 line
arch/x86/kernel/smpboot.c +0 -0
 	/*
I'll have this patch tested and report back.
Thanks,
Joe

Yes, your patch does in fact fix the bug. Would you like any additional information regarding the bug?

Ingo Molnar

2016-05-07 05:16 UTC #9 lore raw

In reply to Thomas Gleixner

Quoted 18 lines · Thomas Gleixner

Quoted 3 lines
I was hoping to get your feedback, since you are the patch author. Do you think gathering any additional data will help diagnose this issue, or would it be best to submit a revert request?
Yuck. That dies with a divide error. And that looks like XEN is supplying crap data in the CPUID.
Does the patch below cure the issue?
Thanks,
tglx
8<---------------

arch/x86/kernel/smpboot.c static void __init smp_init_package_map( · line 339

Quoted 8 lines

arch/x86/kernel/smpboot.c +5 -0

@@ -332,6 +332,11 @@ static void __init smp_init_package_map(
 	 * primary cores.
 	 */
 	ncpus = boot_cpu_data.x86_max_cores;
+	if (!ncpus) {
+		pr_warn("x86_max_cores == zero !?!?");
+		ncpus = 1;
+	}
+

Even if the underlying Xen bug is fixed, I think we should add this protective measure to defend against future CPUID mishaps.

Thanks,

Ingo

tip-bot for Thomas Gleixner

merge bot

2016-05-07 08:13 UTC #10 lore raw

[tip:x86/urgent] x86/topology: Handle CPUID bogosity gracefully

In reply to Thomas Gleixner

x86/topology: Handle CPUID bogosity gracefully

Commit-ID:  56402d63eefe22179f7311a51ff2094731420406
Gitweb:     http://git.kernel.org/tip/56402d63eefe22179f7311a51ff2094731420406
Author:     Thomas Gleixner <[email protected]>
AuthorDate: Fri, 6 May 2016 20:48:16 +0200
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Sat, 7 May 2016 10:06:55 +0200

x86/topology: Handle CPUID bogosity gracefully

Joseph reported that a XEN guest dies with a division by 0 in the package topology setup code. This happens if cpu_info.x86_max_cores is zero.

Handle that case and emit a warning. This does not fix the underlying XEN bug, but makes the code more robust.

Reported-and-tested-by Joseph Salisbury <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Boris Ostrovsky <[email protected]>

Cc David Vrabel <[email protected]>

Link http://lkml.kernel.org/r/alpine.DEB.2.11.1605062046270.3540@nanos open here

Signed-off-by Thomas Gleixner <[email protected]>

---
 arch/x86/kernel/smpboot.c | 5 +++++
 1 file changed, 5 insertions(+)

arch/x86/kernel/smpboot.c +5 -0

index a2065d3..0e4329e 100644
@@ -332,6 +332,11 @@ static void __init smp_init_package_map(void)
 	 * primary cores.
 	 */
 	ncpus = boot_cpu_data.x86_max_cores;
+	if (!ncpus) {
+		pr_warn("x86_max_cores == zero !?!?");
+		ncpus = 1;
+	}
+
 	__max_logical_packages = DIV_ROUND_UP(total_cpus, ncpus);
 
 	/*