Commit Graph

23 Commits

Author SHA1 Message Date
Jeff Tenney
195a351ec7
Tickless idle fixes/improvement (#59)
* Fix tickless idle when stopping systick on zero...

...and don't stop SysTick at all in the eAbortSleep case.

Prior to this commit, if vPortSuppressTicksAndSleep() happens to stop
the SysTick on zero, then after tickless idle ends, xTickCount advances
one full tick more than the time that actually elapsed as measured by
the SysTick.  See "bug 1" in this forum post:
https://forums.freertos.org/t/ultasknotifytake-timeout-accuracy/9629/40

SysTick
-------
The SysTick is the hardware timer that provides the OS tick interrupt
in the official ports for Cortex M.  SysTick starts counting down from
the value stored in its reload register.  When SysTick reaches zero, it
requests an interrupt.  On the next SysTick clock cycle, it loads the
counter again from the reload register.  To get periodic interrupts
every N SysTick clock cycles, the reload register must be N - 1.

Bug Example
-----------
- Idle task calls vPortSuppressTicksAndSleep(xExpectedIdleTime = 2).
  [Doesn't have to be "2" -- could be any number.]
- vPortSuppressTicksAndSleep() stops SysTick, and the current-count
  register happens to stop on zero.
- SysTick ISR executes, setting xPendedTicks = 1
- vPortSuppressTicksAndSleep() masks interrupts and calls
  eTaskConfirmSleepModeStatus() which confirms the sleep operation. ***
- vPortSuppressTicksAndSleep() configures SysTick for 1 full tick
  (xExpectedIdleTime - 1) plus the current-count register (which is 0)
- One tick period elapses in sleep.
- SysTick wakes CPU, ISR executes and increments xPendedTicks to 2.
- vPortSuppressTicksAndSleep() calls vTaskStepTick(1), then returns.
- Idle task resumes scheduler, which increments xTickCount twice (for
  xPendedTicks = 2)

In the end, two ticks elapsed as measured by SysTick, but the code
increments xTickCount three times.  The root cause is that the code
assumes the SysTick current-count register always contains the number of
SysTick counts remaining in the current tick period.  However, when the
current-count register is zero, there are ulTimerCountsForOneTick
counts remaining, not zero.  This error is not the kind of time slippage
normally associated with tickless idle.

*** Note that a recent commit https://github.com/FreeRTOS/FreeRTOS-Kernel/commit/e1b98f0
results in eAbortSleep in this case, due to xPendedTicks != 0.  That
commit does mostly resolve this bug without specifically mentioning
it, and without this commit.  But that resolution allows the code in
port.c not to directly address the special case of stopping SysTick on
zero in any code or comments.  That commit also generates additional
instances of eAbortSleep, and a second purpose of this commit is to
optimize how vPortSuppressTicksAndSleep() behaves for eAbortSleep, as
noted below.

This commit also includes an optimization to avoid stopping the SysTick
when eTaskConfirmSleepModeStatus() returns eAbortSleep.  This
optimization belongs with this fix because the method of handling the
SysTick being stopped on zero changes with this optimization.

* Fix imminent tick rescheduled after tickless idle

Prior to this commit, if something other than systick wakes the CPU from
tickless idle, vPortSuppressTicksAndSleep() might cause xTickCount to
increment once too many times.  See "bug 2" in this forum post:
https://forums.freertos.org/t/ultasknotifytake-timeout-accuracy/9629/40

SysTick
-------
The SysTick is the hardware timer that provides the OS tick interrupt
in the official ports for Cortex M.  SysTick starts counting down from
the value stored in its reload register.  When SysTick reaches zero, it
requests an interrupt.  On the next SysTick clock cycle, it loads the
counter again from the reload register.  To get periodic interrupts
every N SysTick clock cycles, the reload register must be N - 1.

Bug Example
-----------
- CPU is sleeping in vPortSuppressTicksAndSleep()
- Something other than the SysTick wakes the CPU.
- vPortSuppressTicksAndSleep() calculates the number of SysTick counts
  until the next tick.  The bug occurs only if this number is small.
- vPortSuppressTicksAndSleep() puts this small number into the SysTick
  reload register, and starts SysTick.
- vPortSuppressTicksAndSleep() calls vTaskStepTick()
- While vTaskStepTick() executes, the SysTick expires.  The ISR pends
  because interrupts are masked, and SysTick starts a 2nd period still
  based on the small number of counts in its reload register.  This 2nd
  period is undesirable and is likely to cause the error noted below.
- vPortSuppressTicksAndSleep() puts the normal tick duration into the
  SysTick's reload register.
- vPortSuppressTicksAndSleep() unmasks interrupts before the SysTick
  starts a new period based on the new value in the reload register.
  [This is a race condition that can go either way, but for the bug
  to occur, the race must play out this way.]
- The pending SysTick ISR executes and increments xPendedTicks.
- The SysTick expires again, finishing the second very small period, and
  starts a new period this time based on the full tick duration.
- The SysTick ISR increments xPendedTicks (or xTickCount) even though
  only a tiny fraction of a tick period has elapsed since the previous
  tick.

The bug occurs when *two* consecutive small periods of the SysTick are
both counted as ticks.  The root cause is a race caused by the small
SysTick period.  If vPortSuppressTicksAndSleep() unmasks interrupts
*after* the small period expires but *before* the SysTick starts a
period based on the full tick period, then two small periods are
counted as ticks when only one should be counted.

The end result is xTickCount advancing nearly one full tick more than
time actually elapsed as measured by the SysTick.  This is not the kind
of time slippage normally associated with tickless idle.

After this commit the code starts the SysTick and then immediately
modifies the reload register to ensure the very short cycle (if any) is
conducted only once.  This strategy requires special consideration for
the build option that configures SysTick to use a divided clock.  To
avoid waiting around for the SysTick to load value from the reload
register, the new code temporarily configures the SysTick to use the
undivided clock.  The resulting timing error is typical for tickless
idle.  The error (commonly known as drift or slippage in kernel time)
caused by this strategy is equivalent to one or two counts in
ulStoppedTimerCompensation.

This commit also updates comments and #define symbols related to the
SysTick clock option.  The SysTick can optionally be clocked by a
divided version of the CPU clock (commonly divide-by-8).  The new code
in this commit adjusts these comments and symbols to make them clearer
and more useful in configurations that use the divided clock.  The fix
made in this commit requires the use of these symbols, as noted in the
code comments.

* Fix tickless idle with alternate systick clocking

Prior to this commit, in configurations using the alternate SysTick
clocking, vPortSuppressTicksAndSleep() might cause xTickCount to jump
ahead as much as the entire expected idle time or fall behind as much
as one full tick compared to time as measured by the SysTick.

SysTick
-------
The SysTick is the hardware timer that provides the OS tick interrupt
in the official ports for Cortex M. SysTick starts counting down from
the value stored in its reload register. When SysTick reaches zero, it
requests an interrupt. On the next SysTick clock cycle, it loads the
counter again from the reload register. The SysTick has a configuration
option to be clocked by an alternate clock besides the core clock.
This alternate clock is MCU dependent.

Scenarios Fixed
---------------
The new code in this commit handles the following scenarios that were
not handled correctly prior to this commit.

1. Before the sleep, vPortSuppressTicksAndSleep() stops the SysTick on
zero, long after SysTick reached zero.  Prior to this commit, this
scenario caused xTickCount to jump ahead one full tick for the same
reason documented here: 0c7b04bd3a

2. After the sleep, vPortSuppressTicksAndSleep() stops the SysTick
before it loads the counter from the reload register.  Prior to this
commit, this scenario caused xTickCount to jump ahead by the entire
expected idle time (xExpectedIdleTime) because the current-count
register is zero before it loads from the reload register.

3. Prior to return, vPortSuppressTicksAndSleep() attempts to start a
short SysTick period when the current SysTick clock cycle has a lot of
time remaining.  Prior to this commit, this scenario could cause
xTickCount to fall behind by as much as nearly one full tick because the
short SysTick cycle never started.

Note that #3 is partially fixed by 967acc9b20
even though that commit addresses a different issue.  So this commit
completes the partial fix.

* Improve comments and name of preprocessor symbol

Add a note in the code comments that SysTick requests an interrupt when
decrementing from 1 to 0, so that's why stopping SysTick on zero is a
special case.  Readers might unknowingly assume that SysTick requests
an interrupt when wrapping from 0 back to the load-register value.

Reconsider new "_SETTING" suffix since "_CONFIG" suffix seems more
descriptive.  The code relies on *both* of these preprocessor symbols:

portNVIC_SYSTICK_CLK_BIT
portNVIC_SYSTICK_CLK_BIT_CONFIG  **new**

A meaningful suffix is really helpful to distinguish the two symbols.

* Revert introduction of 2nd name for NVIC register

When I added portNVIC_ICSR_REG I didn't realize there was already a
portNVIC_INT_CTRL_REG, which identifies the same register.  Not good
to have both.  Note that portNVIC_INT_CTRL_REG is defined in portmacro.h
and is already used in this file (port.c).

* Replicate to other Cortex M ports

Also set a new fiddle factor based on tests with a CM4F.  I used gcc,
optimizing at -O1.  Users can fine-tune as needed.

Also add configSYSTICK_CLOCK_HZ to the CM0 ports to be just like the
other Cortex M ports.  This change allowed uniformity in the default
tickless implementations across all Cortex M ports.  And CM0 is likely
to benefit from configSYSTICK_CLOCK_HZ, especially considering new CM0
devices with very fast CPU clock speeds.

* Revert changes to IAR-CM0-portmacro.h

portNVIC_INT_CTRL_REG was already defined in port.c.  No need to define
it in portmacro.h.

* Handle edge cases with slow SysTick clock

Co-authored-by: Cobus van Eeden <35851496+cobusve@users.noreply.github.com>
Co-authored-by: abhidixi11 <44424462+abhidixi11@users.noreply.github.com>
Co-authored-by: Joseph Julicher <jjulicher@mac.com>
Co-authored-by: alfred gedeon <28123637+alfred2g@users.noreply.github.com>
2022-10-03 12:39:17 -07:00
swaldhoer
a030d0a02b
fix typo (#399) 2021-10-06 15:32:07 -07:00
Paul Bartell
eec42331b4 Normalize files with mixed line endings (introduced in commit 3a413d1) 2021-06-01 17:55:18 -07:00
Paul Bartell
3a413d1022 Add SPDX-License-Identifier: MIT to MIT licensed files. 2021-05-27 19:57:55 -07:00
Paul Bartell
adfc53368f Update copyright year from 2020 to 2021 2021-05-27 19:57:55 -07:00
Paul Bartell
08dc6f64ee Change kernel revision in each file header from V10.4.3 to <DEVELOPMENT BRANCH> 2021-05-27 19:57:55 -07:00
j4cbo
f37bcd5c14
Wrap macros in do { ... } while( 0 ) (#240) 2021-05-24 14:41:58 -07:00
Cobus van Eeden
ec62f69dab [AUTO][RELEASE]: Bump file header version to "10.4.3" 2020-12-14 10:13:39 -08:00
David Chalco
337bca615e [AUTO][RELEASE]: Bump file header version to "10.4.2" 2020-11-10 14:42:58 -08:00
David Chalco
3604527e3b
Update version number to 10.4.1 (#173) 2020-09-17 15:25:15 -07:00
David Chalco
5dfab0306b
Update version number to 10.4.0 (#153) 2020-09-10 19:49:34 -07:00
alfred gedeon
0afc048cf2
Style: Add uncrustify guards, fix asm (#136)
Co-authored-by: Alfred Gedeon <gedeonag@amazon.com>
2020-08-24 15:32:02 -07:00
alfred gedeon
a038146915
Style: Make freertos.org = FreeRTOS.org and add https (#134)
* Style: make freertos.org = FreeRTOS.org also add https

* Style: Fix freertos into FreeRTOS

* Style: Fix freertos into FreeRTOS

Co-authored-by: Alfred Gedeon <gedeonag@amazon.com>
2020-08-21 11:30:39 -07:00
alfred gedeon
0b0a2060c0
Style: Change FreeRTOS websites in comments (#131)
* Style: Change FreeRTOS websites in comments

* Style: Change freertos to FreeRTOS in comments

* Style: Remove broken link

Co-authored-by: Alfred Gedeon <gedeonag@amazon.com>
2020-08-20 14:59:28 -07:00
alfred gedeon
8c77117c32
Style: Remove tabs and tab == 4 spaces (#120)
* Style: Remove tabls and tab == 4 spaces

* Style: remove xx accidentally left

* Style: revert uncrustify for untested portable directories

* Style: revert more uncrustify files

* Style: Revert more uncrustified files

* Style: Revert some uncrutified files

* Style: change more files

* Style: remove t tab == 4 spaces

* Style: remove tabs = spaces

* Style: revert changed files

* Style: redo the stuyles

* Style: add uncrustify disable parsing for asm

Co-authored-by: Alfred Gedeon <gedeonag@amazon.com>
2020-08-17 14:50:56 -07:00
alfred gedeon
86653e2a1f
Style: Revert uncrustify for portable directories (#122)
* Style: revert uncrustify portable directories

* Style: Uncrustify Some Portable files

Co-authored-by: Alfred Gedeon <gedeonag@amazon.com>
2020-08-17 10:51:02 -07:00
Gaurav-Aggarwal-AWS
a717d9c62b
Update portNVIC_SYSPRI2_REG to portNVIC_SHPR3_REG (#86)
The reason for the change is that the register is called System Handler
Priority Register 3 (SHPR3).

Signed-off-by: Gaurav Aggarwal <aggarg@amazon.com>
2020-07-15 19:44:45 -07:00
Alfred Gedeon
587a83d647 Style: uncrustify kernel files 2020-07-08 10:24:06 -07:00
Alfred Gedeon
2c530ba5c3 Style: uncrustify 2020-07-08 10:24:06 -07:00
Alfred Gedeon
718178c68a Style: uncrusitfy 2020-07-08 10:24:06 -07:00
Alfred Gedeon
a5dbc2b1de Style: uncrustify kernel files 2020-07-08 10:24:06 -07:00
Yuhui.Zheng
88e32327e9
version bump to v10.3.1 (#16)
* Verion bump from 10.3.0 to 10.3.1.
* version bump in task.h
* change history for 10.3.1.
2020-02-18 22:03:54 -08:00
Yuhui Zheng
210b1ffcc8 Re-sync with upstream and stripping away none kernel related. 2020-02-10 13:45:57 -08:00