Date:   Sat, 23 Jul 2022 11:22:01 +0000
From:   <Conor.Dooley@...rochip.com>
To:     <paul.walmsley@...ive.com>, <palmer@...belt.com>,
        <palmer@...osinc.com>, <aou@...s.berkeley.edu>,
        <sudeep.holla@....com>, <catalin.marinas@....com>,
        <will@...nel.org>, <gregkh@...uxfoundation.org>,
        <rafael@...nel.org>
CC:     <Daire.McNamara@...rochip.com>, <Conor.Dooley@...rochip.com>,
        <niklas.cassel@....com>, <damien.lemoal@...nsource.wdc.com>,
        <geert@...ux-m68k.org>, <zong.li@...ive.com>, <kernel@...il.dk>,
        <hahnjo@...njo.de>, <guoren@...nel.org>, <anup@...infault.org>,
        <atishp@...shpatra.org>, <heiko@...ech.de>,
        <philipp.tomsich@...ll.eu>, <robh@...nel.org>, <maz@...nel.org>,
        <viresh.kumar@...aro.org>, <linux-riscv@...ts.infradead.org>,
        <linux-kernel@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>, <Brice.Goglin@...ia.fr>
Subject: Re: [PATCH v4 0/2] Fix RISC-V's arch-topology reporting

On 15/07/2022 18:51, Conor Dooley wrote:
> From: Conor Dooley <conor.dooley@...rochip.com>
> 
> Hey all,
> It's my first time messing around with arch/ code at all, let alone
> more than one arch, so forgive me if I have screwed up how to do a
> migration like this.
> 
> The goal here is the fix the incorrectly reported arch topology on
> RISC-V which seems to have been broken since it was added.
> cpu, package and thread IDs are all currently reported as -1, so tools
> like lstopo think systems have multiple threads on the same core when
> this is not true:
> https://github.com/open-mpi/hwloc/issues/536

Hey,

Not got any feedback on the smpboot changes from the RISC-V side.
I tested it on polarfire, the d1 (with both SMP & !SMP set iirc)
& on the u540. It all looked good to me.

I'd like to have this fixed for v5.20, but there isn't too much
time left before the mw. Not too sure about the cross-tree changes,
does it need an immutable branch or could it go through driver-core?
Catalin suggested removing the CC stable from patch 1/2 & adding it
as a dependency for the 2/2 patch - but obviously that's up to the
committer to sort out.

I guess since it is a fix, it could also go into rc1<

Thanks,
Conor.

> 
> arm64's topology code basically applies to RISC-V too, so it has been
> made generic along with the removal of MPIDR related code, which
> appears to be redudant code since '3102bc0e6ac7 ("arm64: topology: Stop
> using MPIDR for topology information")' replaced the code that actually
> interacted with MPIDR with default values.
> 
> I only built tested for arm{,64} , so hopefully it is not broken when
> used. Testing on both arm64 & !SMP RISC-V would really be appreciated!
> 
> For V2, I dropped the idea of doing a RISC-V specific implementation
> followed by a move to the generic code & just went for the more straight
> forward method of moving to the shared version first. I also dropped the
> RFC.
> 
> V3 moves store_cpu_topology()'s definition down inside the arch check
> alongside the init function so that boot on 32bit arm is not broken.
> 
> V4 has moved the RISC-V boot hart's call to store_cpu_topology() later
> into the boot process it is now right before SMP is brought up (or not
> in the case of !SMP). This prevents calling detect_cache_attributes()
> while we cannot allocate memory.
> 
> V4 is also rebased on next-20220715 to get Sudeep's most recent
> arch_topology patchset.
> 
> Thanks,
> Conor
> 
> Conor Dooley (2):
>   arm64: topology: move store_cpu_topology() to shared code
>   riscv: topology: fix default topology reporting
> 
>  arch/arm64/kernel/topology.c | 40 ------------------------------------
>  arch/riscv/Kconfig           |  2 +-
>  arch/riscv/kernel/smpboot.c  |  3 ++-
>  drivers/base/arch_topology.c | 19 +++++++++++++++++
>  4 files changed, 22 insertions(+), 42 deletions(-)
> 
> 
> base-commit: 6014cfa5bf32cf8c5c58b3cfd5ee0e1542c8a825

Powered by blists - more mailing lists