Re: boot time regressed a lot due to misaligned access probe

From: Jisheng Zhang <jszhang@kernel.org>
To: Evan Green <evan@rivosinc.com>
Cc: Ben Dooks <ben.dooks@codethink.co.uk>,
	linux-riscv@lists.infradead.org,
	PalmerDabbelt <palmer@dabbelt.com>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Albert Ou <aou@eecs.berkeley.edu>
Subject: Re: boot time regressed a lot due to misaligned access probe
Date: Fri, 15 Sep 2023 08:55:42 +0800	[thread overview]
Message-ID: <ZQOrjszzxsO8YJag@xhacker> (raw)
In-Reply-To: <CALs-HssesyM7RJVaoHhPFk6SQP-J1pHF79rseYDZp=A++Cuy3g@mail.gmail.com>

On Wed, Sep 13, 2023 at 12:50:54PM -0700, Evan Green wrote:
> On Wed, Sep 13, 2023 at 8:23 AM Jisheng Zhang <jszhang@kernel.org> wrote:
> >
> > On Wed, Sep 13, 2023 at 11:46:28AM +0100, Ben Dooks wrote:
> > > On 13/09/2023 01:14, Jisheng Zhang wrote:
> > > > Hi all,
> > > >
> > > > Probing one cpu for misaligned access cost about 0.06s, so it will cost
> > > > about 3.8s on platforms with 64 CPUs, for example, milkv pioneer which
> > > > is powered by sg2042.
> > > >
> > > > I'm not sure the reason of probing misaligned access for all CPUs. If
> > > > the HW doesn't behave as SMP from misalligned access side, then unless
> > > > userspace processes force cpu affinity, they always suffer from this
> > > > non-SMP pain.
> > > >
> > > > So, can we only probe the boot cpu?
> 
> Hi Jisheng,
> Thanks for identifying this regression. I'd prefer to keep the probing
> on each cpu, as I don't think it's safe to assume behavior is the same
> across all cores. But there's no reason this needs to be done
> serially, we should be able to do the checking in parallel on each
> cpu. I don't have a physical 64-core system, but I experimented with
> qemu a bit:
> 
> With misaligned probing
> [    0.558930] smp: Bringing up secondary CPUs ...
> [    7.635580] smp: Brought up 1 node, 64 CPUs
> 
> With no misaligned probing
> [    0.473012] smp: Bringing up secondary CPUs ...
> [    5.438450] smp: Brought up 1 node, 64 CPUs
> 
> With change below:
> [    0.615684] smp: Bringing up secondary CPUs ...
> [    5.489045] smp: Brought up 1 node, 64 CPUs
> 
> I also commented out the pr_info() in my testing, mostly to keep the
> UART out of the way. We should strive to improve the smp core bringup
> time in general, but hopefully with this the misaligned probing won't
> be making it worse. If this works for you I can clean it up and submit
> a patch (sorry gmail mangles the diff):

The patch improved the boot time a lot! Thanks

Feel free to add:
Tested-by: Jisheng Zhang <jszhang@kernel.org>
> 
> diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
> index 1b8da4e40a4d..7dce30b7c868 100644
> --- a/arch/riscv/kernel/smpboot.c
> +++ b/arch/riscv/kernel/smpboot.c
> @@ -223,8 +223,18 @@ int __cpu_up(unsigned int cpu, struct task_struct *tidle)
>         return ret;
>  }
> 
> +static void check_unaligned_access_cpu(void *unused)
> +{
> +       unsigned int cpu = smp_processor_id();
> +
> +       /* Someone has to stay behind and tend the jiffies. */
> +       if (cpu != 0)
> +               check_unaligned_access(cpu);
> +}
> +
>  void __init smp_cpus_done(unsigned int max_cpus)
>  {
> +       on_each_cpu(check_unaligned_access_cpu, NULL, 0);
>  }
> 
>  /*
> @@ -246,7 +256,6 @@ asmlinkage __visible void smp_callin(void)
> 
>         numa_add_cpu(curr_cpuid);
>         set_cpu_online(curr_cpuid, 1);
> -       check_unaligned_access(curr_cpuid);
> 
>         if (has_vector()) {
>                 if (riscv_v_setup_vsize())
> 
> -Evan

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv