xtimer: set now pointer correctly in _update_short_timers() loop [backport 2020.04]#13854
Merged
kaspar030 merged 3 commits intoRIOT-OS:2020.04-branchfrom Apr 14, 2020
Conversation
This fixes `xtimer` to use `xtimer_now64()` instead of `xtimer_now()` for updating the `*now` variable during the iteration in `_update_short_timers()` function. The same function is used to initialize `*now` in `_timer_callback()` below. While using `xtimer_now()` in this iteration step does not hinder the proper execution of all timers in the short term timers (for those the `xtimer` module only looks at the `start_time` member, not the `long_start_time` member) at least for the current long term time window (I did not test higher cases), it sets the `long_start_time` member to 0 for all timers following in the list of timers after this iteration step. However, external modules that rely on this to be correct, e.g. evtimer [1], fail their calculations when trying to compare to the current value to `xtimer_now64()`. [1] https://github.com/RIOT-OS/RIOT/blob/11f3d68/sys/evtimer/evtimer.c#L118-L121 Co-Authored-By: Cenk Gündoğan <mail+dev@gundogan.net> (cherry picked from commit 212fe15)
Co-Authored-By: Julian Holzwarth <julian.holzwarth@fu-berlin.de> (cherry picked from commit 4aa4a17)
(cherry picked from commit 085c62e)
miri64
requested review from
MichelRottleuthner,
aabadie,
fjmolinas,
kaspar030,
leandrolanzieri and
smlng
as code owners
miri64
added
Area: timers
81 tasks
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
3 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.
Backport of #13850
Contribution description
This fixes
xtimerto usextimer_now64()instead ofxtimer_now()for updating the*nowvariable during the iteration in_update_short_timers()function. The same function is used to initialize*nowin_timer_callback()below.While using
xtimer_now()in this iteration step does not hinder the proper execution of all timers in the short term timers (for those thextimermodule only looks at thestart_timemember, not thelong_start_timemember) at least for the current long term time window (I did not test higher cases), it sets thelong_start_timemember to 0 for all timers following in the list of timers after this iteration step. However, external modules that rely on this to be correct, e.g. evtimer, fail their calculations when trying to compare to the current value toxtimer_now64().If the changed header becomes the new head this behavior won't show up, as it is overwritten once the update of all headers is done. However, all timers later in the list will now have the wrong values.
Testing procedure
A regression test testing this behavior is provided. Confirm the bug, by reverting the fix and running it, run it with the patch to confirm that it fixes the bug:
Issues/PRs references
None, but should fix a known, but not reported issue, where RPL starts "spamming" DIS every minute instead of every 5 minutes after >71min.