fix(rtc): Bound RTIC monotonic spin to sync horizon, verify compare arming - #1012
Merged
Merged
Conversation
The RTC interrupt handler currently spins on count < compare, to ensure that stale count read does not reach the on_monotonic_interrupt fn. This is fine during normal operation, but under tight timing, the compare target can be moved after the flag is set, causing the spin to wait until the incorrect time. If this time is far in the future, it wedges the handler and everything at its priority and below. This commit adds a one-time check that limit > compare, so that if the compare value has been overwritten in a way that would cause a long spin, the spin will instead be skipped - elimitating the stall, as well as replacing MIN_COMPARE_TICKS with a new unified SYNC_SLACK_TICKS based on the worst-case read latency detailed in the datasheet
Contributor
Author
|
The 16 Tier 2 failures are unrelated: |
Merged
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
The RTC interrupt handler currently spins on count < compare, to ensure that a stale count read does not reach the on_monotonic_interrupt fn.
This is fine during normal operation, but when the handler arms a compare only a few ticks ahead of the counter the match can fire after the handler has already moved CC0 on to the next queued deadline. The re-pended handler then observes a flag raised by an already-serviced compare alongside a CC0 holding an arbitrarily distant target, and spins until COUNT reaches it. Since RTIC places the RTC vector at the same priority as the highest-priority async task, everything at or below that priority is frozen for the duration.
I ran into this causing intermittent issues with my serial comms, specifically under high load, due to the timer getting stuck spinning for longer than my comms timeout.
This commit adds a check that the compare lies within the sync horizon of the current count before spinning, so that if the compare value has been overwritten in a way that would cause a long spin, it will instead be skipped - eliminating the stall. It also replaces MIN_COMPARE_TICKS with a new unified SYNC_SLACK_TICKS based on the worst-case read latency detailed in the datasheet.
set_compare now also re-reads COUNT after the synced write and re-pends the handler if the armed value may already have been passed - an equality match armed in the past otherwise never fires, wedging the queue until counter wrap.
Some RTT logs captured with my reproduction:
post_fix_no_stalls.txt
baseline_stalls.txt
baseline_lost_wake.txt
Checklist
#[allow]certain lints where reasonable, but ideally justify those with a short comment.