-
Notifications
You must be signed in to change notification settings - Fork 1.5k
sched/hrtimer: Fix functional correctness issues and improve the performance. #18224
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
I see lots of update that is not necessary, do you have performance or function comparison between the current implementation and your change? |
088a0b9 to
9ce283a
Compare
I have spent a significant amount of time testing and fixing functional correctness issues in this implementation. These issues should have been fixed during the earlier testing phase or can be avoided by the implementation #17675. Please don't waste our time anymore. As you suggested, I won't waste time "showing off" the performance improvements. If anyone else is interested, I'll provide the performance test data. If you don't believe me, you can test it yourself. |
9ce283a to
1d17973
Compare
1d17973 to
676c55f
Compare
This commit inlined the `hrtimer_start` to allow the compiler to optimize at least 1 branch in hrtimer_start. Signed-off-by: ouyangxiangzhen <ouyangxiangzhen@xiaomi.com>
74e9144 to
ca69a31
Compare
| #endif | ||
|
|
||
| #ifdef CONFIG_SCHED_TICKLESS | ||
| bool g_wdtimernested; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we still need g_wdtimernested
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The in_expiration parameter was added because we can not restart the hrtimer within its callback function; instead, we should return the nanoseconds of the next delay. Its semantics are different from g_wdtimernested.
This commit renamed the hrtimer_is_armed to hrtimer_is_pending, which is more accurate in the semamtic, and simplify it. Signed-off-by: ouyangxiangzhen <ouyangxiangzhen@xiaomi.com>
This commit simplified the rbtree in hrtimer and provided better branchless compare function. Signed-off-by: ouyangxiangzhen <ouyangxiangzhen@xiaomi.com>
When a hrtimer is removed and then re-enqueued, if the removed hrtimer is the head node and the re-enqueued node is not the head node, the hardware timer still needs to be reset. This patch fixes this issue and simplifies the enqueuing. Signed-off-by: ouyangxiangzhen <ouyangxiangzhen@xiaomi.com>
This commit simplified the hrtimer_process. Signed-off-by: ouyangxiangzhen <ouyangxiangzhen@xiaomi.com>
This commit fixed the overflow check and renamed the period to delay, since the callback return value is the next delay not the period. Signed-off-by: ouyangxiangzhen <ouyangxiangzhen@xiaomi.com>
Determining whether a red-black tree node is the left-mode node using `RB_LEFT(hrtimer, node) == NULL` is functionally incorrect. This is because the left node of any leaf node can also be NULL. For example, in the following rbtree: 5 / \ 3 7 \ 4 the left node of the right-most node 7 would also be NULL. To avoid extra performance overhead to find the left-most node when the rb-tree changed, this commit used `g_cached_first` to cache the left-most node. Signed-off-by: ouyangxiangzhen <ouyangxiangzhen@xiaomi.com>
If there are no timers in the hrtimer queue, we should cancel the timers to avoid unnecessary timer interruptions. Since we currently do not have a timer cancellation interface, we can achieve cancellation by setting the timer to its maximum value. Signed-off-by: ouyangxiangzhen <ouyangxiangzhen@xiaomi.com>
This commit added the guard timer for hrtimer. The guard timer uses a small memory footprint, offering two main advantages: - Reduced branches checking for an empty hrtimer queue, simplifying code implementation and improving the performance. - Additional health monitoring allows the system to enter a safe state in case of time acquisition errors, and supports custom error handling callback functions. Signed-off-by: ouyangxiangzhen <ouyangxiangzhen@xiaomi.com>
999ea24 to
1816155
Compare
This commit simplified the timer expiration for hrtimer. Signed-off-by: ouyangxiangzhen <ouyangxiangzhen@xiaomi.com>
This commit updated the comments. Signed-off-by: ouyangxiangzhen <ouyangxiangzhen@xiaomi.com>
This commit simplified the hrtimer. Signed-off-by: ouyangxiangzhen <ouyangxiangzhen@xiaomi.com>
1816155 to
6d840d0
Compare
This commit provided workaround for incorrect SCHED_RR behavior in tickless mode. Signed-off-by: ouyangxiangzhen <ouyangxiangzhen@xiaomi.com>
Since the type of the `NSEC_PER_TICK` is long, we should convert it explicitly, or the `div_const` will not work as intended (it requires that the second argument `base` is `uint32_t`). Signed-off-by: ouyangxiangzhen <ouyangxiangzhen@xiaomi.com>
102480a to
9c8a9f3
Compare
Summary
This PR is part of the #17675, it introduces significant improvements to the high-resolution timer (hrtimer) subsystem and its integration with the watchdog timer system. The key changes include:
Guard Timer Implementation: Added a guard timer (
g_hrtimer_guard) that always remains in the timer queue with maximum expiration time. This simplifies empty queue checks, reduces branch conditions, and provides health monitoring capabilities for time acquisition errors.Watchdog-HRTimer Integration: Replaced the separate scheduler tick timer with direct hrtimer integration for watchdog processing. The watchdog now uses its own hrtimer (
g_wdog_hrtimer) instead of a separate tick-based mechanism, simplifying the architecture.Red-Black Tree Optimization: Fixed left-most node detection in the hrtimer RB-tree by introducing
g_cached_firstcaching, eliminating incorrect NULL checks on left child nodes.API Clarifications: Updated function comments and signatures for better documentation, particularly for
hrtimer_cancel()andhrtimer_cancel_sync()to clarify ownership semantics and concurrency considerations.Code Simplification: Removed redundant timer processing logic, inlined
hrtimer_start()for performance, and consolidated timer expiration handling between tick and tickless modes.Impact
hrtimer_start()eliminates at least 1 branch.sched_timer.c) and unifying watchdog processing under hrtimer.sched_timer.c, renamedsched_tickexpiration.ctosched_processtickless.c).Test
Tested on
rv-virt:smp,ostestpassed, parallel stressed hrtimer test passed.