From: Prarit Bhargava <prarit@redhat.com>
To: linux-kernel@vger.kernel.org
Cc: Prarit Bhargava <prarit@redhat.com>,
Thomas Gleixner <tglx@linutronix.de>,
Ingo Molnar <mingo@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>,
x86@kernel.org, Peter Zijlstra <peterz@infradead.org>,
Andi Kleen <ak@linux.intel.com>,
Dave Hansen <dave.hansen@intel.com>,
Piotr Luc <piotr.luc@intel.com>, Kan Liang <kan.liang@intel.com>,
Borislav Petkov <bp@suse.de>,
Stephane Eranian <eranian@google.com>,
Arvind Yadav <arvind.yadav.cs@gmail.com>,
Andy Lutomirski <luto@kernel.org>,
Christian Borntraeger <borntraeger@de.ibm.com>,
"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
Tom Lendacky <thomas.lendacky@amd.com>,
Mathias Krause <minipli@googlemail.com>,
Tim Chen <tim.c.chen@linux.intel.com>,
Vitaly Kuznetsov <vkuznets@redhat.com>
Subject: [PATCH 3/3] x86/smpboot: Fix __max_logical_packages estimate
Date: Tue, 24 Oct 2017 14:08:02 -0400 [thread overview]
Message-ID: <20171024180802.8422-4-prarit@redhat.com> (raw)
In-Reply-To: <20171024180802.8422-1-prarit@redhat.com>
A system booted with a small number of cores enabled per package
panics because the estimate of __max_logical_packages is too low.
This occurs when the total number of active cores across all packages
is less than the maximum core count for a single package.
ie) On a 4 package system with 20 cores/package where only 4 cores
are enabled on each package, the value of __max_logical_packages is
calculated as DIV_ROUND_UP(16 / 20) = 1 and not 4.
Here's an example of the panic:
smpboot: Booting Node 1, Processors #1 OK
smpboot: Package 1 of CPU 1 exceeds BIOS package data 1.
------------[ cut here ]------------
kernel BUG at arch/x86/kernel/cpu/common.c:1087!
invalid opcode: 0000 [#1] SMP
Modules linked in:
CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.14.0-rc2+ #4
Calculate __max_logical_packages after the cpu enumeration has completed.
Use the boot cpu's data to extrapolate the number of packages.
Signed-off-by: Prarit Bhargava <prarit@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: x86@kernel.org
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Piotr Luc <piotr.luc@intel.com>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: Stephane Eranian <eranian@google.com>
Cc: Arvind Yadav <arvind.yadav.cs@gmail.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Mathias Krause <minipli@googlemail.com>
Cc: Tim Chen <tim.c.chen@linux.intel.com>
Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
---
arch/x86/kernel/smpboot.c | 57 +++++++++--------------------------------------
1 file changed, 10 insertions(+), 47 deletions(-)
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index ca615bc94e82..6307e92f54ae 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -307,12 +307,6 @@ int topology_update_package_map(unsigned int pkg, unsigned int cpu)
if (new >= 0)
goto found;
- if (logical_packages >= __max_logical_packages) {
- pr_warn("Package %u of CPU %u exceeds BIOS package data %u.\n",
- logical_packages, cpu, __max_logical_packages);
- return -ENOSPC;
- }
-
new = logical_packages++;
/* Allocate and copy a new array */
@@ -335,46 +329,6 @@ int topology_update_package_map(unsigned int pkg, unsigned int cpu)
return 0;
}
-static void __init smp_init_package_map(struct cpuinfo_x86 *c, unsigned int cpu)
-{
- unsigned int ncpus;
-
- /*
- * Today neither Intel nor AMD support heterogenous systems. That
- * might change in the future....
- *
- * While ideally we'd want '* smp_num_siblings' in the below @ncpus
- * computation, this won't actually work since some Intel BIOSes
- * report inconsistent HT data when they disable HT.
- *
- * In particular, they reduce the APIC-IDs to only include the cores,
- * but leave the CPUID topology to say there are (2) siblings.
- * This means we don't know how many threads there will be until
- * after the APIC enumeration.
- *
- * By not including this we'll sometimes over-estimate the number of
- * logical packages by the amount of !present siblings, but this is
- * still better than MAX_LOCAL_APIC.
- *
- * We use total_cpus not nr_cpu_ids because nr_cpu_ids can be limited
- * on the command line leading to a similar issue as the HT disable
- * problem because the hyperthreads are usually enumerated after the
- * 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);
- pr_info("Max logical packages: %u\n", __max_logical_packages);
-
- logical_packages = 0;
-
- topology_update_package_map(c->phys_proc_id, cpu);
-}
-
void __init smp_store_boot_cpu_info(void)
{
int id = 0; /* CPU 0 */
@@ -382,7 +336,7 @@ void __init smp_store_boot_cpu_info(void)
*c = boot_cpu_data;
c->cpu_index = id;
- smp_init_package_map(c, id);
+ topology_update_package_map(c->phys_proc_id, id);
}
/*
@@ -1382,7 +1336,16 @@ void __init native_smp_prepare_boot_cpu(void)
void __init native_smp_cpus_done(unsigned int max_cpus)
{
+ int ncpus;
+
pr_debug("Boot done\n");
+ /*
+ * Today neither Intel nor AMD support heterogenous systems so
+ * extrapolate the boot cpu's data to all packages.
+ */
+ ncpus = cpu_data(0).booted_cores * smp_num_siblings;
+ __max_logical_packages = DIV_ROUND_UP(nr_cpu_ids, ncpus);
+ pr_info("Max logical packages: %u\n", __max_logical_packages);
if (x86_has_numa_in_package)
set_sched_topology(x86_numa_in_package_topology);
--
2.15.0.rc0.39.g2f0e14e64
next prev parent reply other threads:[~2017-10-24 18:08 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-10-24 18:07 [PATCH 0/3 v4] Fix panic in logical packages calculation Prarit Bhargava
2017-10-24 18:08 ` [PATCH 1/3] perf/x86/intel/uncore: Cache logical pkg id in uncore driver Prarit Bhargava
2017-10-24 18:08 ` [PATCH 2/3] x86/topology: Avoid wasting 128k for package id array Prarit Bhargava
2017-10-29 14:20 ` [lkp-robot] [x86/topology] 45d87f5bb7: WARNING:at_kernel/locking/lockdep.c:#lockdep_trace_alloc kernel test robot
2017-10-29 14:20 ` kernel test robot
2017-10-24 18:08 ` Prarit Bhargava [this message]
-- strict thread matches above, loose matches on Subject: below --
2017-09-08 19:57 [PATCH 0/3] x86/smpboot: Cleanup logical package ID Prarit Bhargava
2017-09-08 19:57 ` [PATCH 3/3] x86/smpboot: Fix __max_logical_packages estimate Prarit Bhargava
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20171024180802.8422-4-prarit@redhat.com \
--to=prarit@redhat.com \
--cc=ak@linux.intel.com \
--cc=arvind.yadav.cs@gmail.com \
--cc=borntraeger@de.ibm.com \
--cc=bp@suse.de \
--cc=dave.hansen@intel.com \
--cc=eranian@google.com \
--cc=hpa@zytor.com \
--cc=kan.liang@intel.com \
--cc=kirill.shutemov@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=luto@kernel.org \
--cc=mingo@redhat.com \
--cc=minipli@googlemail.com \
--cc=peterz@infradead.org \
--cc=piotr.luc@intel.com \
--cc=tglx@linutronix.de \
--cc=thomas.lendacky@amd.com \
--cc=tim.c.chen@linux.intel.com \
--cc=vkuznets@redhat.com \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.