On 09/02/2023 03:53, Paul E. McKenney wrote:
On Tue, Feb 07, 2023 at 11:04:27PM +0000, Usama Arif wrote:
Tested on v7, doing INIT/SIPI/SIPI in parallel brings down the time for
smpboot from ~700ms to 100ms (85% improvement) on a server with 128 CPUs
split across 2 NUMA nodes.
The major change over v6 is keeping parallel smp support enabled in AMD.
APIC ID for parallel CPU bringup is now obtained from CPUID leaf 0x0B
(for x2APIC mode) otherwise CPUID leaf 0x1 (8 bits).
The patch for reusing timer calibration for secondary CPUs is also removed
from the series as its not part of parallel smp bringup and needs to be
further thought about.
Running rcutorture on this got me the following NULL pointer dereference
on scenario TREE01:
------------------------------------------------------------------------
[ 34.662066] smpboot: CPU 0 is now offline
[ 34.674075] rcu: NOCB: Cannot CB-offload offline CPU 25
[ 35.038003] rcu: De-offloading 5
[ 35.112997] rcu: Offloading 12
[ 35.716011] smpboot: Booting Node 0 Processor 0 APIC 0x0
[ 35.762685] BUG: kernel NULL pointer dereference, address: 0000000000000001
[ 35.764278] #PF: supervisor instruction fetch in kernel mode
[ 35.765530] #PF: error_code(0x0010) - not-present page
[ 35.766700] PGD 0 P4D 0
[ 35.767278] Oops: 0010 [#1] PREEMPT SMP PTI
[ 35.768223] CPU: 36 PID: 0 Comm: swapper/36 Not tainted 6.2.0-rc1-00206-g18a37610b632-dirty #3563
[ 35.770201] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.16.0-0-gd239552ce722-prebuilt.qemu.org 04/01/2014
------------------------------------------------------------------------
Given an x86 system with KVM and qemu, this can be reproduced by running
the following from the top-level directory in the Linux-kernel source
tree:
tools/testing/selftests/rcutorture/bin/kvm.sh --allcpus --configs "TREE01 TINY01" --trust-make
Out of 15 runs, 14 blew up just after the first attempt to bring CPU
0 back online. The 15th run blew up just after the second attempt to
bring CPU 0 online, the first attempt having succeeded.
My guess is that the CONFIG_BOOTPARAM_HOTPLUG_CPU0=y Kconfig option is
tickling this bug. This Kconfig option has been added to the TREE01
scenario in the -rcu tree's "dev" branch, which might mean that this test
would pass on mainline. But CONFIG_BOOTPARAM_HOTPLUG_CPU0=y is not new,
only rcutorture's testing of it.
Thoughts?
Thanx, Paul
It looks like its because of the initial_gs, initial_stack and
early_gdt_descr not being setup properly for CPU0 hotplug, i.e.
init_cpu_data isnt called in cpu0 hotplug case.
Its easy to test, just by doing
echo 0 > /sys/devices/system/cpu/cpu0/online;
echo 1 > /sys/devices/system/cpu/cpu0/online;
As a quick check, if we do something like below (probably there is a
much better place to set these..), the above hotplug commands will work.
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 3ec5182d9698..184135c47ee5 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -1080,6 +1080,7 @@ wakeup_cpu_via_init_nmi(int cpu, unsigned long
start_ip, int apicid,
wakeup_cpu0_nmi, 0, "wake_cpu0");
if (!boot_error) {
+ initial_gs = per_cpu_offset(cpu);
enable_start_cpu0 = 1;
*cpu0_nmi_registered = 1;
id = apic->dest_mode_logical ? cpu0_logical_apicid :
apicid;
@@ -1188,10 +1189,14 @@ static int do_boot_cpu(int apicid, int cpu,
struct task_struct *idle,
boot_error = apic->wakeup_secondary_cpu_64(apicid,
start_ip);
else if (apic->wakeup_secondary_cpu)
boot_error = apic->wakeup_secondary_cpu(apicid, start_ip);
- else
+ else {
+ if(!cpu) {
+ early_gdt_descr.address = (unsigned
long)get_cpu_gdt_rw(cpu);
+ initial_stack = idle->thread.sp;
+ }
boot_error = wakeup_cpu_via_init_nmi(cpu, start_ip, apicid,
cpu0_nmi_registered);
-
+ }
return boot_error;
}
