lkml.org 
[lkml]   [2023]   [Feb]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH v6 10/11] x86/smpboot: Serialize topology updates for secondary bringup
    Date
    From: David Woodhouse <dwmw@amazon.co.uk>

    If we bring up secondaries in parallel they might get confused unless we
    impose some ordering here:

    [ 1.360149] x86: Booting SMP configuration:
    [ 1.360221] .... node #0, CPUs: #1 #2 #3 #4 #5 #6 #7 #8 #9 #10 #11 #12 #13 #14 #15 #16 #17 #18 #19 #20 #21 #22 #23
    [ 1.366225] .... node #1, CPUs: #24 #25 #26 #27 #28 #29 #30 #31 #32 #33 #34 #35 #36 #37 #38 #39 #40 #41 #42 #43 #44 #45 #46 #47
    [ 1.370219] .... node #0, CPUs: #48 #49 #50 #51 #52 #53 #54 #55 #56 #57 #58 #59 #60 #61 #62 #63 #64 #65 #66 #67 #68 #69 #70 #71
    [ 1.378226] .... node #1, CPUs: #72 #73 #74 #75 #76 #77 #78 #79 #80 #81 #82 #83 #84 #85 #86 #87 #88 #89 #90 #91 #92 #93 #94 #95
    [ 1.382037] Brought 96 CPUs to x86/cpu:kick in 72232606 cycles
    [ 0.104104] smpboot: CPU 26 Converting physical 0 to logical die 1
    [ 0.104104] smpboot: CPU 27 Converting physical 1 to logical package 2
    [ 0.104104] smpboot: CPU 24 Converting physical 1 to logical package 3
    [ 0.104104] smpboot: CPU 27 Converting physical 0 to logical die 2
    [ 0.104104] smpboot: CPU 25 Converting physical 1 to logical package 4
    [ 1.385609] Brought 96 CPUs to x86/cpu:wait-init in 9269218 cycles
    [ 1.395285] Brought CPUs online in 28930764 cycles
    [ 1.395469] smp: Brought up 2 nodes, 96 CPUs
    [ 1.395689] smpboot: Max logical packages: 2
    [ 1.396222] smpboot: Total of 96 processors activated (576000.00 BogoMIPS)

    Do the full topology update in smp_store_cpu_info() under a spinlock
    to ensure that things remain consistent.

    [Usama Arif: fixed rebase conflict]
    Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
    Signed-off-by: Usama Arif <usama.arif@bytedance.com>
    Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
    ---
    arch/x86/include/asm/smp.h | 4 +-
    arch/x86/include/asm/topology.h | 2 -
    arch/x86/kernel/cpu/common.c | 6 +--
    arch/x86/kernel/smpboot.c | 73 ++++++++++++++++++++-------------
    arch/x86/xen/smp_pv.c | 4 +-
    5 files changed, 48 insertions(+), 41 deletions(-)

    diff --git a/arch/x86/include/asm/smp.h b/arch/x86/include/asm/smp.h
    index 9b3c0cd35f5f..1760aa269673 100644
    --- a/arch/x86/include/asm/smp.h
    +++ b/arch/x86/include/asm/smp.h
    @@ -48,8 +48,6 @@ struct smp_ops {
    };

    /* Globals due to paravirt */
    -extern void set_cpu_sibling_map(int cpu);
    -
    #ifdef CONFIG_SMP
    extern struct smp_ops smp_ops;

    @@ -137,7 +135,7 @@ void native_send_call_func_single_ipi(int cpu);
    void x86_idle_thread_init(unsigned int cpu, struct task_struct *idle);

    void smp_store_boot_cpu_info(void);
    -void smp_store_cpu_info(int id);
    +void smp_store_cpu_info(int id, bool force_single_core);

    asmlinkage __visible void smp_reboot_interrupt(void);
    __visible void smp_reschedule_interrupt(struct pt_regs *regs);
    diff --git a/arch/x86/include/asm/topology.h b/arch/x86/include/asm/topology.h
    index 458c891a8273..4bccbd949a99 100644
    --- a/arch/x86/include/asm/topology.h
    +++ b/arch/x86/include/asm/topology.h
    @@ -136,8 +136,6 @@ static inline int topology_max_smt_threads(void)
    return __max_smt_threads;
    }

    -int topology_update_package_map(unsigned int apicid, unsigned int cpu);
    -int topology_update_die_map(unsigned int dieid, unsigned int cpu);
    int topology_phys_to_logical_pkg(unsigned int pkg);
    int topology_phys_to_logical_die(unsigned int die, unsigned int cpu);
    bool topology_is_primary_thread(unsigned int cpu);
    diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
    index 9cfca3d7d0e2..cf1a4eff7a76 100644
    --- a/arch/x86/kernel/cpu/common.c
    +++ b/arch/x86/kernel/cpu/common.c
    @@ -1766,7 +1766,7 @@ static void generic_identify(struct cpuinfo_x86 *c)
    * Validate that ACPI/mptables have the same information about the
    * effective APIC id and update the package map.
    */
    -static void validate_apic_and_package_id(struct cpuinfo_x86 *c)
    +static void validate_apic_id(struct cpuinfo_x86 *c)
    {
    #ifdef CONFIG_SMP
    unsigned int apicid, cpu = smp_processor_id();
    @@ -1777,8 +1777,6 @@ static void validate_apic_and_package_id(struct cpuinfo_x86 *c)
    pr_err(FW_BUG "CPU%u: APIC id mismatch. Firmware: %x APIC: %x\n",
    cpu, apicid, c->initial_apicid);
    }
    - BUG_ON(topology_update_package_map(c->phys_proc_id, cpu));
    - BUG_ON(topology_update_die_map(c->cpu_die_id, cpu));
    #else
    c->logical_proc_id = 0;
    #endif
    @@ -1969,7 +1967,7 @@ void identify_secondary_cpu(struct cpuinfo_x86 *c)
    #ifdef CONFIG_X86_32
    enable_sep_cpu();
    #endif
    - validate_apic_and_package_id(c);
    + validate_apic_id(c);
    x86_spec_ctrl_setup_ap();
    update_srbds_msr();

    diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
    index 77c509e95571..030eaa84aea0 100644
    --- a/arch/x86/kernel/smpboot.c
    +++ b/arch/x86/kernel/smpboot.c
    @@ -182,16 +182,12 @@ static void smp_callin(void)
    apic_ap_setup();

    /*
    - * Save our processor parameters. Note: this information
    - * is needed for clock calibration.
    - */
    - smp_store_cpu_info(cpuid);
    -
    - /*
    + * Save our processor parameters and update topology.
    + * Note: this information is needed for clock calibration.
    * The topology information must be up to date before
    * calibrate_delay() and notify_cpu_starting().
    */
    - set_cpu_sibling_map(raw_smp_processor_id());
    + smp_store_cpu_info(cpuid, false);

    ap_init_aperfmperf();

    @@ -246,6 +242,12 @@ static void notrace start_secondary(void *unused)
    * smp_callout_mask to release them.
    */
    cpu_init_secondary();
    +
    + /*
    + * Even though notify_cpu_starting() will do this, it does so too late
    + * as the AP may already have triggered lockdep splats by then. See
    + * commit 29368e093 ("x86/smpboot: Move rcu_cpu_starting() earlier").
    + */
    rcu_cpu_starting(raw_smp_processor_id());
    x86_cpuinit.early_percpu_clock_init();

    @@ -354,7 +356,7 @@ EXPORT_SYMBOL(topology_phys_to_logical_die);
    * @pkg: The physical package id as retrieved via CPUID
    * @cpu: The cpu for which this is updated
    */
    -int topology_update_package_map(unsigned int pkg, unsigned int cpu)
    +static int topology_update_package_map(unsigned int pkg, unsigned int cpu)
    {
    int new;

    @@ -377,7 +379,7 @@ int topology_update_package_map(unsigned int pkg, unsigned int cpu)
    * @die: The die id as retrieved via CPUID
    * @cpu: The cpu for which this is updated
    */
    -int topology_update_die_map(unsigned int die, unsigned int cpu)
    +static int topology_update_die_map(unsigned int die, unsigned int cpu)
    {
    int new;

    @@ -408,25 +410,7 @@ void __init smp_store_boot_cpu_info(void)
    c->initialized = true;
    }

    -/*
    - * The bootstrap kernel entry code has set these up. Save them for
    - * a given CPU
    - */
    -void smp_store_cpu_info(int id)
    -{
    - struct cpuinfo_x86 *c = &cpu_data(id);
    -
    - /* Copy boot_cpu_data only on the first bringup */
    - if (!c->initialized)
    - *c = boot_cpu_data;
    - c->cpu_index = id;
    - /*
    - * During boot time, CPU0 has this setup already. Save the info when
    - * bringing up AP or offlined CPU0.
    - */
    - identify_secondary_cpu(c);
    - c->initialized = true;
    -}
    +static arch_spinlock_t topology_lock = __ARCH_SPIN_LOCK_UNLOCKED;

    static bool
    topology_same_node(struct cpuinfo_x86 *c, struct cpuinfo_x86 *o)
    @@ -632,7 +616,7 @@ static struct sched_domain_topology_level x86_topology[] = {
    */
    static bool x86_has_numa_in_package;

    -void set_cpu_sibling_map(int cpu)
    +static void set_cpu_sibling_map(int cpu)
    {
    bool has_smt = smp_num_siblings > 1;
    bool has_mp = has_smt || boot_cpu_data.x86_max_cores > 1;
    @@ -711,6 +695,37 @@ void set_cpu_sibling_map(int cpu)
    }
    }

    +/*
    + * The bootstrap kernel entry code has set these up. Save them for
    + * a given CPU
    + */
    +void smp_store_cpu_info(int id, bool force_single_core)
    +{
    + struct cpuinfo_x86 *c = &cpu_data(id);
    +
    + /* Copy boot_cpu_data only on the first bringup */
    + if (!c->initialized)
    + *c = boot_cpu_data;
    + c->cpu_index = id;
    + /*
    + * During boot time, CPU0 has this setup already. Save the info when
    + * bringing up AP or offlined CPU0.
    + */
    + identify_secondary_cpu(c);
    +
    + arch_spin_lock(&topology_lock);
    + BUG_ON(topology_update_package_map(c->phys_proc_id, id));
    + BUG_ON(topology_update_die_map(c->cpu_die_id, id));
    + c->initialized = true;
    +
    + /* For Xen PV */
    + if (force_single_core)
    + c->x86_max_cores = 1;
    +
    + set_cpu_sibling_map(id);
    + arch_spin_unlock(&topology_lock);
    +}
    +
    /* maps the cpu to the sched domain representing multi-core */
    const struct cpumask *cpu_coregroup_mask(int cpu)
    {
    diff --git a/arch/x86/xen/smp_pv.c b/arch/x86/xen/smp_pv.c
    index 6175f2c5c822..09f94f940689 100644
    --- a/arch/x86/xen/smp_pv.c
    +++ b/arch/x86/xen/smp_pv.c
    @@ -71,9 +71,7 @@ static void cpu_bringup(void)
    xen_enable_syscall();
    }
    cpu = smp_processor_id();
    - smp_store_cpu_info(cpu);
    - cpu_data(cpu).x86_max_cores = 1;
    - set_cpu_sibling_map(cpu);
    + smp_store_cpu_info(cpu, true);

    speculative_store_bypass_ht_init();

    --
    2.25.1
    \
     
     \ /
      Last update: 2023-03-27 00:08    [W:8.076 / U:0.028 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site