mbox series

[GIT,PULL] Fix RISC-V's arch-topology reporting

Message ID 4849490e-b362-c13a-c2e4-82acc3268a3f@microchip.com (mailing list archive)
State New, archived
Headers show
Series [GIT,PULL] Fix RISC-V's arch-topology reporting | expand

Pull-request

https://git.kernel.org/pub/scm/linux/kernel/git/conor/linux.git/ tags/riscv-topo-on-6.0-rc1

Message

Conor Dooley Aug. 15, 2022, 10:14 p.m. UTC
Hey Will/Palmer/Sudeep,

Catalin suggested [0] dropping the CC: stable for the arm64 patch and
instead making it a specific prereq of the RISC-V patch & making a PR,
so here we are.. I was still up when -rc1 came out so pushed it last
night to get the test coverage, but LKP seems to not have reported a
build success since early on the 13th so not holding my horses! I built
it again for both ARMs and RISC-V myself.

I tagged it tonight, so it's on conor/linux.git as riscv-topo-on-6.0-rc1
with the prereq specified.

Not sure if you want to merge this too Sudeep or if that's up to Greg?

Thanks,
Conor.

0 - https://lore.kernel.org/linux-riscv/Ytac7G1zlq6WW4jt@arm.com/

The following changes since commit 568035b01cfb107af8d2e4bd2fb9aea22cf5b868:

  Linux 6.0-rc1 (2022-08-14 15:50:18 -0700)

are available in the Git repository at:

  https://git.kernel.org/pub/scm/linux/kernel/git/conor/linux.git/ tags/riscv-topo-on-6.0-rc1

for you to fetch changes up to fbd92809997a391f28075f1c8b5ee314c225557c:

  riscv: topology: fix default topology reporting (2022-08-15 22:07:34 +0100)

----------------------------------------------------------------
Fix RISC-V's topology reporting

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

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.

----------------------------------------------------------------
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(-)

Comments

Sudeep Holla Aug. 16, 2022, 9:12 a.m. UTC | #1
On Mon, Aug 15, 2022 at 10:14:55PM +0000, Conor.Dooley@microchip.com wrote:
> Hey Will/Palmer/Sudeep,
> 
> Catalin suggested [0] dropping the CC: stable for the arm64 patch and
> instead making it a specific prereq of the RISC-V patch & making a PR,
> so here we are.. I was still up when -rc1 came out so pushed it last
> night to get the test coverage, but LKP seems to not have reported a
> build success since early on the 13th so not holding my horses! I built
> it again for both ARMs and RISC-V myself.
> 
> I tagged it tonight, so it's on conor/linux.git as riscv-topo-on-6.0-rc1
> with the prereq specified.
> 
> Not sure if you want to merge this too Sudeep or if that's up to Greg?

I don't have any other patch(other than a fix that I requested Greg to
pick it up directly yesterday) and since you have tagged it already, may be
Greg can pull this or patches directly based on his preference.

--
Regards,
Sudeep
Palmer Dabbelt Aug. 18, 2022, 9:03 p.m. UTC | #2
On Mon, 15 Aug 2022 15:14:55 PDT (-0700), Conor.Dooley@microchip.com wrote:
> Hey Will/Palmer/Sudeep,
> 
> Catalin suggested [0] dropping the CC: stable for the arm64 patch and
> instead making it a specific prereq of the RISC-V patch & making a PR,
> so here we are.. I was still up when -rc1 came out so pushed it last
> night to get the test coverage, but LKP seems to not have reported a
> build success since early on the 13th so not holding my horses! I built
> it again for both ARMs and RISC-V myself.
> 
> I tagged it tonight, so it's on conor/linux.git as riscv-topo-on-6.0-rc1
> with the prereq specified.
> 
> Not sure if you want to merge this too Sudeep or if that's up to Greg?

It's a little bit vague what you're asking for here, so I'm just going 
to kind of guessing here but this on riscv/for-next.  I'm not sure if 
you were looking for me to just merge the arch/riscv bits or if this 
should be on fixes (just looking at the patch makes it look like it 
should be), but I don't want to send up some arm64 code to fixes without 
it being pretty explicit that I should do so

