From 672368954e20bcd3ddc10484a1caa93f07d3a2fb Mon Sep 17 00:00:00 2001 From: Josarn Date: Mon, 28 May 2018 15:39:16 +0200 Subject: [PATCH] xtimer: timer & target overflow, hang resolved. 1. When the 32 bit target of the xtimer overflowed the timer was not placed in the right list. 2. When the hardware timer overflowed the comparison was wrong for setting next target. 3. Backoff condition --- sys/include/xtimer/implementation.h | 14 +++++++++ sys/xtimer/xtimer_core.c | 48 ++++++++++++++++++++++++----- 2 files changed, 54 insertions(+), 8 deletions(-) diff --git a/sys/include/xtimer/implementation.h b/sys/include/xtimer/implementation.h index ed16aa91ad71..10baf7c5d3ba 100644 --- a/sys/include/xtimer/implementation.h +++ b/sys/include/xtimer/implementation.h @@ -65,7 +65,21 @@ static inline uint32_t _xtimer_lltimer_mask(uint32_t val) * @brief xtimer internal stuff * @internal */ + uint64_t _xtimer_now64(void); + +/** + * @brief Sets the timer to the appropriate timer_list or list_head. + * + * @note The target to set the timer to has to be at least bigger then the + * ticks needed to jump into the function and calculate '_xtimer_now()'. + * So that 'now' did not pass the target. + * This is crucial when using low CPU frequencies and/or when the + * '_xtimer_now()' call needs multiple xtimer ticks to evaluate. + * + * @param[in] timer pointer to xtimer_t which is added to the list. + * @param[in] target Absolute target value in ticks. + */ int _xtimer_set_absolute(xtimer_t *timer, uint32_t target); void _xtimer_set(xtimer_t *timer, uint32_t offset); void _xtimer_set64(xtimer_t *timer, uint32_t offset, uint32_t long_offset); diff --git a/sys/xtimer/xtimer_core.c b/sys/xtimer/xtimer_core.c index 5423eee1d29f..841155839979 100644 --- a/sys/xtimer/xtimer_core.c +++ b/sys/xtimer/xtimer_core.c @@ -1,6 +1,7 @@ /** * Copyright (C) 2015 Kaspar Schleiser - * Copyright (C) 2016 Eistec AB + * 2016 Eistec AB + * 2018 Josua Arndt * * This file is subject to the terms and conditions of the GNU Lesser * General Public License v2.1. See the file LICENSE in the top level @@ -15,6 +16,7 @@ * @brief xtimer core functionality * @author Kaspar Schleiser * @author Joakim Nohlgård + * @author Josua Arndt * @} */ @@ -178,12 +180,27 @@ int _xtimer_set_absolute(xtimer_t *timer, uint32_t target) uint32_t now = _xtimer_now(); int res = 0; - DEBUG("timer_set_absolute(): now=%" PRIu32 " target=%" PRIu32 "\n", now, target); - timer->next = NULL; - if ((target >= now) && ((target - XTIMER_BACKOFF) < now)) { + + /* Ensure that offset is bigger than 'XTIMER_BACKOFF', + * 'target - now' will allways be the offset no matter if target < or > now. + * + * This expects that target was not set too close to now and overrun now, so + * from setting target up until the call of '_xtimer_now()' above now has not + * become equal or bigger than target. + * This is crucial when using low CPU frequencies so reaching the '_xtimer_now()' + * call needs multiple xtimer ticks. + * + * '_xtimer_set()' and `_xtimer_periodic_wakeup()` ensure this by already + * backing off for small values. */ + uint32_t offset = (target - now); + + DEBUG("timer_set_absolute(): now=%" PRIu32 " target=%" PRIu32 " offset=%" PRIu32 "\n", + now, target, offset); + + if (offset <= XTIMER_BACKOFF) { /* backoff */ - xtimer_spin_until(target + XTIMER_BACKOFF); + xtimer_spin_until(target); _shoot(timer); return 0; } @@ -195,6 +212,17 @@ int _xtimer_set_absolute(xtimer_t *timer, uint32_t target) timer->target = target; timer->long_target = _long_cnt; + + /* Ensure timer is fired in right timer period. + * Backoff condition above ensures that 'target - XTIMER_OVERHEAD` is later + * than 'now', also for values when now will overflow and the value of target + * is smaller then now. + * If `target < XTIMER_OVERHEAD` the new target will be at the end of this + * 32bit period, as `target - XTIMER_OVERHEAD` is a big number instead of a + * small at the beginning of the next period. */ + target = target - XTIMER_OVERHEAD; + + /* 32 bit target overflow, target is in next 32bit period */ if (target < now) { timer->long_target++; } @@ -214,7 +242,7 @@ int _xtimer_set_absolute(xtimer_t *timer, uint32_t target) if (timer_list_head == timer) { DEBUG("timer_set_absolute(): timer is new list head. updating lltimer.\n"); - _lltimer_set(target - XTIMER_OVERHEAD); + _lltimer_set(target); } } } @@ -493,10 +521,14 @@ static void _timer_callback(void) * time to overflow. In that case we advance to * next timer period and check again for expired * timers.*/ - if (reference > _xtimer_lltimer_now()) { + /* check if the end of this period is very soon */ + uint32_t now = _xtimer_lltimer_now() + XTIMER_ISR_BACKOFF; + if (now < reference) { DEBUG("_timer_callback: overflowed while executing callbacks. %i\n", timer_list_head != NULL); _next_period(); + /* wait till overflow */ + while( reference < _xtimer_lltimer_now()){} reference = 0; goto overflow; } @@ -506,7 +538,7 @@ static void _timer_callback(void) next_target = timer_list_head->target - XTIMER_OVERHEAD; /* make sure we're not setting a time in the past */ - if (next_target < (_xtimer_lltimer_now() + XTIMER_ISR_BACKOFF)) { + if (next_target < (_xtimer_now() + XTIMER_ISR_BACKOFF)) { goto overflow; } }