Fix-Point commented on PR #15938: URL: https://github.com/apache/nuttx/pull/15938#issuecomment-2703593592
> Adding one example with real numbers how the timer drift causes the tick timer & watchdog timers to fail: > > Example: > > Counter frequency 24000000 (24MHz) Tick frequency 100 Hz > > 1. Tick starts at 1645919972; tick number is 1645919972 * 100 / 24000000 = 6857.9999 = 6857 > 2. Set interrupt in 1 tick time > 3. Next tick starts at 1646160004; tick number is 1646160004 * 100 / 24000000 = 6859.0000 = 6859 > > Result: > > 2 ticks has passed (6859 - 6857 = 2). > > Actual time passed is (1646160004 - 1645919972) / 24000000 = 0.010001 s (10 ms). > > So it has progressed 2 10 ms ticks in 10 ms -> epic fail. > > Addendum: And how does this break tick based wdog you may ask? This is one example, variations of this exist. > > 1. Imagine you set e.g. a sem_wait_noninterruptible for 1 tick, at a moment in time close to the end of current OS tick > 2. Next tick starts (timer interrupt) e.g. after 10 ns > 3. If now the tick timer progresses 2 ticks instead of 1 (which is shown to happen above every now and then), the WDOG fires immediately now > 4. the WDOG which was supposed to sleep _at least_ for 1 OS tick fires within 10 ns of being set. Thank you for your detailed explanation. I understand the time drift problem you are talking about. But this patch is to solve the time drift problem caused by another frequency. For example, on the Cortex R-82, the core frequency is 2.73Mhz. And we use `USEC_PER_TICK = 10`. If we use `cycle_per_tick` for calculation, it will cause precision loss: cycle_per_tick = 2,730,000 / (1,000,000 / 10) = 2,730,000 / 100, 000 = 27.3 = 27. Here, using `cycle_per_tick` to calculate will cause a time error of about 1% to accumulate for each tick, but if we use the frequency to calculate directly, the error will not accumulate. Regarding the problem you described, the original implementation of aligning the tick scale also has this problem. Let's take an example. Assume that the frequency is `62500000`, `USEC_PER_TICK = 10`, `cycle_per_tick = 625`. tick |--------------------|-----------t0---t1--| let `t_0 = 3539791 (625 * 5663 + 416)` be the time when we call `sleep(1tick)`. let `t_1 = 3539876 (625 * 5664 - 124)` be the time when we call arm64_tick_start(1tick). We calculate the next expiration cycle via `arm64_arch_timer_count() / priv->cycle_per_tick * priv->cycle_per_tick +ticks * priv->cycle_per_tick` That is: `3539876 / 625 * 625 + 1 * 625 = 3540000` (exact tick 5664). The time difference between next expiration time and `t0` is `3540000 - 3539791 = 209`. This makes us only sleep for `209` cycles to trigger the timer, instead of at least 625 cycles to trigger the timer, causing the timer fired prematurely. In the case of using relative timer: The next expiration cycle is calculate by: `arm64_arch_timer_count() + ticks * priv->frequency / TICK_PER_SEC = 3539876 + 1 * 62500000 / 100000 = 3539876 + 625 = 3540501`. Which is 710 cycles next to the `t_0`, and greater than 1 tick. Satisfy the requirement of sleeping for at least 1 tick. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