Acked-by: Palmer Dabbelt <palmer@rivosinc.com> # arch/riscv bits, for 6.0-rc

in case someone else wants to send it up before I get back to this, I'm 
fine either way.

Thanks!

> 
> Thanks,
> Conor.
> 
> 0 - https://lore.kernel.org/linux-riscv/Ytac7G1zlq6WW4jt@arm.com/
> 
> The following changes since commit 568035b01cfb107af8d2e4bd2fb9aea22cf5b868:
> 
>   Linux 6.0-rc1 (2022-08-14 15:50:18 -0700)
> 
> are available in the Git repository at:
> 
>   https://git.kernel.org/pub/scm/linux/kernel/git/conor/linux.git/ tags/riscv-topo-on-6.0-rc1
> 
> for you to fetch changes up to fbd92809997a391f28075f1c8b5ee314c225557c:
> 
>   riscv: topology: fix default topology reporting (2022-08-15 22:07:34 +0100)
> 
> ----------------------------------------------------------------
> Fix RISC-V's topology reporting
> 
> 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
> 
> 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.
> 
> ----------------------------------------------------------------
> 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(-)
Conor Dooley Aug. 18, 2022, 9:10 p.m. UTC | #3
On 18/08/2022 22:03, Palmer Dabbelt wrote:
> On Mon, 15 Aug 2022 15:14:55 PDT (-0700), Conor.Dooley@microchip.com wrote:
>> Hey Will/Palmer/Sudeep,
>>
>> Catalin suggested [0] dropping the CC: stable for the arm64 patch and
>> instead making it a specific prereq of the RISC-V patch & making a PR,
>> so here we are.. I was still up when -rc1 came out so pushed it last
>> night to get the test coverage, but LKP seems to not have reported a
>> build success since early on the 13th so not holding my horses! I built
>> it again for both ARMs and RISC-V myself.
>>
>> I tagged it tonight, so it's on conor/linux.git as riscv-topo-on-6.0-rc1
>> with the prereq specified.
>>
>> Not sure if you want to merge this too Sudeep or if that's up to Greg?
> 
> It's a little bit vague what you're asking for here, so I'm just
> going to kind of guessing here but this on riscv/for-next.  I'm not
> sure if you were looking for me to just merge the arch/riscv bits or
> if this should be on fixes (just looking at the patch makes it look
> like it should be), but I don't want to send up some arm64 code to
> fixes without it being pretty explicit that I should do so

Ahh sorry, I completely forgot to mention that Will said "for 5.21" in
his response to my original patchset. for-next sounds fine to me - all
in-tree devicetrees had the optional properties added to them that solve
the issue - and they were AUTOSEL'ed too from what I can see.

Thanks & sorry!
Conor.

