jlaitine commented on PR #13984:
URL: https://github.com/apache/nuttx/pull/13984#issuecomment-2404211400
Now I am able to re-produce the issue you are reporting in qemu, but not in
real HW.
The issue in qemu is, that sometimes servicing the timer interrupt is
delayed until close to the end of the next tick, or past it. This issue must
be related to real-time behaviour of qemu.
The same would happen also in a real HW, in case of interrupt storm or in
case of very long critical section, delaying the timer interrupt.
There is no way around that issue! In case servicing the timer interrupt is
late, it just MUST proceed several ticks at once to keep up. The same would
happen if you e.g. break the target with a debugger, and the timer is freely
running in the background.
Sorry to say, but I believe this patch is not right. As I explained before,
every time you set the timer comparator, you must set the next value to be
exactly the previous_comparator_time + tick_time. So you need to know the
previous time, which was set somehow.
If you do as suggested in this patch, setting it to current_time() + tick
time, you add a delay of
`deltaT = current_time() - previous_time; `
to each tick, causing timer drift.
However, when inspecting the full flow, I did find one race condition, which
could cause an error. This is in theory, so unlikely that testing it is
probably impossible without specifically planned instrumentation code...
This part in arm64_arch_timer.c
```
next_cycle =
arm64_arch_timer_count() / priv->cycle_per_tick * priv->cycle_per_tick +
ticks * priv->cycle_per_tick;
arm64_arch_timer_set_compare(next_cycle);
```
Has a potential race in the case that tick timer interrupt is missed (long
due), and the new tick occurs between the lines of calculating next cycle and
setting the compare, this could set the compare to the past.
Even though the possibility of this would be non-existent in practice, we
can fix this easily if we want, by just checking the possibility that we didn't
set the time in the past:
```
do
{
next_cycle =
arm64_arch_timer_count() / priv->cycle_per_tick * priv->cycle_per_tick
+
ticks * priv->cycle_per_tick;
arm64_arch_timer_set_compare(next_cycle);
} while(next_cycle - arm64_arch_timer_count() < priv->cycle_per_tick); /*
arm64_arch_timer_count() < next_cycle */
```
I will provide a patch for this. Even if it is highly theoretical case, it
is probably worth fixing. This has nothing to do with the issue you are seeing
in qemu, though.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
