linux-kernel - Re: [PATCH] MIPS: SMP: Implement parallel CPU bring up for EyeQ

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAAhV-H65b5Ae-cCYYHTx0QBhYJ_fzSVLFGY0RH1PCq0XbvNPQA@mail.gmail.com>
Date: Wed, 30 Apr 2025 15:21:23 +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: Implement parallel CPU bring up for EyeQ

On Wed, Apr 30, 2025 at 3:09 PM Gregory CLEMENT
<gregory.clement@...tlin.com> wrote:
>
> Hello Huacai,
>
> > Hi, Gregory,
> >
> > On Sun, Apr 27, 2025 at 6:13 PM Huacai Chen <chenhuacai@...nel.org> wrote:
> >>
> >> Hi, Gregory and Thomas,
> >>
> >> I'm sorry I'm late, but I have some questions about this patch.
> >>
> >> On Mon, Apr 14, 2025 at 3:12 AM Gregory CLEMENT
> >> <gregory.clement@...tlin.com> wrote:
> >> >
> >> > Added support for starting CPUs in parallel on EyeQ to speed up boot time.
> >> >
> >> > On EyeQ5, booting 8 CPUs is now ~90ms faster.
> >> > On EyeQ6, booting 32 CPUs is now ~650ms faster.
> >> >
> >> > Signed-off-by: Gregory CLEMENT <gregory.clement@...tlin.com>
> >> > ---
> >> > Hello,
> >> >
> >> > This patch allows CPUs to start in parallel. It has been tested on
> >> > EyeQ5 and EyeQ6, which are both MIPS64 and use the I6500 design. These
> >> > systems use CPS to support SMP.
> >> >
> >> > As noted in the commit log, on EyeQ6, booting 32 CPUs is now ~650ms
> >> > faster.
> >> >
> >> > Currently, this support is only for EyeQ SoC. However, it should also
> >> > work for other CPUs using CPS. I am less sure about MT ASE support,
> >> > but this patch can be a good starting point. If anyone wants to add
> >> > support for other systems, I can share some ideas, especially for the
> >> > MIPS_GENERIC setup that needs to handle both types of SMP setups.
> >> >
> [...]
> >> >   * A logical cpu mask containing only one VPE per core to
> >> > @@ -74,6 +76,8 @@ static cpumask_t cpu_core_setup_map;
> >> >
> >> >  cpumask_t cpu_coherent_mask;
> >> >
> >> > +struct cpumask __cpu_primary_thread_mask __read_mostly;
> >> > +
> >> >  unsigned int smp_max_threads __initdata = UINT_MAX;
> >> >
> >> >  static int __init early_nosmt(char *s)
> >> > @@ -374,10 +378,15 @@ asmlinkage void start_secondary(void)
> >> >         set_cpu_core_map(cpu);
> >> >
> >> >         cpumask_set_cpu(cpu, &cpu_coherent_mask);
> >> > +#ifdef CONFIG_HOTPLUG_PARALLEL
> >> > +       cpuhp_ap_sync_alive();
> >> This is a "synchronization point" due to the description from commit
> >> 9244724fbf8ab394a7210e8e93bf037abc, which means things are parallel
> >> before this point and serialized after this point.
> >>
> >> But unfortunately, set_cpu_sibling_map() and set_cpu_core_map() cannot
> >> be executed in parallel. Maybe you haven't observed problems, but in
> >> theory it's not correct.
>
> I am working on it. To address your remark, I have a few options that I
> evaluate.
I suggest to revert this patch temporary in mips-next.

Huacai

>
> > I don't know whether you have done reboot tests (for ~1000 times),
> > Jiaxun Yang submitted similar patches for LoongArch [1], but during
> > reboot tests we encountered problems that I have described in my
> > previous reply.
> >
> > [1] https://lore.kernel.org/loongarch/20240716-loongarch-hotplug-v3-0-af59b3bb35c8@flygoat.com/
>
> I saw that series and I wondered why the last patch was not merged.
>
> I performed around 100 tests so far without encountering any issues; I
> plan to automate them further to gather more data.
>
> Gregpory
>
> >
> > Huacai
> >
> >>
> >> Huacai
> >>
> >> > +#endif
> >> >         notify_cpu_starting(cpu);
> >> >
> >> > +#ifndef CONFIG_HOTPLUG_PARALLEL
> >> >         /* Notify boot CPU that we're starting & ready to sync counters */
> >> >         complete(&cpu_starting);
> >> > +#endif
> >> >
> >> >         synchronise_count_slave(cpu);
> >> >
> >> > @@ -386,11 +395,13 @@ asmlinkage void start_secondary(void)
> >> >
> >> >         calculate_cpu_foreign_map();
> >> >
> >> > +#ifndef CONFIG_HOTPLUG_PARALLEL
> >> >         /*
> >> >          * Notify boot CPU that we're up & online and it can safely return
> >> >          * from __cpu_up
> >> >          */
> >> >         complete(&cpu_running);
> >> > +#endif
> >> >
> >> >         /*
> >> >          * irq will be enabled in ->smp_finish(), enabling it too early
> >> > @@ -447,6 +458,12 @@ void __init smp_prepare_boot_cpu(void)
> >> >         set_cpu_online(0, true);
> >> >  }
> >> >
> >> > +#ifdef CONFIG_HOTPLUG_PARALLEL
> >> > +int arch_cpuhp_kick_ap_alive(unsigned int cpu, struct task_struct *tidle)
> >> > +{
> >> > +       return mp_ops->boot_secondary(cpu, tidle);
> >> > +}
> >> > +#else
> >> >  int __cpu_up(unsigned int cpu, struct task_struct *tidle)
> >> >  {
> >> >         int err;
> >> > @@ -466,6 +483,7 @@ int __cpu_up(unsigned int cpu, struct task_struct *tidle)
> >> >         wait_for_completion(&cpu_running);
> >> >         return 0;
> >> >  }
> >> > +#endif
> >> >
> >> >  #ifdef CONFIG_PROFILING
> >> >  /* Not really SMP stuff ... */
> >> >
> >> > ---
> >> > base-commit: 0af2f6be1b4281385b618cb86ad946eded089ac8
> >> > change-id: 20250411-parallel-cpu-bringup-78999a9235ea
> >> >
> >> > Best regards,
> >> > --
> >> > Grégory CLEMENT, Bootlin
> >> > Embedded Linux and Kernel engineering
> >> > https://bootlin.com
> >> >
> >> >
>
> --
> Grégory CLEMENT, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com