> 
> Acked-by: Palmer Dabbelt <palmer@rivosinc.com> # arch/riscv bits, for 6.0-rc
> 
> in case someone else wants to send it up before I get back to this,
> I'm fine either way.
> 
> Thanks!
> 
>>
>> Thanks,
>> Conor.
>>
>> 0 - https://lore.kernel.org/linux-riscv/Ytac7G1zlq6WW4jt@arm.com/
>>
>> The following changes since commit 568035b01cfb107af8d2e4bd2fb9aea22cf5b868:
>>
>>   Linux 6.0-rc1 (2022-08-14 15:50:18 -0700)
>>
>> are available in the Git repository at:
>>
>>   https://git.kernel.org/pub/scm/linux/kernel/git/conor/linux.git/ tags/riscv-topo-on-6.0-rc1
>>
>> for you to fetch changes up to fbd92809997a391f28075f1c8b5ee314c225557c:
>>
>>   riscv: topology: fix default topology reporting (2022-08-15 22:07:34 +0100)
>>
>> ----------------------------------------------------------------
>> Fix RISC-V's topology reporting
>>
>> 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
>>
>> 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.
>>
>> ----------------------------------------------------------------
>> 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(-)
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
Palmer Dabbelt Aug. 18, 2022, 9:15 p.m. UTC | #4
On Thu, 18 Aug 2022 14:10:54 PDT (-0700), Conor.Dooley@microchip.com wrote:
> 
> 
> On 18/08/2022 22:03, Palmer Dabbelt wrote:
>> On Mon, 15 Aug 2022 15:14:55 PDT (-0700), Conor.Dooley@microchip.com wrote:
>>> Hey Will/Palmer/Sudeep,
>>>
>>> Catalin suggested [0] dropping the CC: stable for the arm64 patch and
>>> instead making it a specific prereq of the RISC-V patch & making a PR,
>>> so here we are.. I was still up when -rc1 came out so pushed it last
>>> night to get the test coverage, but LKP seems to not have reported a
>>> build success since early on the 13th so not holding my horses! I built
>>> it again for both ARMs and RISC-V myself.
>>>
>>> I tagged it tonight, so it's on conor/linux.git as riscv-topo-on-6.0-rc1
>>> with the prereq specified.
>>>
>>> Not sure if you want to merge this too Sudeep or if that's up to Greg?
>> 
>> It's a little bit vague what you're asking for here, so I'm just
>> going to kind of guessing here but this on riscv/for-next.  I'm not
>> sure if you were looking for me to just merge the arch/riscv bits or
>> if this should be on fixes (just looking at the patch makes it look
>> like it should be), but I don't want to send up some arm64 code to
>> fixes without it being pretty explicit that I should do so
> 
> Ahh sorry, I completely forgot to mention that Will said "for 5.21" in
> his response to my original patchset. for-next sounds fine to me - all
> in-tree devicetrees had the optional properties added to them that solve
> the issue - and they were AUTOSEL'ed too from what I can see.

OK, sounds like we're good then.

> 
> Thanks & sorry!
> Conor.
> 
>> 
>> Acked-by: Palmer Dabbelt <palmer@rivosinc.com> # arch/riscv bits, for 6.0-rc
>> 
>> in case someone else wants to send it up before I get back to this,
>> I'm fine either way.
>> 
>> Thanks!
>> 
>>>
>>> Thanks,
>>> Conor.
>>>
>>> 0 - https://lore.kernel.org/linux-riscv/Ytac7G1zlq6WW4jt@arm.com/
>>>
>>> The following changes since commit 568035b01cfb107af8d2e4bd2fb9aea22cf5b868:
>>>
>>>   Linux 6.0-rc1 (2022-08-14 15:50:18 -0700)
>>>
>>> are available in the Git repository at:
>>>
>>>   https://git.kernel.org/pub/scm/linux/kernel/git/conor/linux.git/ tags/riscv-topo-on-6.0-rc1
>>>
>>> for you to fetch changes up to fbd92809997a391f28075f1c8b5ee314c225557c:
>>>
>>>   riscv: topology: fix default topology reporting (2022-08-15 22:07:34 +0100)
>>>
>>> ----------------------------------------------------------------
>>> Fix RISC-V's topology reporting
>>>
>>> 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
>>>
>>> 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.
>>>
>>> ----------------------------------------------------------------
>>> 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(-)
>> 
>> _______________________________________________
>> linux-riscv mailing list
>> linux-riscv@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-riscv
Conor Dooley Sept. 7, 2022, 4:39 p.m. UTC | #5
On 15/08/2022 23:14, Conor Dooley wrote:
> Hey Will/Palmer/Sudeep,
> 
> Catalin suggested [0] dropping the CC: stable for the arm64 patch and
> instead making it a specific prereq of the RISC-V patch & making a PR,
> so here we are.. I was still up when -rc1 came out so pushed it last
> night to get the test coverage, but LKP seems to not have reported a
> build success since early on the 13th so not holding my horses! I built
> it again for both ARMs and RISC-V myself.
> 
> I tagged it tonight, so it's on conor/linux.git as riscv-topo-on-6.0-rc1
> with the prereq specified.
> 
> Not sure if you want to merge this too Sudeep or if that's up to Greg?

What's the story with this from an arm64 & topology PoV?
Palmer merged this into riscv/for-next a couple weeks ago, so just
wondering what the craic is on the other fronts.

