Improve and debug Xtimer #10393
Closed
Improve and debug Xtimer #10393
Conversation
This file contains 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
rearange test for overflowed base timer. Previous implementation fired some xtimers one low level timer period too early. Most probably caused by a double check for overflowed llcounter and calling _next_period(): The issue could be observed on a platform with 16 bit and 125000 Hz xtimer. So when printing the test sequence every second, the set 1 second timeout required to have 2 low level counter overflows. With this changes there is still a drift running xtimer_drift. But it is not a whole low level counter period.
When sleeping in the main function with xtimer_sleep not every percentage will be printed as the sleep duration and the processing time added do not result in full percentage values. Using xtimer_periodic_wakeup ensures that every integer percentage is printed.
Previously one pin was used for both slacker threads. For better debugging seperating the threads to use one pin each is better.
When the slacker threads and the worker thread have the same priority the drift and jitter depend on the workload. Timer set before the worker thread will be fired before the worker and so the result is not the timings the system might reach when proper priority is set. Increasing the priority of the worker thread ensures that the measureed timings are the achievable timings.
This commit ensures that the _time_left function is able to detect an overflown llcounter and a target which is in the next period and to return the correct difference.
When the reference is set to `_xtimer_lltimer_now()` a interrupt at the verge of the llcounter overflow could lead to early shooting of the xtimer. Using the target as reference which will be compared to now in `_time_left` calculates the right time which is left before shooting.
|
@cladmi maybe we can use your mock to show some of this issues. |
miri64
added
Type: enhancement
|
As I am really not focused on timer, it is hard for me to concentrate on PRs changing many things as it takes a lot of time to review. It would simplify my task if you could split small parts, still keep the main PR as a reference, but focus on one small part to fix first. The lower level it is, the better as it solves the ground problems first so we can build on top. |
|
@haukepetersen as posted above, there is still a lot to get into master until we can target the issues the xtimer has and which get observeable on 16bit platforms. Maybe someone will have time to dig into. |
ben-postman
pushed a commit
to ben-postman/RIOT
that referenced
this pull request
May 30, 2019
Once RIOT-OS issue RIOT-OS#10393 (or potentially another, there are many open xtimer issues), that solution ought to be used instead. The problem is that if the hardware timer overflows immediately after (or at the same time as) a timer expires, that the reference time gets set in the wrong period. There appear to be many issues with xtimer and overflow, however, so a more holistic solution ought to be applied when one becomes available.
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions. |
stale
bot
added
the
State: stale
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.
This PR is a stack of commits which depends upon each other and are a rework of some things done in #8990.
Tries to handle the same problem as xtimer: Skip ahead when target time has already passed #7629 and xtimer: periodic race #9595 .
_timer_callback changed to only check once for an overflow. Avoiding early firing of xtimer when _next_period is called 2 times. Encountered when testing with xtimer_drift.
xtimer_hang test schould use xtimer_periodic_wakeup instead of sleep. This will ensure that all percentage values are printed.
Using multiple pins to distinguish the threads.
Ensure that worker_thread is higher prior than slacker_thread.
_time_left was not able to calculate the time left for a target which is at the beginning of the next period, but has to be fired in this period because of XTIMER_OVERHEAD.
Xtimer where fired to early when running xtimer_drift. Which was caused by a overflowing llcounter. As the reference was set to now. The function _time_left calculated the wrong time to wait.
See the commit messages for more details.
Following Test run with good results on the jiminy rfr2