[smart] Optimize the vDSO build process and code #10266
Conversation
… components - Implemented vDSO functionality for the RISC-V architecture, including the necessary source files and linker scripts. - Introduced a new `vdso_sys.c` file for RISC-V, containing functions to handle time retrieval using the vDSO mechanism. - Created architecture-specific linker scripts (`vdso.lds.S`) for both AArch64 and RISC-V. - Updated the build system to support vDSO compilation for RISC-V, including necessary adjustments in `SConstruct` files. - Refactored existing vDSO code to improve compatibility and maintainability across architectures. - Adjusted the maximum number of PTY devices in the terminal configuration from 64 to 32 for better resource management. - Fixed minor issues in existing code, including correcting the path for the vDSO shared library and ensuring proper function definitions.
There was a problem hiding this comment.
构建后会生成一些输出文件，但输出路径不在 bsp 的 build 下，而是在 components/lwp/vdso 下，scons -c 还无法清理干净。
建议构建的输出尽量都放到 bsp 的 build 下，实在满足不了的，看看 scons -c 能否清理掉。
components/lwp/vdso/user/build/libvdso.socomponents/lwp/vdso/user/arch/risc-v/.sconsign.dblitecomponents/lwp/vdso/user/arch/risc-v/vdso_sys.oscomponents/lwp/vdso/user/arch/risc-v/vdso.lds
另外对于以下文件的文件名我感觉有些疑惑：
components/lwp/arch/common/vdso.ccomponents/lwp/arch/risc-v/common/vdso_data.ccomponents/lwp/arch/aarch64/common/vdso_data.c
为啥有些叫vdso.c，有些叫vdso_data.c
我理解这些文件中的内容都是为了实现components/lwp/vdso/kernel/vdso.h中声明的函数，所以全部叫vdso.c是不是更好，否则总要让人想一下 data 这里有何区别？
另外提个小建议，[smart] Add vDSO support for RISC-V architecture and refactor related components 这个 commit 修改了太多内容，如果能拆分一下就更好 review 了。
components/lwp/vdso/Kconfig
Outdated
| menuconfig RT_USING_VDSO | ||
| bool "vDSO" | ||
| config RT_USING_VDSO | ||
| bool "vDSO (??Virtual Dynamic Shared Object)" |
| env = Environment(tools=['gcc', 'link'], | ||
| AS = AS, ASFLAGS = AFLAGS, | ||
| CC = CC, CFLAGS = CFLAGS, | ||
| CPP = CPP, AR = AR, |
There was a problem hiding this comment.
行尾有空格，请用 https://github.com/mysterywolf/formatting 检查一下所有修改
| .rela.dyn : ALIGN(8) { *(.rela .rela*) } | ||
|
|
||
| .rodata : { | ||
| *(.rodata*) |
libvdso.so，vdso_sys.os会在
vdso_data.c应该有些不同的，做了一定的划分，后续在看看对vdso整体在代码层面是否做一个重构。目前PR主要还是偏构建层面多一些。 |
|
我看这次改动涉及了 vdso_data.c 能否一并修一下这个 issue : |
有警告吗？我这边打开后没见到。或者这个警告下一个PR再解吧，先能合并 |
不确定你那边为啥看不到，这个 issue 是基于没有合入这个 PR 之前发现的。 |
There was a problem hiding this comment.
Pull Request Overview
This PR optimizes the vDSO build process by refining build scripts for different architectures and relocating weak inline functions for RISCV. In addition, the changes update library naming conventions and remove obsolete build files.
- Updated the VDSO configuration to use a new library name.
- Removed outdated build scripts (xmake.lua and obsolete SConstruct) in favor of updated SConstruct/SConscript logic.
- Moved weak inline functions for RISCV from headers into a dedicated .c file and updated vDSO ABI naming.
Reviewed Changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| components/lwp/vdso/vdso_config.h | Updated the VDSO library path for consistent naming. |
| components/lwp/vdso/user/xmake.lua | Deleted obsolete build script. |
| components/lwp/vdso/user/arch/risc-v/vdso_sys.h | Added RISCV vDSO header with new functionality. |
| components/lwp/vdso/user/arch/risc-v/vdso_sys.c | Added RISCV vDSO implementation and relocated weak inline functions. |
| components/lwp/vdso/user/arch/risc-v/vdso.lds.S | Introduced a new linker script for RISCV. |
| components/lwp/vdso/user/arch/risc-v/SConstruct | Provided SConstruct file for RISCV build support. |
| components/lwp/vdso/user/arch/aarch64/SConstruct | Added SConstruct file for Aarch64 builds. |
| components/lwp/vdso/user/SConstruct | Removed obsolete SConstruct file. |
| components/lwp/vdso/SConscript | Updated build script to support multiple architectures. |
| components/lwp/vdso/Kconfig | Modified configuration strings for clarity. |
| components/lwp/terminal/Kconfig | Reduced default pty device limit from 64 to 32. |
| components/lwp/arch/risc-v/common/vdso_data.c | Added RISCV vDSO data update implementation. |
| components/lwp/arch/common/vdso.c | Refactored vDSO ABI handling and updated code for common vDSO usage. |
| components/lwp/SConscript | Adjusted source file inclusion to reflect the new build structure. |
| components/lwp/Kconfig | Updated lwP configuration description. |
Comments suppressed due to low confidence (2)
components/lwp/Kconfig:2
- [nitpick] The term 'lwP' in the configuration is unconventional; consider using 'LWP (light weight process)' for improved clarity.
bool "lwP(light weight Process)"
components/lwp/vdso/SConscript:9
- [nitpick] Avoid using 'list' as a variable name since it shadows the built-in Python type. Consider renaming it to a less ambiguous identifier such as 'files' or 'entries'.
list = os.listdir(cwd)
|
|
||
| do { | ||
| seq = rt_vdso_read_begin(vd); | ||
| cycles = __arch_get_hw_counter(vd->clock_mode, vd); |
There was a problem hiding this comment.
The function __arch_get_hw_counter is defined without any parameters but is invoked here with two arguments, which will cause a compile-time error. Consider updating the function signature or its invocation to match.
Copilot uses AI. Check for mistakes.
拉取/合并请求描述：(PR description)
[
为什么提交这份PR (why to submit this PR)
vDSO的构建过程是在kernel scons构建中的一个独立环节，会有一定耦合关系。这里对它进行了优化调整，使得它更简单；同时对riscv-vdso的inline weak函数进行调整，不应该把weak应用于inline函数上。
你的解决方案是什么 (what is your solution)
优化vDSO的scons构建过程，同时把riscv-vdso weak inline函数移出，放置到.c文件中。
请提供验证的bsp和config (provide the config and bsp)
]
当前拉取/合并请求的状态 Intent for your PR
必须选择一项 Choose one (Mandatory):
代码质量 Code Quality：
我在这个拉取/合并请求中已经考虑了 As part of this pull request, I've considered the following:
#if 0代码，不包含已经被注释了的代码 All redundant code is removed and cleaned up