timer: make sure we use the right monotonic timestamp

This reworks timer_enter_waiting() in a couple of ways in order to clean
it up a bit and fix #5629.

Most importantly, we previously we initialized ts_monotonic to either
the current time in CLOCK_MONOTONIC or in CLOCK_BOOTTIME, depending on
t->wake_system. Then given specific conditions we'd use this time as
base for our timers. And afterwards, if t->wake_system was on we'd
convetr the resulting value from CLOCK_MONOTONIC to CLOCK_BOOTTIME again
— which of course is wrong since we already were in CLOCK_BOOTTIME! This
fixes this logic, by using a triple timestamp so that we always have the
right base around, and initially only calculate in CLOCK_MONOTONIC and
only convert as last step.

Conversion between the clocks is now done with the generic
usec_shift_clock(), and additions via usec_add() making these
calculations a bit safer.

Fixes: #5629

diff --git a/src/core/timer.c b/src/core/timer.c
index af67b7591a54c658453f2111c3ab637c8a63bbca..701949fd60df580a3c84ce81f476bdd8d53f5cd8 100644 (file)
--- a/src/core/timer.c
+++ b/src/core/timer.c
@@ -316,21 +316,6 @@ static void timer_enter_elapsed(Timer *t, bool leave_around) {
                 timer_enter_dead(t, TIMER_SUCCESS);
 }
 
-static usec_t monotonic_to_boottime(usec_t t) {
-        usec_t a, b;
-
-        if (t <= 0)
-                return 0;
-
-        a = now(clock_boottime_or_monotonic());
-        b = now(CLOCK_MONOTONIC);
-
-        if (t + a > b)
-                return t + a - b;
-        else
-                return 0;
-}
-
 static void add_random(Timer *t, usec_t *v) {
         char s[FORMAT_TIMESPAN_MAX];
         usec_t add;
@@ -355,9 +340,9 @@ static void add_random(Timer *t, usec_t *v) {
 
 static void timer_enter_waiting(Timer *t, bool initial) {
         bool found_monotonic = false, found_realtime = false;
-        usec_t ts_realtime, ts_monotonic;
-        usec_t base = 0;
         bool leave_around = false;
+        triple_timestamp ts;
+        usec_t base = 0;
         TimerValue *v;
         Unit *trigger;
         int r;
@@ -371,11 +356,7 @@ static void timer_enter_waiting(Timer *t, bool initial) {
                 return;
         }
 
-        /* If we shall wake the system we use the boottime clock
-         * rather than the monotonic clock. */
-
-        ts_realtime = now(CLOCK_REALTIME);
-        ts_monotonic = now(t->wake_system ? clock_boottime_or_monotonic() : CLOCK_MONOTONIC);
+        triple_timestamp_get(&ts);
         t->next_elapse_monotonic_or_boottime = t->next_elapse_realtime = 0;
 
         LIST_FOREACH(value, v, t->values) {
@@ -391,7 +372,7 @@ static void timer_enter_waiting(Timer *t, bool initial) {
                          * to that. If we don't just start from
                          * now. */
 
-                        b = t->last_trigger.realtime > 0 ? t->last_trigger.realtime : ts_realtime;
+                        b = t->last_trigger.realtime > 0 ? t->last_trigger.realtime : ts.realtime;
 
                         r = calendar_spec_next_usec(v->calendar_spec, b, &v->next_elapse);
                         if (r < 0)
@@ -405,13 +386,14 @@ static void timer_enter_waiting(Timer *t, bool initial) {
                         found_realtime = true;
 
                 } else  {
+
                         switch (v->base) {
 
                         case TIMER_ACTIVE:
                                 if (state_translation_table[t->state] == UNIT_ACTIVE)
                                         base = UNIT(t)->inactive_exit_timestamp.monotonic;
                                 else
-                                        base = ts_monotonic;
+                                        base = ts.monotonic;
                                 break;
 
                         case TIMER_BOOT:
@@ -456,12 +438,11 @@ static void timer_enter_waiting(Timer *t, bool initial) {
                                 assert_not_reached("Unknown timer base");
                         }
 
-                        if (t->wake_system)
-                                base = monotonic_to_boottime(base);
-
-                        v->next_elapse = base + v->value;
+                        v->next_elapse = usec_add(usec_shift_clock(base, CLOCK_MONOTONIC, TIMER_MONOTONIC_CLOCK(t)), v->value);
 
-                        if (!initial && v->next_elapse < ts_monotonic && IN_SET(v->base, TIMER_ACTIVE, TIMER_BOOT, TIMER_STARTUP)) {
+                        if (!initial &&
+                            v->next_elapse < triple_timestamp_by_clock(&ts, TIMER_MONOTONIC_CLOCK(t)) &&
+                            IN_SET(v->base, TIMER_ACTIVE, TIMER_BOOT, TIMER_STARTUP)) {
                                 /* This is a one time trigger, disable it now */
                                 v->disabled = true;
                                 continue;
@@ -488,7 +469,7 @@ static void timer_enter_waiting(Timer *t, bool initial) {
 
                 add_random(t, &t->next_elapse_monotonic_or_boottime);
 
-                left = t->next_elapse_monotonic_or_boottime > ts_monotonic ? t->next_elapse_monotonic_or_boottime - ts_monotonic : 0;
+                left = usec_sub_unsigned(t->next_elapse_monotonic_or_boottime, triple_timestamp_by_clock(&ts, TIMER_MONOTONIC_CLOCK(t)));
                 log_unit_debug(UNIT(t), "Monotonic timer elapses in %s.", format_timespan(buf, sizeof(buf), left, 0));
 
                 if (t->monotonic_event_source) {