sched: Fix the timer reprogramming and SCHED_RR issues. - #20057
Open
Fix-Point wants to merge 6 commits into
Open
sched: Fix the timer reprogramming and SCHED_RR issues.#20057Fix-Point wants to merge 6 commits into
Fix-Point wants to merge 6 commits into
Conversation
…= CLOCK_MAX When maxticks equals CLOCK_MAX (all bits set), the loop that builds the mask by (*mask << 1) | 1 never terminates because the shifted value wraps around to the same mask value, making next > maxticks always false. Replace the loop with a single flsx-based expression that computes the mask directly, which naturally covers the CLOCK_MAX case. Signed-off-by: ouyangxiangzhen <ouyangxiangzhen@xiaomi.com>
Use round-up logic in clkcnt_delta_time2cnt() to prevent timer sleep duration being too short due to truncation. Signed-off-by: ouyangxiangzhen <ouyangxiangzhen@xiaomi.com>
…rtimer In hrtimer_start_absolute, when a pending hrtimer is removed (was the head) and reinserted with a later expiration time, the reprogram flag remains true but the hrtimer is no longer the earliest timer in the queue. The old code passed hrtimer->expired to hrtimer_reprogram, which was incorrect. Use hrtimer_get_first()->expired to ensure the hardware timer is reprogrammed with the actual earliest timer's expiration time. Signed-off-by: ouyangxiangzhen <ouyangxiangzhen@xiaomi.com>
In tickless mode, the scheduler timer is stopped whenever the currently running task requires no time slicing (CLOCK_MAX). When a SCHED_RR task was later switched in, nothing re-armed the timer, so the task could run indefinitely without round-robin rotation. Reassess the scheduler timer in nxsched_switch_context() before the context switch when the task being switched in uses round-robin scheduling, so that the timer is always armed while an RR task is running. Hooking into nxsched_switch_context() covers all context switch paths (task context switch, interrupt exit, syscall and task exit) since every architecture calls it on every switch. Signed-off-by: ouyangxiangzhen <ouyangxiangzhen@xiaomi.com>
In tickless mode, the scheduler timer is stopped whenever the currently running task requires no time slicing (CLOCK_MAX). When a SCHED_RR task was later switched in, nothing re-armed the timer, so the task could run indefinitely without round-robin rotation. Also, when a SCHED_RR task was preempted, its timeslice counter was not decremented for the time already consumed, effectively giving the task "bonus" CPU time when resumed. Solve both by performing RR accounting on context switches: - nxsched_suspend_roundrobin() charges the elapsed execution time against the timeslice of the RR task being switched out - nxsched_resume_roundrobin() restarts the scheduler timer for the remaining timeslice of the RR task being switched in, so the timer is always armed while an RR task is running This also removes the previous workaround in nxsched_process_timer that triggered the scheduler on every timer tick. Signed-off-by: ouyangxiangzhen <ouyangxiangzhen@xiaomi.com>
Fix-Point
requested review from
GUIDINGLI,
gustavonihei,
pussuw and
xiaoxiang781216
as code owners
September 4, 2026 03:40
|
Fix checkpatch "Missing blank line after declarations" errors in drivers/timers/arch_timer.c and sched/sched/sched_processtickless.c. These are pre-existing issues, not introduced by the recent tickless RR series. Signed-off-by: ouyangxiangzhen <ouyangxiangzhen@xiaomi.com>
Contributor
|
@Fix-Point did you use AI to help with this PR? If so, please add: Assisted-by: AI Vendor and Model |
Contributor
|
@Fix-Point another question, does this PR fixes this Issue: #19370 ? |
xiaoxiang781216
approved these changes
Sep 4, 2026
acassis
requested changes
Sep 4, 2026
|
|
||
| *mask = next; | ||
| } | ||
| *mask = CLOCK_MAX >> (sizeof(clock_t) * 8u - flsx(maxticks)); |
Contributor
There was a problem hiding this comment.
I think there is an issue here if maxticks == 0, in the original code if maxticks is equal 0, that "if (next > maxticks)" was preventing the *mask = next to be executed, so *mask = 0 still valid. with you modification we will have an issue because clock_t is 64-bit (8 bytes), so 8 * 8u = 64 and CLOCK_MAX << 64 is undefined behavior on C:
"If the value of the right operand is negative or is greater than or equal to the width of the promoted left operand, the behavior is undefined."
Please change to:
*mask = maxticks == 0
? 0
: CLOCK_MAX >> (sizeof(clock_t) * 8u - flsx(maxticks));
|
|
||
| *mask = next; | ||
| } | ||
| *mask = CLOCK_MAX >> (sizeof(clock_t) * 8u - flsx(maxticks)); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fix three issues in the tickless scheduler / hrtimer path:
sched/hrtimer: Fix reprogram with wrong expiration when reinserting hrtimer. In
hrtimer_start_absolute(), when a pending hrtimer (previously the head of the queue) is removed and reinserted with a later expiration, thereprogramflag remains set although the timer is no longer the earliest one in the queue. The old code passedhrtimer->expiredtohrtimer_reprogram(); usehrtimer_get_first()->expiredinstead so the hardware timer is always reprogrammed with the actual earliest expiration.sched/sched: Fix roundrobin if SCHED_TICKLESS enabled. In tickless mode the scheduler timer is stopped whenever the running task requires no time slicing (
CLOCK_MAX). When a SCHED_RR task was later switched in, nothing re-armed the timer, so the task could run indefinitely without round-robin rotation. Reassess the scheduler timer innxsched_switch_context()before the context switch when the incoming task uses round-robin scheduling. Every architecture invokesnxsched_switch_context()exactly once per context switch (task switch, syscall, IRQ exit and task exit paths), so this covers all switch paths.sched/tickless: Fix SCHED_RR timeslice accounting on preemption. When an RR task was preempted, its timeslice counter was not decremented for the time already consumed, effectively granting the task bonus CPU time when resumed. Perform RR accounting on context switches:
nxsched_suspend_roundrobin()charges the elapsed execution time against the timeslice of the RR task being switched out, andnxsched_resume_roundrobin()re-arms the scheduler timer for the remaining timeslice of the RR task being switched in, so the timer is always armed while an RR task is running. This also removes the previous workaround innxsched_process_timer()that ran the scheduler logic on every timer tick.Impact
CONFIG_SCHED_TICKLESSconfigurations withCONFIG_RR_INTERVAL > 0(plus hrtimer users for the first fix). Periodic-tick builds are unchanged: the new scheduler code is compiled out, and hrtimer behavior only changes in the corner case described above.Testing
riscv64-unknown-elf-gcctoolchain, QEMU (qemu-system-riscv32).rv-virt:smpconfiguration withCONFIG_SCHED_TICKLESS=y.make -jcompletes without errors or new warnings.qemu-system-riscv32 -semihosting -M virt,aclint=on -cpu rv32 -smp 8 -bios none -kernel nuttx -nographic— NSH starts and is fully responsive.ostestapplication under QEMU: all subtests pass, in particular the round-robin test, verifying that RR rotation works under tickless mode and that preemption no longer grants bonus timeslice to RR tasks.