From c4234d5087ce2b9bd189af5bfdda1e9d29b76d17 Mon Sep 17 00:00:00 2001 From: Giovanni Di Sirio Date: Fri, 19 Jun 2020 06:57:14 +0000 Subject: [PATCH] Fixed bug #1104. git-svn-id: svn://svn.code.sf.net/p/chibios/svn/branches/stable_20.3.x@13717 27425a3e-05d8-49a3-a47f-9c15f0e5edd8 --- os/rt/include/chtime.h | 6 +- os/rt/include/chvt.h | 111 +----------------- os/rt/src/chvt.c | 250 +++++++++++++++++++++++++++++++++++------ readme.txt | 1 + 4 files changed, 219 insertions(+), 149 deletions(-) diff --git a/os/rt/include/chtime.h b/os/rt/include/chtime.h index 79fab6a85..c6c1de8cc 100644 --- a/os/rt/include/chtime.h +++ b/os/rt/include/chtime.h @@ -266,9 +266,9 @@ typedef uint32_t time_conv_t; * @api */ #define TIME_I2US(interval) \ - (time_msecs_t)((((time_conv_t)(interval) * (time_conv_t)1000000) + \ - (time_conv_t)CH_CFG_ST_FREQUENCY - (time_conv_t)1) / \ - (time_conv_t)CH_CFG_ST_FREQUENCY) + (time_msecs_t)((((time_conv_t)(interval) * (time_conv_t)1000000) + \ + (time_conv_t)CH_CFG_ST_FREQUENCY - (time_conv_t)1) / \ + (time_conv_t)CH_CFG_ST_FREQUENCY) /** @} */ /*===========================================================================*/ diff --git a/os/rt/include/chvt.h b/os/rt/include/chvt.h index 0e763bf5f..ed1c512f3 100644 --- a/os/rt/include/chvt.h +++ b/os/rt/include/chvt.h @@ -75,6 +75,7 @@ extern "C" { void chVTDoSetI(virtual_timer_t *vtp, sysinterval_t delay, vtfunc_t vtfunc, void *par); void chVTDoResetI(virtual_timer_t *vtp); + void chVTDoTickI(void); #ifdef __cplusplus } #endif @@ -356,116 +357,6 @@ static inline void chVTSet(virtual_timer_t *vtp, sysinterval_t delay, chSysUnlock(); } -/** - * @brief Virtual timers ticker. - * @note The system lock is released before entering the callback and - * re-acquired immediately after. It is callback's responsibility - * to acquire the lock if needed. This is done in order to reduce - * interrupts jitter when many timers are in use. - * - * @iclass - */ -static inline void chVTDoTickI(void) { - - chDbgCheckClassI(); - -#if CH_CFG_ST_TIMEDELTA == 0 - ch.vtlist.systime++; - if (&ch.vtlist != (virtual_timers_list_t *)ch.vtlist.next) { - /* The list is not empty, processing elements on top.*/ - --ch.vtlist.next->delta; - while (ch.vtlist.next->delta == (sysinterval_t)0) { - virtual_timer_t *vtp; - vtfunc_t fn; - - vtp = ch.vtlist.next; - fn = vtp->func; - vtp->func = NULL; - vtp->next->prev = (virtual_timer_t *)&ch.vtlist; - ch.vtlist.next = vtp->next; - chSysUnlockFromISR(); - fn(vtp->par); - chSysLockFromISR(); - } - } -#else /* CH_CFG_ST_TIMEDELTA > 0 */ - virtual_timer_t *vtp; - systime_t now; - sysinterval_t delta, nowdelta; - - /* Looping through timers.*/ - vtp = ch.vtlist.next; - while (true) { - - /* Getting the system time as reference.*/ - now = chVTGetSystemTimeX(); - nowdelta = chTimeDiffX(ch.vtlist.lasttime, now); - - /* The list scan is limited by the timers header having - "ch.vtlist.vt_delta == (sysinterval_t)-1" which is - greater than all deltas.*/ - if (nowdelta < vtp->delta) { - break; - } - - /* Consuming all timers between "vtp->lasttime" and now.*/ - do { - vtfunc_t fn; - - /* The "last time" becomes this timer's expiration time.*/ - ch.vtlist.lasttime += vtp->delta; - nowdelta -= vtp->delta; - - vtp->next->prev = (virtual_timer_t *)&ch.vtlist; - ch.vtlist.next = vtp->next; - fn = vtp->func; - vtp->func = NULL; - - /* If the list becomes empty then the timer is stopped.*/ - if (ch.vtlist.next == (virtual_timer_t *)&ch.vtlist) { - port_timer_stop_alarm(); - } - - /* The callback is invoked outside the kernel critical zone.*/ - chSysUnlockFromISR(); - fn(vtp->par); - chSysLockFromISR(); - - /* Next element in the list.*/ - vtp = ch.vtlist.next; - } - while (vtp->delta <= nowdelta); - } - - /* If the list is empty, nothing else to do.*/ - if (ch.vtlist.next == (virtual_timer_t *)&ch.vtlist) { - return; - } - - /* The "unprocessed nowdelta" time slice is added to "last time" - and subtracted to next timer's delta.*/ - ch.vtlist.lasttime += nowdelta; - ch.vtlist.next->delta -= nowdelta; - - /* Recalculating the next alarm time.*/ - delta = vtp->delta - chTimeDiffX(ch.vtlist.lasttime, now); - if (delta < (sysinterval_t)CH_CFG_ST_TIMEDELTA) { - delta = (sysinterval_t)CH_CFG_ST_TIMEDELTA; - } -#if CH_CFG_INTERVALS_SIZE > CH_CFG_ST_RESOLUTION - /* The delta could be too large for the physical timer to handle.*/ - else if (delta > (sysinterval_t)TIME_MAX_SYSTIME) { - delta = (sysinterval_t)TIME_MAX_SYSTIME; - } -#endif - port_timer_set_alarm(chTimeAddX(now, delta)); - - chDbgAssert(chTimeDiffX(ch.vtlist.lasttime, chVTGetSystemTimeX()) <= - chTimeDiffX(ch.vtlist.lasttime, chTimeAddX(now, delta)), - "exceeding delta"); -#endif /* CH_CFG_ST_TIMEDELTA > 0 */ -} - #endif /* CHVT_H */ /** @} */ diff --git a/os/rt/src/chvt.c b/os/rt/src/chvt.c index 915961a72..41d7521a4 100644 --- a/os/rt/src/chvt.c +++ b/os/rt/src/chvt.c @@ -32,6 +32,45 @@ /* Module local definitions. */ /*===========================================================================*/ +/** + * @brief List empty check. + * + * @param[in] vtlp pointer to the list header + * + * @notapi + */ +#define is_vtlist_empty(vtlp) ((vtlp) == (virtual_timers_list_t *)(vtlp)->next) + +/** + * @brief Last timer in the list check. + * + * @param[in] vtlp pointer to the list header + * @param[in] vtp pointer to the timer header + * + * @notapi + */ +#define is_last_timer(vtlp, vtp) ((vtp)->next == (virtual_timer_t *)(vtlp)) + +/** + * @brief Fist timer in the list check. + * + * @param[in] vtlp pointer to the list header + * @param[in] vtp pointer to the timer header + * + * @notapi + */ +#define is_first_timer(vtlp, vtp) ((vtlp)->next == (vtp)) + +/** + * @brief Timer check. + * + * @param[in] vtlp pointer to the list header + * @param[in] vtp pointer to the timer header + * + * @notapi + */ +#define is_timer(vtlp, vtp) ((vtp) != (virtual_timer_t *)(vtlp)) + /*===========================================================================*/ /* Module exported variables. */ /*===========================================================================*/ @@ -48,6 +87,36 @@ /* Module local functions. */ /*===========================================================================*/ +#if (CH_CFG_ST_TIMEDELTA > 0) || defined(__DOXYGEN__) +/** + * @brief Delta list compression. + * + * @param[in] vtpl pointer to the delta list to be compressed + * @param[in] deltanow interval to be compacted starting from "lasttime" + * + * @notapi + */ +static void vt_list_compress(virtual_timers_list_t *vtlp, + sysinterval_t deltanow) { + virtual_timer_t *vtp = vtlp->next; + + /* The loop is bounded because the delta list header has the delta field + set to (sysinterval_t)-1 which is larger than all deltas.*/ + while (vtp->delta < deltanow) { + deltanow -= vtp->delta; + vtp->delta = (sysinterval_t)0; + vtp = vtp->next; + } + + vtlp->lasttime = vtlp->lasttime + deltanow; + + /* Adjusting next timer in the list, if any.*/ + if (is_timer(vtlp, vtp)) { + vtp->delta -= deltanow; + } +} +#endif + /*===========================================================================*/ /* Module exported functions. */ /*===========================================================================*/ @@ -94,6 +163,7 @@ void _vt_init(void) { */ void chVTDoSetI(virtual_timer_t *vtp, sysinterval_t delay, vtfunc_t vtfunc, void *par) { + virtual_timers_list_t *vtlp = &ch.vtlist; virtual_timer_t *p; sysinterval_t delta; @@ -106,6 +176,7 @@ void chVTDoSetI(virtual_timer_t *vtp, sysinterval_t delay, #if CH_CFG_ST_TIMEDELTA > 0 { systime_t now = chVTGetSystemTimeX(); + sysinterval_t deltanow; /* If the requested delay is lower than the minimum safe delta then it is raised to the minimum safe value.*/ @@ -114,15 +185,15 @@ void chVTDoSetI(virtual_timer_t *vtp, sysinterval_t delay, } /* Special case where the timers list is empty.*/ - if (&ch.vtlist == (virtual_timers_list_t *)ch.vtlist.next) { + if (is_vtlist_empty(vtlp)) { /* The delta list is empty, the current time becomes the new delta list base time, the timer is inserted.*/ - ch.vtlist.lasttime = now; - ch.vtlist.next = vtp; - ch.vtlist.prev = vtp; - vtp->next = (virtual_timer_t *)&ch.vtlist; - vtp->prev = (virtual_timer_t *)&ch.vtlist; + vtlp->lasttime = now; + vtlp->next = vtp; + vtlp->prev = vtp; + vtp->next = (virtual_timer_t *)vtlp; + vtp->prev = (virtual_timer_t *)vtlp; vtp->delta = delay; #if CH_CFG_INTERVALS_SIZE > CH_CFG_ST_RESOLUTION @@ -133,26 +204,23 @@ void chVTDoSetI(virtual_timer_t *vtp, sysinterval_t delay, #endif /* Being the first element in the list the alarm timer is started.*/ - port_timer_start_alarm(chTimeAddX(ch.vtlist.lasttime, delay)); + port_timer_start_alarm(chTimeAddX(vtlp->lasttime, delay)); return; } - /* Pointer to the first element in the delta list, which is non-empty.*/ - p = ch.vtlist.next; - /* Delay as delta from 'lasttime'. Note, it can overflow and the value - becomes lower than 'now'.*/ - delta = chTimeDiffX(ch.vtlist.lasttime, now) + delay; + becomes lower than 'deltanow'.*/ + deltanow = chTimeDiffX(vtlp->lasttime, now); + delta = deltanow + delay; - if (delta < chTimeDiffX(ch.vtlist.lasttime, now)) { - /* Scenario where a very large delay excedeed the numeric range, it - requires a special handling. We need to skip the first element and - adjust the delta to wrap back in the previous numeric range.*/ - delta -= p->delta; - p = p->next; + /* Scenario where a very large delay exceeded the numeric range, it + requires a special handling, the compression procedure.*/ + if (delta < deltanow) { + vt_list_compress(vtlp, deltanow); + delta -= deltanow; } - else if (delta < p->delta) { + else if (delta < vtlp->next->delta) { sysinterval_t deadline_delta; /* A small delay that will become the first element in the delta list @@ -164,19 +232,17 @@ void chVTDoSetI(virtual_timer_t *vtp, sysinterval_t delay, deadline_delta = (sysinterval_t)TIME_MAX_SYSTIME; } #endif - port_timer_set_alarm(chTimeAddX(ch.vtlist.lasttime, deadline_delta)); + port_timer_set_alarm(chTimeAddX(vtlp->lasttime, deadline_delta)); } } #else /* CH_CFG_ST_TIMEDELTA == 0 */ /* Delta is initially equal to the specified delay.*/ delta = delay; - - /* Pointer to the first element in the delta list.*/ - p = ch.vtlist.next; #endif /* CH_CFG_ST_TIMEDELTA == 0 */ /* The delta list is scanned in order to find the correct position for this timer. */ + p = vtlp->next; while (p->delta < delta) { /* Debug assert if the timer is already in the list.*/ chDbgAssert(p != vtp, "timer already armed"); @@ -197,7 +263,7 @@ void chVTDoSetI(virtual_timer_t *vtp, sysinterval_t delay, /* Special case when the timer is in last position in the list, the value in the header must be restored.*/ - ch.vtlist.delta = (sysinterval_t)-1; + vtlp->delta = (sysinterval_t)-1; } /** @@ -209,6 +275,7 @@ void chVTDoSetI(virtual_timer_t *vtp, sysinterval_t delay, * @iclass */ void chVTDoResetI(virtual_timer_t *vtp) { + virtual_timers_list_t *vtlp = &ch.vtlist; chDbgCheckClassI(); chDbgCheck(vtp != NULL); @@ -226,57 +293,57 @@ void chVTDoResetI(virtual_timer_t *vtp) { /* The above code changes the value in the header when the removed element is the last of the list, restoring it.*/ - ch.vtlist.delta = (sysinterval_t)-1; + vtlp->delta = (sysinterval_t)-1; #else /* CH_CFG_ST_TIMEDELTA > 0 */ sysinterval_t nowdelta, delta; /* If the timer is not the first of the list then it is simply unlinked else the operation is more complex.*/ - if (ch.vtlist.next != vtp) { + if (!is_first_timer(vtlp, vtp)) { /* Removing the element from the delta list.*/ vtp->prev->next = vtp->next; vtp->next->prev = vtp->prev; vtp->func = NULL; /* Adding delta to the next element, if it is not the last one.*/ - if (&ch.vtlist != (virtual_timers_list_t *)vtp->next) + if (is_timer(vtlp, vtp->next)) vtp->next->delta += vtp->delta; return; } /* Removing the first timer from the list.*/ - ch.vtlist.next = vtp->next; - ch.vtlist.next->prev = (virtual_timer_t *)&ch.vtlist; + vtlp->next = vtp->next; + vtlp->next->prev = (virtual_timer_t *)vtlp; vtp->func = NULL; /* If the list become empty then the alarm timer is stopped and done.*/ - if (&ch.vtlist == (virtual_timers_list_t *)ch.vtlist.next) { + if (is_vtlist_empty(vtlp)) { port_timer_stop_alarm(); return; } /* The delta of the removed timer is added to the new first timer.*/ - ch.vtlist.next->delta += vtp->delta; + vtlp->next->delta += vtp->delta; /* If the new first timer has a delta of zero then the alarm is not modified, the already programmed alarm will serve it.*/ -/* if (ch.vtlist.next->delta == 0) { +/* if (vtlp->next->delta == 0) { return; }*/ /* Distance in ticks between the last alarm event and current time.*/ - nowdelta = chTimeDiffX(ch.vtlist.lasttime, chVTGetSystemTimeX()); + nowdelta = chTimeDiffX(vtlp->lasttime, chVTGetSystemTimeX()); /* If the current time surpassed the time of the next element in list then the event interrupt is already pending, just return.*/ - if (nowdelta >= ch.vtlist.next->delta) { + if (nowdelta >= vtlp->next->delta) { return; } /* Distance from the next scheduled event and now.*/ - delta = ch.vtlist.next->delta - nowdelta; + delta = vtlp->next->delta - nowdelta; /* Making sure to not schedule an event closer than CH_CFG_ST_TIMEDELTA ticks from now.*/ @@ -292,7 +359,118 @@ void chVTDoResetI(virtual_timer_t *vtp) { } #endif } - port_timer_set_alarm(chTimeAddX(ch.vtlist.lasttime, delta)); + port_timer_set_alarm(chTimeAddX(vtlp->lasttime, delta)); +#endif /* CH_CFG_ST_TIMEDELTA > 0 */ +} + +/** + * @brief Virtual timers ticker. + * @note The system lock is released before entering the callback and + * re-acquired immediately after. It is callback's responsibility + * to acquire the lock if needed. This is done in order to reduce + * interrupts jitter when many timers are in use. + * + * @iclass + */ +void chVTDoTickI(void) { + virtual_timers_list_t *vtlp = &ch.vtlist; + + chDbgCheckClassI(); + +#if CH_CFG_ST_TIMEDELTA == 0 + vtlp->systime++; + if (!is_vtlist_empty(vtlp)) { + /* The list is not empty, processing elements on top.*/ + --vtlp->next->delta; + while (vtlp->next->delta == (sysinterval_t)0) { + virtual_timer_t *vtp; + vtfunc_t fn; + + vtp = vtlp->next; + fn = vtp->func; + vtp->func = NULL; + vtp->next->prev = (virtual_timer_t *)vtlp; + vtlp->next = vtp->next; + chSysUnlockFromISR(); + fn(vtp->par); + chSysLockFromISR(); + } + } +#else /* CH_CFG_ST_TIMEDELTA > 0 */ + virtual_timer_t *vtp; + systime_t now; + sysinterval_t delta, nowdelta; + + /* Looping through timers.*/ + vtp = vtlp->next; + while (true) { + + /* Getting the system time as reference.*/ + now = chVTGetSystemTimeX(); + nowdelta = chTimeDiffX(vtlp->lasttime, now); + + /* The list scan is limited by the timers header having + "vtlp->vt_delta == (sysinterval_t)-1" which is + greater than all deltas.*/ + if (nowdelta < vtp->delta) { + break; + } + + /* Consuming all timers between "vtp->lasttime" and now.*/ + do { + vtfunc_t fn; + + /* The "last time" becomes this timer's expiration time.*/ + vtlp->lasttime += vtp->delta; + nowdelta -= vtp->delta; + + vtp->next->prev = (virtual_timer_t *)vtlp; + vtlp->next = vtp->next; + fn = vtp->func; + vtp->func = NULL; + + /* If the list becomes empty then the timer is stopped.*/ + if (is_vtlist_empty(vtlp)) { + port_timer_stop_alarm(); + } + + /* The callback is invoked outside the kernel critical zone.*/ + chSysUnlockFromISR(); + fn(vtp->par); + chSysLockFromISR(); + + /* Next element in the list.*/ + vtp = vtlp->next; + } + while (vtp->delta <= nowdelta); + } + + /* If the list is empty, nothing else to do.*/ + if (is_vtlist_empty(vtlp)) { + return; + } + + /* The "unprocessed nowdelta" time slice is added to "last time" + and subtracted to next timer's delta.*/ + vtlp->lasttime += nowdelta; + vtlp->next->delta -= nowdelta; + + /* Recalculating the next alarm time.*/ + delta = vtp->delta - chTimeDiffX(vtlp->lasttime, now); + if (delta < (sysinterval_t)CH_CFG_ST_TIMEDELTA) { + delta = (sysinterval_t)CH_CFG_ST_TIMEDELTA; + } +#if CH_CFG_INTERVALS_SIZE > CH_CFG_ST_RESOLUTION + /* The delta could be too large for the physical timer to handle.*/ + else if (delta > (sysinterval_t)TIME_MAX_SYSTIME) { + delta = (sysinterval_t)TIME_MAX_SYSTIME; + } +#endif + port_timer_set_alarm(chTimeAddX(now, delta)); + + chDbgAssert(chTimeDiffX(vtlp->lasttime, chVTGetSystemTimeX()) <= + chTimeDiffX(vtlp->lasttime, chTimeAddX(now, delta)), + "exceeding delta"); #endif /* CH_CFG_ST_TIMEDELTA > 0 */ } diff --git a/readme.txt b/readme.txt index 1dbad823b..4b58e96f2 100644 --- a/readme.txt +++ b/readme.txt @@ -74,6 +74,7 @@ ***************************************************************************** *** 20.3.2 *** +- FIX: Fixed Virtual Timers corner case (bug #1104). - FIX: Fixed GCC6 problem breaks Cortex-M0 port (bug #985). - FIX: Fixed a wrong management of the SPI TX buffer in the ADUCM port (bug #1103).