Message-ID: <7fda05c9-9d4a-6005-0ce5-91bda1bb06c1@bytedance.com>
Date:   Wed, 1 Feb 2023 16:38:14 +0000
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.2
Subject: Re: [External] Re: [PATCH v4 0/9] Parallel CPU bringup for x86_64
Content-Language: en-US
To:     David Woodhouse <dwmw2@infradead.org>,
        Thomas Gleixner <tglx@linutronix.de>
Cc:     Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        Dave Hansen <dave.hansen@linux.intel.com>, x86@kernel.org,
        "H . Peter Anvin" <hpa@zytor.com>,
        Paolo Bonzini <pbonzini@redhat.com>,
        "Paul E . McKenney" <paulmck@kernel.org>,
        linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
        rcu@vger.kernel.org, mimoja@mimoja.de, hewenliang4@huawei.com,
        hushiyuan@huawei.com, luolongjun@huawei.com, hejingxian@huawei.com,
        Tom Lendacky <thomas.lendacky@amd.com>,
        Sean Christopherson <seanjc@google.com>,
        Paul Menzel <pmenzel@molgen.mpg.de>,
        Fam Zheng <fam.zheng@bytedance.com>,
        Punit Agrawal <punit.agrawal@bytedance.com>,
        simon.evans@bytedance.com, liangma@liangbit.com
References: <20220201205328.123066-1-dwmw2@infradead.org>
 <ff876008-b642-4dbc-aa41-1639905e08b6@bytedance.com>
 <1d0ed92ab68409b62a14cd29d0021f92c6e2568a.camel@infradead.org>
From:   Usama Arif <usama.arif@bytedance.com>
In-Reply-To: <1d0ed92ab68409b62a14cd29d0021f92c6e2568a.camel@infradead.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk


On 01/02/2023 15:08, David Woodhouse wrote:
> On Wed, 2023-02-01 at 14:40 +0000, Usama Arif wrote:
>> On 01/02/2022 20:53, David Woodhouse wrote:
>>> Doing the INIT/SIPI/SIPI in parallel for all APs and *then* waiting for
>>> them shaves about 80% off the AP bringup time on a 96-thread 2-socket
>>> Skylake box (EC2 c5.metal) — from about 500ms to 100ms.
>>>
>>> There are more wins to be had with further parallelisation, but this is
>>> the simple part.
>>>
>>
>> Hi,
>>
>> We are interested in reducing the boot time of servers (with kexec), and
>> smpboot takes up a significant amount of time while booting. When
>> testing the patch series (rebased to v6.1) on a server with 128 CPUs
>> split across 2 NUMA nodes, it brought down the smpboot time from ~700ms
>> to 100ms. Adding another cpuhp state for do_wait_cpu_initialized to make
>> sure cpu_init is reached (as done in v1 of the series + using the
>> cpu_finishup_mask) brought it down further to ~30ms.
>>
>> I just wanted to check what was needed to progress the patch series
>> further for review? There weren't any comments on v4 of the patch so I
>> couldn't figure out what more is needed. I think its quite useful to
>> have this working so would be really glad help in anything needed to
>> restart the review.
> 
> 
> I believe the only thing holding it back was the fact that it broke on
> some AMD CPUs.
> 
> We don't *think* there are any remaining software issues; we think it's
> hardware. Either an actual hardware race in CPU or chipset, or perhaps
> even something as simple as a voltage regulator which can't cope with
> an increase in power draw from *all* the CPUs at the same time.
> 
> We have prodded AMD a few times to investigate, but so far to no avail.
> 
> Last time I actually spoke to Thomas in person, I think he agreed that
> we should just merge it and disable the parallel mode for the affected
> AMD CPUs.
>

 From the comments in v3, it seems to affect multiple generations, would 
it be worth proceeding with the patches by disabling it on all AMD CPUs 
to be on the safe side, until the actual issue is found and what causes 
it, and then follow up later if the issue is found by disabling it only 
on affected cpus. Maybe simply do something like below?

diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 0f144773a7fc..6b8884592341 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -1575,7 +1575,8 @@ void __init native_smp_prepare_cpus(unsigned int 
max_cpus)
          * for SEV-ES guests because they can't use CPUID that early.
          */
         if (IS_ENABLED(CONFIG_X86_32) || boot_cpu_data.cpuid_level < 
0x0B ||
-           cc_platform_has(CC_ATTR_GUEST_STATE_ENCRYPT))
+           cc_platform_has(CC_ATTR_GUEST_STATE_ENCRYPT) ||
+           boot_cpu_data.x86_vendor == X86_VENDOR_AMD)
                 do_parallel_bringup = false;

         if (do_parallel_bringup) {


> If you've already rebased to a newer kernel and tested it, perhaps now
> is the time to do just that.

If you would like me to repost the rebased patches to restart the 
reviews (with do_parallel_bringup disabled for AMD), please let me know!

Thanks,
Usama