sched: Fix the timer reprogramming and SCHED_RR issues. - #20057
Merged
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>
Fix-Point
requested review from
GUIDINGLI,
gustavonihei,
pussuw and
xiaoxiang781216
as code owners
September 4, 2026 03:40
Fix-Point
changed the title
github-actions
Bot
added
Area: Drivers
|
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
previously approved these changes
Sep 4, 2026
acassis
requested changes
Sep 4, 2026
The mask computation introduced by "fix infinite loop in up_timer_getmask when maxticks == CLOCK_MAX" has two problems: 1. If maxticks == 0, flsx(0) expands to __builtin_clz(0), which is undefined behavior, and the shift count becomes 8 * sizeof(clock_t) = 64 for a 64-bit clock_t, which is undefined behavior as well. The loop-based code that was replaced kept *mask = 0 in this case. 2. CLOCK_MAX is INT64_MAX, i.e. 63 one bits, not a full-width bit pattern. The resulting mask is always one bit narrower than the one produced by the original loop; e.g. a 32-bit timer got 0x7fffffff instead of 0xffffffff, so counter deltas >= 2^31 were truncated in the clock timekeeping code. Fix this by keeping *mask = 0 when maxticks == 0 and by deriving the mask from the full-width unsigned constant (uint64_t)-1, which restores the all-ones semantics of the original loop and still covers the maxticks == CLOCK_MAX case. Also initialize maxticks in arch_timer.c: if the lower half does not implement the maxtimeout ops, the value is left untouched and would otherwise be read uninitialized. Assisted-by: Zhipu GLM-5.3 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. Assisted-by: Zhipu GLM-5.3 Signed-off-by: ouyangxiangzhen <ouyangxiangzhen@xiaomi.com>
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. Assisted-by: Zhipu GLM-5.3 Signed-off-by: ouyangxiangzhen <ouyangxiangzhen@xiaomi.com>
Fix-Point
force-pushed
the
fixschedtimer
branch
from
September 7, 2026 02:23
a7ce269 to
599a2ec
Compare
Contributor
Author
Done. |
Contributor
Author
The issue mentioned in #19370 is caused by unfair pthread mutex, not the scheduler. If we remove the qemu-system-riscv32 -semihosting -M virt,aclint=on -cpu rv32 -smp 8 -bios none -nographic -kernel nuttx
ABC
NuttShell (NSH) NuttX-12.13.0
nsh>
nsh>
nsh> hello
Low task 1 count: 1468315765
Low task 2 count: 1339093170
Total operations: 2807408935
Elapsed time: 5001.25 ms
Average time per operation: 0.001 us
nsh> hello
Low task 1 count: 1468315765
Low task 2 count: 1339093170
Total operations: 2807408935
Elapsed time: 5001.35 ms
Average time per operation: 0.001 us
nsh> |
xiaoxiang781216
approved these changes
Sep 7, 2026
jerpelea
approved these changes
Sep 7, 2026
acassis
approved these changes
Sep 7, 2026
Contributor
What do you mean by "unfair pthread mutex" ? Should it be "unpaired" ? |
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
4 participants
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.