Fix cumulative drift in periodic timers

[visitorckw]

Currently, periodic timers allow errors to accumulate because the next deadline is calculated relative to the current system tick (now + period). Any latency in the tick handler execution—whether due to interrupt jitter or system load—permanently shifts the phase of all future expirations.

Fix the drift by anchoring the timer schedule to an absolute baseline. A new field, last_expected_fire_tick, is introduced to track the theoretical expiration time.

When the timer fires, the next deadline is now computed by advancing this expected time by the period, rather than relying on the actual execution time. This ensures the timer maintains its long-term frequency accuracy regardless of short-term scheduling delays.

---

Summary by cubic

Prevent cumulative drift in periodic timers by scheduling against an expected baseline tick instead of now + period. Timers keep stable frequency even under interrupt jitter or handler delays.

• Bug Fixes
  • Added last_expected_fire_tick to track the theoretical next fire time.
  • On start, initialize last_expected_fire_tick and set deadline_ticks to this value.
  • On each auto-reload, advance last_expected_fire_tick by the period and set deadline_ticks from it.

^{Written for commit fff7036. Summary will update automatically on new commits.}

[cubic-dev-ai]

No issues found across 2 files

[jserv]

I defer to @HeatCrab for confirmation.

[visitorckw]

I ran clang-format18 -i *.[ch] locally and didn't see any changes, but the CI results are showing clang-format errors. I'm confused about what I might have missed.

[jserv]

I ran clang-format18 -i *.[ch] locally and didn't see any changes, but the CI results are showing clang-format errors. I'm confused about what I might have missed.

Consider the changes below:

--- a/include/sys/timer.h
+++ b/include/sys/timer.h
@@ -26,8 +26,10 @@ typedef enum {
 typedef struct {
     /* Timing Parameters */
     uint32_t deadline_ticks; /* Expiration time in absolute system ticks */
-    uint32_t last_expected_fire_tick; /* Last calculated expected fire time for periodic timers */
-    uint32_t period_ms;      /* Reload period in milliseconds */
+    uint32_t last_expected_fire_tick; /* Last calculated expected fire time for
+                                       * periodic timer
+                                       */
+    uint32_t period_ms;               /* Reload period in milliseconds */
 
     /* Timer Identification and State */
     uint16_t id;       /* Unique handle assigned by the kernel */

I plan to migrate to newer clang-format versions once Ubuntu 24.04 LTS provides them.

[HeatCrab]

Hi, the fix is actually LGTM!

However, while reviewing the timer tick calculation, I was wondering if this could cause unexpected overflow conditions.

After tracing the code, I found out that _timer_tick_handler and timer_sorted_insert in kernel/timer.c may have potential wrap-around issues.

Take _timer_tick_handler function as an example:

void _timer_tick_handler(void)
{
	// omitted	

        if (now >= t->deadline_ticks) {
            expired_timers[expired_count++] = t;
            kcb->timer_list->head->next = node->next;
            kcb->timer_list->length--;
            return_timer_node(node);
	...

In the line if (now >= t->deadline_ticks) {, if deadline_ticks now overflow, this check is going to fail badly.

To clarify, the issue is pre-exist and is out of scope for this PR, so I don't think we need to fix it here to avoid scope creep.
But, we should probably track this. Would you mind opening a separate issue for the overflow safety, or do you have other suggestions?

[visitorckw]

Yeah, I've actually thought about the same thing, but I haven't dug deep enough to figure out a solution yet.
Feel free to open an issue for further discussion.
Thanks!

[HeatCrab]

Yeah, I've actually thought about the same thing, but I haven't dug deep enough to figure out a solution yet. Feel free to open an issue for further discussion. Thanks!

Sure! I've opened an issue (#61) to track this.

[jserv]

Thank @visitorckw for contributing!