Thanks,
Conor.

> 
> Thanks,
> Conor.
> 
> 0 - https://lore.kernel.org/linux-riscv/Ytac7G1zlq6WW4jt@arm.com/
> 
> The following changes since commit 568035b01cfb107af8d2e4bd2fb9aea22cf5b868:
> 
>   Linux 6.0-rc1 (2022-08-14 15:50:18 -0700)
> 
> are available in the Git repository at:
> 
>   https://git.kernel.org/pub/scm/linux/kernel/git/conor/linux.git/ tags/riscv-topo-on-6.0-rc1
> 
> for you to fetch changes up to fbd92809997a391f28075f1c8b5ee314c225557c:
> 
>   riscv: topology: fix default topology reporting (2022-08-15 22:07:34 +0100)
> 
> ----------------------------------------------------------------
> Fix RISC-V's topology reporting
> 
> 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
> 
> 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.
> 
> ----------------------------------------------------------------
> 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(-)
Will Deacon Sept. 7, 2022, 4:41 p.m. UTC | #6
On Wed, Sep 07, 2022 at 04:39:31PM +0000, Conor.Dooley@microchip.com wrote:
> On 15/08/2022 23:14, Conor Dooley wrote:
> > Hey Will/Palmer/Sudeep,
> > 
> > Catalin suggested [0] dropping the CC: stable for the arm64 patch and
> > instead making it a specific prereq of the RISC-V patch & making a PR,
> > so here we are.. I was still up when -rc1 came out so pushed it last
> > night to get the test coverage, but LKP seems to not have reported a
> > build success since early on the 13th so not holding my horses! I built
> > it again for both ARMs and RISC-V myself.
> > 
> > I tagged it tonight, so it's on conor/linux.git as riscv-topo-on-6.0-rc1
> > with the prereq specified.
> > 
> > Not sure if you want to merge this too Sudeep or if that's up to Greg?
> 
> What's the story with this from an arm64 & topology PoV?
> Palmer merged this into riscv/for-next a couple weeks ago, so just
> wondering what the craic is on the other fronts.

If it's merged in the riscv tree, then I guess we don't need to do anything
on the arm64 side. It would be handy if it's on its own branch, however,
just in case we run into a conflict later on during the cycle.

Catalin -- this is the series we spoke about the other day touching the
topology code.

Cheers,

Will
Conor Dooley Sept. 7, 2022, 4:44 p.m. UTC | #7
On 07/09/2022 17:41, Will Deacon wrote:
> On Wed, Sep 07, 2022 at 04:39:31PM +0000, Conor.Dooley@microchip.com wrote:
>> On 15/08/2022 23:14, Conor Dooley wrote:
>>> Hey Will/Palmer/Sudeep,
>>>
>>> Catalin suggested [0] dropping the CC: stable for the arm64 patch and
>>> instead making it a specific prereq of the RISC-V patch & making a PR,
>>> so here we are.. I was still up when -rc1 came out so pushed it last
>>> night to get the test coverage, but LKP seems to not have reported a
>>> build success since early on the 13th so not holding my horses! I built
>>> it again for both ARMs and RISC-V myself.
>>>
>>> I tagged it tonight, so it's on conor/linux.git as riscv-topo-on-6.0-rc1
>>> with the prereq specified.
>>>
>>> Not sure if you want to merge this too Sudeep or if that's up to Greg?
>>
>> What's the story with this from an arm64 & topology PoV?
>> Palmer merged this into riscv/for-next a couple weeks ago, so just
>> wondering what the craic is on the other fronts.
> 
> If it's merged in the riscv tree, then I guess we don't need to do anything
> on the arm64 side. It would be handy if it's on its own branch, however,
> just in case we run into a conflict later on during the cycle.

https://git.kernel.org/pub/scm/linux/kernel/git/conor/linux.git/log/?h=arch-topo

FWIW it's here as 2 commits on top of v6.0-rc1

> 
> Catalin -- this is the series we spoke about the other day touching the
> topology code.
> 
> Cheers,
> 
> Will
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv