| Message ID | 1543534100-3654-1-git-send-email-atish.patra@wdc.com |
|---|---|
| Headers | show |
| Series | Unify CPU topology across ARM64 & RISC-V | expand |
On Thu, Nov 29, 2018 at 03:28:19PM -0800, Atish Patra wrote: > Both RISC-V & ARM64 are using cpu-map device tree to describe > their cpu topology. It's better to move the relevant code to > a common place instead of duplicate code. > > Signed-off-by: Atish Patra <atish.patra@wdc.com> > --- > arch/arm64/include/asm/topology.h | 22 --- > arch/arm64/kernel/topology.c | 303 +------------------------------------- > drivers/base/arch_topology.c | 294 ++++++++++++++++++++++++++++++++++++ Whilst I'm happy seeing this code moved out, I'd like to have an entry in MAINTAINERS so that somebody can review changes from the arm64 side without having to poll a load of different lists. I assume patches would still go via Rafael and Greg, but if we could have Sudeep added as Reviewer for this file, then I'd really appreciate it. Cheers, Will
On Thu, Nov 29, 2018 at 03:28:20PM -0800, Atish Patra wrote: > Currently, there are no topology defined for RISC-V. > Parse the cpu-map node from device tree and setup the > cpu topology. > > CPU topology after applying the patch. > $cat /sys/devices/system/cpu/cpu2/topology/core_siblings_list > 0-3 > $cat /sys/devices/system/cpu/cpu3/topology/core_siblings_list > 0-3 > $cat /sys/devices/system/cpu/cpu3/topology/physical_package_id > 0 > $cat /sys/devices/system/cpu/cpu3/topology/core_id > 3 > That looks simple. Good reuse and thanks for unifying. FWIW, Acked-by:Sudeep Holla <sudeep.holla@arm.com> -- Regards, Sudeep
(Fixed Juri Lelli's email) On Mon, Dec 03, 2018 at 04:58:34PM +0000, Will Deacon wrote: > On Thu, Nov 29, 2018 at 03:28:19PM -0800, Atish Patra wrote: > > Both RISC-V & ARM64 are using cpu-map device tree to describe > > their cpu topology. It's better to move the relevant code to > > a common place instead of duplicate code. > > > > Signed-off-by: Atish Patra <atish.patra@wdc.com> > > --- > > arch/arm64/include/asm/topology.h | 22 --- > > arch/arm64/kernel/topology.c | 303 +------------------------------------- > > drivers/base/arch_topology.c | 294 ++++++++++++++++++++++++++++++++++++ > > Whilst I'm happy seeing this code moved out, I'd like to have an entry in > MAINTAINERS so that somebody can review changes from the arm64 side without > having to poll a load of different lists. I assume patches would still go > via Rafael and Greg, but if we could have Sudeep added as Reviewer for this > file, then I'd really appreciate it. > Thanks Will for pointing this. I agree with you and like to be added as reviewer for this. Since arch_topology.c was mostly added for ARM/ARM64 and by Juri Lelli (who is no longer with ARM) and I did review the initial version, I would like to assume maintenance for this file if both Juri and Greg are OK with it so that I can keep an eye on this as we expect more changes to this both for ARM and RISC-V. I am fine if anyone who has contributed more to this file wants to maintain. -- Regards, Sudeep
On Thu, Nov 29, 2018 at 03:28:19PM -0800, Atish Patra wrote: > Both RISC-V & ARM64 are using cpu-map device tree to describe > their cpu topology. It's better to move the relevant code to > a common place instead of duplicate code. > > Signed-off-by: Atish Patra <atish.patra@wdc.com> > --- > arch/arm64/include/asm/topology.h | 22 --- > arch/arm64/kernel/topology.c | 303 +------------------------------------- > drivers/base/arch_topology.c | 294 ++++++++++++++++++++++++++++++++++++ > include/linux/arch_topology.h | 26 ++++ > include/linux/topology.h | 1 + > 5 files changed, 325 insertions(+), 321 deletions(-) > >From a quick look and diffstat, it looks like a simple move. However I would like to throw this at 0-day bot to test various build configurations so that it's not breaking anything. I can push to a branch in my git @kernel.org but not sure on the build coverage. Anyways will update with any results on build and testing on my side ASAP. -- Regards, Sudeep
On 12/3/18 9:16 AM, Sudeep Holla wrote: > On Thu, Nov 29, 2018 at 03:28:19PM -0800, Atish Patra wrote: >> Both RISC-V & ARM64 are using cpu-map device tree to describe >> their cpu topology. It's better to move the relevant code to >> a common place instead of duplicate code. >> >> Signed-off-by: Atish Patra <atish.patra@wdc.com> >> --- >> arch/arm64/include/asm/topology.h | 22 --- >> arch/arm64/kernel/topology.c | 303 +------------------------------------- >> drivers/base/arch_topology.c | 294 ++++++++++++++++++++++++++++++++++++ >> include/linux/arch_topology.h | 26 ++++ >> include/linux/topology.h | 1 + >> 5 files changed, 325 insertions(+), 321 deletions(-) >> > From a quick look and diffstat, it looks like a simple move. However > I would like to throw this at 0-day bot to test various build configurations > so that it's not breaking anything. I can push to a branch in my git > @kernel.org but not sure on the build coverage. Anyways will update > with any results on build and testing on my side ASAP. > Thanks. It definitely need to rigorous build testing for different configurations. What is the best practice to trigger expansive build tests? linux-kernel@vger.kernel.org is in CC. Regards, Atish > -- > Regards, > Sudeep >
Hi, On 03/12/18 17:12, Sudeep Holla wrote: > (Fixed Juri Lelli's email) > > On Mon, Dec 03, 2018 at 04:58:34PM +0000, Will Deacon wrote: > > On Thu, Nov 29, 2018 at 03:28:19PM -0800, Atish Patra wrote: > > > Both RISC-V & ARM64 are using cpu-map device tree to describe > > > their cpu topology. It's better to move the relevant code to > > > a common place instead of duplicate code. > > > > > > Signed-off-by: Atish Patra <atish.patra@wdc.com> > > > --- > > > arch/arm64/include/asm/topology.h | 22 --- > > > arch/arm64/kernel/topology.c | 303 +------------------------------------- > > > drivers/base/arch_topology.c | 294 ++++++++++++++++++++++++++++++++++++ > > > > Whilst I'm happy seeing this code moved out, I'd like to have an entry in > > MAINTAINERS so that somebody can review changes from the arm64 side without > > having to poll a load of different lists. I assume patches would still go > > via Rafael and Greg, but if we could have Sudeep added as Reviewer for this > > file, then I'd really appreciate it. > > > > Thanks Will for pointing this. I agree with you and like to be added as > reviewer for this. > > Since arch_topology.c was mostly added for ARM/ARM64 and by Juri Lelli > (who is no longer with ARM) and I did review the initial version, I would > like to assume maintenance for this file if both Juri and Greg are OK > with it so that I can keep an eye on this as we expect more changes to > this both for ARM and RISC-V. Sure, this is OK with me. Thanks for keeping an eye on this. Best, - Juri
On 11/29/2018 4:28 PM, Atish Patra wrote: > The cpu-map DT entry in ARM64 can describe the CPU topology in > much better way compared to other existing approaches. RISC-V can > easily adopt this binding to represent it's own CPU topology. > Thus, both cpu-map DT binding and topology parsing code can be > moved to a common location so that RISC-V or any other > architecture can leverage that. > > The relevant discussion regarding unifying cpu topology can be > found in [1]. > > arch_topology seems to be a perfect place to move the common > code. I have not introduced any functional changes in the moved > code. The only downside in this approach is that the capacity > code will be executed for RISC-V as well. But, it will exit > immediately after not able to find the appropriate DT node. If > the overhead is considered too much, we can always compile out > capacity related functions under a different config for the > architectures that do not support them. > > The patches have been tested for RISC-V and compile tested for > ARM64 & x86. > > The socket change[2] is also now part of this series. > > [1] https://lkml.org/lkml/2018/11/6/19 > [2] https://lkml.org/lkml/2018/11/7/918 > > QEMU changes for RISC-V topology are available at > > https://github.com/atishp04/riscv-qemu/tree/cpu_topo > > Apologies for the previous patch series with incorrect title and > was sent only to kernel mailing list due to a bug in my config. > Please ignore that. > > Atish Patra (3): > dt-binding: cpu-topology: Move cpu-map to a common binding. > cpu-topology: Move cpu topology code to common code. > RISC-V: Parse cpu topology during boot. > > Sudeep Holla (1): > Documentation: DT: arm: add support for sockets defining package > boundaries > > .../{arm/topology.txt => cpu/cpu-topology.txt} | 133 +++++++-- > arch/arm64/include/asm/topology.h | 22 -- > arch/arm64/kernel/topology.c | 303 +-------------------- > arch/riscv/Kconfig | 1 + > arch/riscv/kernel/smpboot.c | 3 + > drivers/base/arch_topology.c | 294 ++++++++++++++++++++ > include/linux/arch_topology.h | 26 ++ > include/linux/topology.h | 1 + > 8 files changed, 435 insertions(+), 348 deletions(-) > rename Documentation/devicetree/bindings/{arm/topology.txt => cpu/cpu-topology.txt} (66%) > > -- > 2.7.4 > Seems to test fine on QDF2400. Tested-by: Jeffrey Hugo <jhugo@codeaurora.org> I did see that git am complained about patch #2 - patch:103: space before tab in indent. }; patch:114: space before tab in indent. }; warning: 2 lines add whitespace errors.
Hi, On Thu, Nov 29, 2018 at 03:28:16PM -0800, Atish Patra wrote: > The cpu-map DT entry in ARM64 can describe the CPU topology in > much better way compared to other existing approaches. RISC-V can > easily adopt this binding to represent it's own CPU topology. > Thus, both cpu-map DT binding and topology parsing code can be > moved to a common location so that RISC-V or any other > architecture can leverage that. > > The relevant discussion regarding unifying cpu topology can be > found in [1]. > > arch_topology seems to be a perfect place to move the common > code. I have not introduced any functional changes in the moved > code. The only downside in this approach is that the capacity > code will be executed for RISC-V as well. But, it will exit > immediately after not able to find the appropriate DT node. If > the overhead is considered too much, we can always compile out > capacity related functions under a different config for the > architectures that do not support them. > > The patches have been tested for RISC-V and compile tested for > ARM64 & x86. The cpu-map bindings are used for arch/arm too, and so is arch_topology.c. In fact, it was introduced to allow code-sharing between arm and arm64. Applying patch three breaks arm. Moving the DT parsing to arch_topology.c we have to unify all three architectures. Be aware that arm and arm64 have some differences in how they detect cpu capacities. I think we might have to look at the split of code between arch/* and arch_topology.c again :-/ Morten
On Fri, Dec 07, 2018 at 01:45:21PM +0000, Morten Rasmussen wrote: > Hi, > > On Thu, Nov 29, 2018 at 03:28:16PM -0800, Atish Patra wrote: > > The cpu-map DT entry in ARM64 can describe the CPU topology in > > much better way compared to other existing approaches. RISC-V can > > easily adopt this binding to represent it's own CPU topology. > > Thus, both cpu-map DT binding and topology parsing code can be > > moved to a common location so that RISC-V or any other > > architecture can leverage that. > > > > The relevant discussion regarding unifying cpu topology can be > > found in [1]. > > > > arch_topology seems to be a perfect place to move the common > > code. I have not introduced any functional changes in the moved > > code. The only downside in this approach is that the capacity > > code will be executed for RISC-V as well. But, it will exit > > immediately after not able to find the appropriate DT node. If > > the overhead is considered too much, we can always compile out > > capacity related functions under a different config for the > > architectures that do not support them. > > > > The patches have been tested for RISC-V and compile tested for > > ARM64 & x86. > > The cpu-map bindings are used for arch/arm too, and so is > arch_topology.c. In fact, it was introduced to allow code-sharing > between arm and arm64. Applying patch three breaks arm. > Ah right. Though I remember the whole point of moving these to arch_topology was to share between ARM and ARM64, I completely forgot to check the impact of this for ARM platforms. > Moving the DT parsing to arch_topology.c we have to unify all three > architectures. Be aware that arm and arm64 have some differences in how > they detect cpu capacities. I think we might have to look at the split > of code between arch/* and arch_topology.c again :-/ > Thanks for pointing this out. I completely agree and apologies for forgetting about arm32. Regards, Sudeep
On 12/7/18 5:45 AM, Morten Rasmussen wrote: > Hi, > > On Thu, Nov 29, 2018 at 03:28:16PM -0800, Atish Patra wrote: >> The cpu-map DT entry in ARM64 can describe the CPU topology in >> much better way compared to other existing approaches. RISC-V can >> easily adopt this binding to represent it's own CPU topology. >> Thus, both cpu-map DT binding and topology parsing code can be >> moved to a common location so that RISC-V or any other >> architecture can leverage that. >> >> The relevant discussion regarding unifying cpu topology can be >> found in [1]. >> >> arch_topology seems to be a perfect place to move the common >> code. I have not introduced any functional changes in the moved >> code. The only downside in this approach is that the capacity >> code will be executed for RISC-V as well. But, it will exit >> immediately after not able to find the appropriate DT node. If >> the overhead is considered too much, we can always compile out >> capacity related functions under a different config for the >> architectures that do not support them. >> >> The patches have been tested for RISC-V and compile tested for >> ARM64 & x86. > > The cpu-map bindings are used for arch/arm too, and so is > arch_topology.c. In fact, it was introduced to allow code-sharing > between arm and arm64. Applying patch three breaks arm. > > Moving the DT parsing to arch_topology.c we have to unify all three > architectures. Be aware that arm and arm64 have some differences in how > they detect cpu capacities. I think we might have to look at the split > of code between arch/* and arch_topology.c again :-/ > > Morten > Thank you for bringing this up. I will send a new version and make sure that it works on arm32 as well. Regards, Atish
On 12/5/18 9:53 AM, Jeffrey Hugo wrote: > On 11/29/2018 4:28 PM, Atish Patra wrote: >> The cpu-map DT entry in ARM64 can describe the CPU topology in >> much better way compared to other existing approaches. RISC-V can >> easily adopt this binding to represent it's own CPU topology. >> Thus, both cpu-map DT binding and topology parsing code can be >> moved to a common location so that RISC-V or any other >> architecture can leverage that. >> >> The relevant discussion regarding unifying cpu topology can be >> found in [1]. >> >> arch_topology seems to be a perfect place to move the common >> code. I have not introduced any functional changes in the moved >> code. The only downside in this approach is that the capacity >> code will be executed for RISC-V as well. But, it will exit >> immediately after not able to find the appropriate DT node. If >> the overhead is considered too much, we can always compile out >> capacity related functions under a different config for the >> architectures that do not support them. >> >> The patches have been tested for RISC-V and compile tested for >> ARM64 & x86. >> >> The socket change[2] is also now part of this series. >> >> [1] https://lkml.org/lkml/2018/11/6/19 >> [2] https://lkml.org/lkml/2018/11/7/918 >> >> QEMU changes for RISC-V topology are available at >> >> https://github.com/atishp04/riscv-qemu/tree/cpu_topo >> >> Apologies for the previous patch series with incorrect title and >> was sent only to kernel mailing list due to a bug in my config. >> Please ignore that. >> >> Atish Patra (3): >> dt-binding: cpu-topology: Move cpu-map to a common binding. >> cpu-topology: Move cpu topology code to common code. >> RISC-V: Parse cpu topology during boot. >> >> Sudeep Holla (1): >> Documentation: DT: arm: add support for sockets defining package >> boundaries >> >> .../{arm/topology.txt => cpu/cpu-topology.txt} | 133 +++++++-- >> arch/arm64/include/asm/topology.h | 22 -- >> arch/arm64/kernel/topology.c | 303 +-------------------- >> arch/riscv/Kconfig | 1 + >> arch/riscv/kernel/smpboot.c | 3 + >> drivers/base/arch_topology.c | 294 ++++++++++++++++++++ >> include/linux/arch_topology.h | 26 ++ >> include/linux/topology.h | 1 + >> 8 files changed, 435 insertions(+), 348 deletions(-) >> rename Documentation/devicetree/bindings/{arm/topology.txt => cpu/cpu-topology.txt} (66%) >> >> -- >> 2.7.4 >> > > Seems to test fine on QDF2400. > > Tested-by: Jeffrey Hugo <jhugo@codeaurora.org> > Thanks for verifying the patches. > I did see that git am complained about patch #2 - > > patch:103: space before tab in indent. > }; > patch:114: space before tab in indent. > }; > warning: 2 lines add whitespace errors. > > Thanks for pointing it out. For some reason, cherry-pick did not complain about these. I will fix them. Regards, Atish
The cpu-map DT entry in ARM64 can describe the CPU topology in much better way compared to other existing approaches. RISC-V can easily adopt this binding to represent it's own CPU topology. Thus, both cpu-map DT binding and topology parsing code can be moved to a common location so that RISC-V or any other architecture can leverage that. The relevant discussion regarding unifying cpu topology can be found in [1]. arch_topology seems to be a perfect place to move the common code. I have not introduced any functional changes in the moved code. The only downside in this approach is that the capacity code will be executed for RISC-V as well. But, it will exit immediately after not able to find the appropriate DT node. If the overhead is considered too much, we can always compile out capacity related functions under a different config for the architectures that do not support them. The patches have been tested for RISC-V and compile tested for ARM64 & x86. The socket change[2] is also now part of this series. [1] https://lkml.org/lkml/2018/11/6/19 [2] https://lkml.org/lkml/2018/11/7/918 QEMU changes for RISC-V topology are available at https://github.com/atishp04/riscv-qemu/tree/cpu_topo Apologies for the previous patch series with incorrect title and was sent only to kernel mailing list due to a bug in my config. Please ignore that. Atish Patra (3): dt-binding: cpu-topology: Move cpu-map to a common binding. cpu-topology: Move cpu topology code to common code. RISC-V: Parse cpu topology during boot. Sudeep Holla (1): Documentation: DT: arm: add support for sockets defining package boundaries .../{arm/topology.txt => cpu/cpu-topology.txt} | 133 +++++++-- arch/arm64/include/asm/topology.h | 22 -- arch/arm64/kernel/topology.c | 303 +-------------------- arch/riscv/Kconfig | 1 + arch/riscv/kernel/smpboot.c | 3 + drivers/base/arch_topology.c | 294 ++++++++++++++++++++ include/linux/arch_topology.h | 26 ++ include/linux/topology.h | 1 + 8 files changed, 435 insertions(+), 348 deletions(-) rename Documentation/devicetree/bindings/{arm/topology.txt => cpu/cpu-topology.txt} (66%) -- 2.7.4