boot time regressed a lot due to misaligned access probe

[Evan Green]

On Wed, Sep 13, 2023 at 8:23 AM Jisheng Zhang <jszhang at 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):

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