[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAAhV-H54bTCatqbabhOuvr6O-oOBa-Z4TGyvoT7uGnTjR6OfDw@mail.gmail.com>
Date: Tue, 6 May 2025 20:44:29 +0800
From: Huacai Chen <chenhuacai@...nel.org>
To: Gregory CLEMENT <gregory.clement@...tlin.com>
Cc: Thomas Bogendoerfer <tsbogend@...ha.franken.de>, Jiaxun Yang <jiaxun.yang@...goat.com>,
Vladimir Kondratiev <vladimir.kondratiev@...ileye.com>, Théo Lebrun <theo.lebrun@...tlin.com>,
Tawfik Bayouk <tawfik.bayouk@...ileye.com>,
Thomas Petazzoni <thomas.petazzoni@...tlin.com>, linux-mips@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] MIPS: SMP: Move the AP sync point before the non-parallel
aware functions
Hi, Gregory,
On Mon, May 5, 2025 at 8:58 PM Gregory CLEMENT
<gregory.clement@...tlin.com> wrote:
>
> When CONFIG_HOTPLUG_PARALLEL is enabled, the code executing before
> cpuhp_ap_sync_alive() is executed in parallel, while after it is
> serialized. The functions set_cpu_sibling_map() and set_cpu_core_map()
> were not designed to be executed in parallel, so by moving the
> cpuhp_ap_sync_alive() before cpuhp_ap_sync_alive(), we then ensure
> they will be called serialized.
set_cpu_sibling_map() and set_cpu_core_map() are the most obvious
functions that need serialization, but you'd better check other places
to make sure there are no similar functions executed in parallel.
Huacai
>
> The measurement done on EyeQ5 did not show any relevant boot time
> increase after applying this patch.
>
> Reported-by: Huacai Chen <chenhuacai@...nel.org>
> Signed-off-by: Gregory CLEMENT <gregory.clement@...tlin.com>
> ---
> Hello,
>
> As discussed last week [1], this is the patch that fixes the potential
> issue with the functions set_cpu_sibling_map() and set_cpu_core_map().
>
> Gregory
>
> [1]: https://lore.kernel.org/all/aBVBHFGH2kICjnT3@alpha.franken.de/
> ---
> arch/mips/kernel/smp.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/mips/kernel/smp.c b/arch/mips/kernel/smp.c
> index 1726744f2b2ec10a44420a7b9b9cd04f06c4d2f6..7901b59d8f60eddefc020cf2a137716af963f09e 100644
> --- a/arch/mips/kernel/smp.c
> +++ b/arch/mips/kernel/smp.c
> @@ -374,13 +374,13 @@ asmlinkage void start_secondary(void)
> calibrate_delay();
> cpu_data[cpu].udelay_val = loops_per_jiffy;
>
> +#ifdef CONFIG_HOTPLUG_PARALLEL
> + cpuhp_ap_sync_alive();
> +#endif
> set_cpu_sibling_map(cpu);
> set_cpu_core_map(cpu);
>
> cpumask_set_cpu(cpu, &cpu_coherent_mask);
> -#ifdef CONFIG_HOTPLUG_PARALLEL
> - cpuhp_ap_sync_alive();
> -#endif
> notify_cpu_starting(cpu);
>
> #ifndef CONFIG_HOTPLUG_PARALLEL
>
> ---
> base-commit: 3b3704261e851e25983860e4c352f1f73786f4ab
> change-id: 20250505-hotplug-paralell-fix-25224cd229c6
>
> Best regards,
> --
> Grégory CLEMENT, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
>
Powered by blists - more mailing lists