fix: make rt_tick_t 64-bit on ARCH_CPU_64BIT and improve clock time precision#10749
fix: make rt_tick_t 64-bit on ARCH_CPU_64BIT and improve clock time precision#10749lygstate wants to merge 1 commit into
Conversation
github-actions
Bot
added
BSP
Kernel
|
👋 感谢您对 RT-Thread 的贡献！Thank you for your contribution to RT-Thread! 为确保代码符合 RT-Thread 的编码规范，请在你的仓库中执行以下步骤运行代码格式化工作流。 🛠 操作步骤 | Steps
完成后，提交将自动更新至 如有问题欢迎联系我们，再次感谢您的贡献！💐 |
📌 Code Review Assignment🏷️ Tag: componentsReviewers: Maihuanyi Changed Files (Click to expand)
🏷️ Tag: components_libcReviewers: GorrayLi mysterywolf Changed Files (Click to expand)
🏷️ Tag: documentationReviewers: CXSforHPU GorrayLi lianux-mm Changed Files (Click to expand)
🏷️ Tag: kernelReviewers: GorrayLi ReviewSun hamburger-os lianux-mm wdfk-prog xu18838022837 Changed Files (Click to expand)
📊 Current Review Status (Last Updated: 2026-06-17 14:14 CST)
📝 Review Instructions
|
lygstate
force-pushed
the
rt_tick_t_64_bit
branch
3 times, most recently
from
0bc44ca to
5839f13
Compare
|
|
lygstate
force-pushed
the
rt_tick_t_64_bit
branch
2 times, most recently
from
c448a01 to
264c203
Compare
lygstate
force-pushed
the
rt_tick_t_64_bit
branch
3 times, most recently
from
28b2e48 to
d8c1321
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR fixes overflow issues in clock_gettime and rt_ktime_boottime_get_ns by improving time conversion precision and enabling 64-bit tick counts on 64-bit architectures. The changes replace the previous resolution-based approach with direct frequency-based calculations to ensure more accurate timing operations.
Key changes:
- Introduces precision-preserving multiplication/division functions (
rt_muldiv_u64,rt_muldiv_u32) - Makes
rt_tick_t64-bit on 64-bit architectures to support long-running programs - Replaces resolution-based time calculations with frequency-based conversions throughout the timing subsystem
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/rttypes.c |
Adds new multiplication/division functions for precision-preserving arithmetic |
include/rttypes.h |
Declares the new rt_muldiv_u64 and rt_muldiv_u32 functions |
include/rtdef.h |
Makes RT_TICK_MAX 64-bit on 64-bit architectures |
components/libc/compilers/common/ctime.c |
Updates timer implementations to use frequency-based calculations |
components/drivers/rtc/dev_soft_rtc.c |
Updates RTC resolution calculation |
components/drivers/ktime/src/ |
Refactors timing subsystem to use frequency instead of resolution |
bsp/rockchip/rk3500/driver/hwtimer/ |
Updates hardware timer interface |
Comments suppressed due to low confidence (2)
src/rttypes.c:1
- Corrected spelling of 'loseless' to 'lossless'.
/*
src/rttypes.c:1
- Corrected spelling of 'loseless' to 'lossless'.
/*
d8c1321 to
9c73d71
Compare
|
@Rbb666 ping |
lygstate
force-pushed
the
rt_tick_t_64_bit
branch
from
9c73d71 to
e13dc95
Compare
lygstate
force-pushed
the
rt_tick_t_64_bit
branch
from
e13dc95 to
ed41d9e
Compare
lygstate
force-pushed
the
rt_tick_t_64_bit
branch
from
ed41d9e to
679bfb5
Compare
lygstate
force-pushed
the
rt_tick_t_64_bit
branch
from
679bfb5 to
5ee5f1d
Compare
lygstate
changed the title
|
ping for review, and avoid newly conflict like #11111 After this, unlock #10735 would be strait-forward |
…recision - Make rt_tick_t and RT_TICK_MAX 64-bit when ARCH_CPU_64BIT is defined - Add rt_muldiv_u64() and rt_muldiv_u32() for lossless counter and nanosecond conversions - Convert boottime, clock_gettime, POSIX timers, and hrtimer paths with clock source frequency instead of scaled resolution - Rewrite rt_clock_time_counter_to_ns() and rt_clock_time_ns_to_counter() with rt_muldiv_u64() for the VDSO path - Remove RT_CLOCK_TIME_RESMUL, res_scale, and scaled-resolution APIs: - res_scale was only a fixed-point multiplier for resolution math (res_scaled = 1e9 * res_scale / freq); it did not customize clock frequency - All in-tree devices always used the default RT_CLOCK_TIME_RESMUL value, so the field provided no real per-board tuning - Converting through res_scaled introduced extra rounding error compared with using get_freq() directly - Custom frequency remains available through rt_clock_time_ops.get_freq, rt_clock_time_source_init(), and rt_clock_time_device_register() - Drop RT_CLOCK_TIME_RESMUL, struct rt_clock_time_device.res_scale, rt_clock_time_get_res_scaled(), rt_clock_time_get_event_res_scaled(), and rt_clock_hrtimer_getres() - Port the original ktime fixes to the clock_time API - Keep soft-timer fallback rounding in _hrtimer_cnt_to_tick() so sub-tick timeouts schedule at least one OS tick - Update clock_time core documentation
lygstate
force-pushed
the
rt_tick_t_64_bit
branch
from
5ee5f1d to
a6b5b05
Compare
| case CLOCK_THREAD_CPUTIME_ID: | ||
| res->tv_sec = 0; | ||
| res->tv_nsec = (rt_clock_time_get_res_scaled() / RT_CLOCK_TIME_RESMUL); | ||
| res->tv_nsec = (rt_uint32_t)(NANOSECOND_PER_SECOND / rt_clock_time_get_freq()); |
There was a problem hiding this comment.
If the clock source frequency is 0, it will divide by zero; it is recommended to add a check.
There was a problem hiding this comment.
good catch, I do it as a plain replace of rt_clock_time_get_res_scaled, so it's exposed a existing issue.
There was a problem hiding this comment.
It can be placed in components/drivers/clock_time/clock_time_core.c as a static helper, or a new clock_time private header such as clock_time_internal.h can be added for clock_boottime.c and clock_hrtimer.c to share.
| unsigned long remain_cnt; | ||
| rt_tick_t remain_cnt; | ||
| rt_uint64_t remain_relative_cnt; | ||
| const rt_uint64_t freq = rt_clock_time_get_freq(); |
There was a problem hiding this comment.
It is recommended to check if freq == 0 first and return an error, similar to rt_clock_boottime_get_*().
| } | ||
|
|
||
| /** | ||
| * https://ridiculousfish.com/blog/posts/labor-of-division-episode-v.html |
There was a problem hiding this comment.
Please ensure your comments conform to Doxygen's comment style. See: https://rt-thread.github.io/rt-thread/page_howto_doxygen.html
There was a problem hiding this comment.
rt_muldiv_u64() is a numeric helper, not a type primitive. Adding src/rttypes.c just for this function makes the rttypes module misleading. Since current users are all clock-time conversions, please keep it private under components/drivers/clock_time/ first. If it is intended as a generic helper, place it together with existing 64-bit division helpers such as rt_do_div / rt_div_u64 instead of exposing it from rttypes.h.
There was a problem hiding this comment.
How about rt_time_muldiv_u64 or other name? or _rt_muldiv_u64 for private usage only?
| return ((rt_uint64_t)q1 << 32) | q0; | ||
| } | ||
|
|
||
| rt_uint64_t rt_muldiv_u64(rt_uint64_t a, rt_uint64_t b, rt_uint64_t c, rt_uint64_t *r) |
There was a problem hiding this comment.
rt_muldiv_u64() is documented as a lossless helper, and the copied u128_div_u64_u64() comment says that when the quotient requires more than 64 bits it should return the max value. However, the current implementation does:
numhi = numhi % den;
when numhi >= den, which silently discards the overflow part and returns a truncated quotient.
For example, rt_muldiv_u64(UINT64_MAX, 2, 1, NULL) returns UINT64_MAX - 1, while the mathematical result is larger than UINT64_MAX. This is risky because the helper is used for time conversions, where truncating an overflowed timeout/counter value can produce a much smaller delay than requested.
Please either implement the documented saturation behavior, e.g. return RT_UINT64_MAX and set the remainder consistently when the quotient does not fit in 64 bits, or explicitly document the truncation semantics and avoid using this helper on paths where overflow is possible.
拉取/合并请求描述：(PR description)
continue work of #9008
Fixes overflow of
clock_gettimert_ktime_boottime_get_nsthoroughlyCurrently the return value of clock_gettime and rt_ktime_boottime_get_ns are restricted by
rt_ktime_cputimer_getres, as the value ofrt_ktime_cputimer_getresare not exactly represent thefrequency of the clock, directly usingrt_ktime_cputimer_getfrqwill ensure the return value of rt_ktime_boottime_get_ns be more exact.And also add function rt_muldiv_u64 rt_muldiv_u32 for not losing precision when convert between
cputimerandhrtimerandnanoseconds.Because
rt_ktime_boottime_get_nsdepends onrt_ktime_cputimer_getcntandrt_ktime_cputimer_getcntdependsrt_tick_getSo the rt_tick_t should be 64bit for long running program.
Use
#if defined(ARCH_CPU_64BIT)is forfuture allow rt_tick_t can be 64bit on 32bit CPU.
Tick count should not be restricted to the CPU
[
为什么提交这份PR (why to submit this PR)
你的解决方案是什么 (what is your solution)
请提供验证的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