Merge pull request #16536 from poettering/time-clock-map-fixes

time-util: clock mapping improvements

4 files changed, 108 insertions, 30 deletions

diff --git a/src/basic/time-util.c b/src/basic/time-util.c
index 105584e2e7..15cc1b8851 100644
--- a/src/basic/time-util.c
+++ b/src/basic/time-util.c
@@ -82,43 +82,82 @@ triple_timestamp* triple_timestamp_get(triple_timestamp *ts) {
         return ts;
 }
 
+static usec_t map_clock_usec_internal(usec_t from, usec_t from_base, usec_t to_base) {
+
+        /* Maps the time 'from' between two clocks, based on a common reference point where the first clock
+         * is at 'from_base' and the second clock at 'to_base'. Basically calculates:
+         *
+         *         from - from_base + to_base
+         *
+         * But takes care of overflows/underflows and avoids signed operations. */
+
+        if (from >= from_base) { /* In the future */
+                usec_t delta = from - from_base;
+
+                if (to_base >= USEC_INFINITY - delta) /* overflow? */
+                        return USEC_INFINITY;
+
+                return to_base + delta;
+
+        } else { /* In the past */
+                usec_t delta = from_base - from;
+
+                if (to_base <= delta) /* underflow? */
+                        return 0;
+
+                return to_base - delta;
+        }
+}
+
+usec_t map_clock_usec(usec_t from, clockid_t from_clock, clockid_t to_clock) {
+
+        /* Try to avoid any inaccuracy needlessly added in case we convert from effectively the same clock
+         * onto itself */
+        if (map_clock_id(from_clock) == map_clock_id(to_clock))
+                return from;
+
+        /* Keep infinity as is */
+        if (from == USEC_INFINITY)
+                return from;
+
+        return map_clock_usec_internal(from, now(from_clock), now(to_clock));
+}
+
 dual_timestamp* dual_timestamp_from_realtime(dual_timestamp *ts, usec_t u) {
-        int64_t delta;
         assert(ts);
 
-        if (u == USEC_INFINITY || u <= 0) {
+        if (u == USEC_INFINITY || u == 0) {
                 ts->realtime = ts->monotonic = u;
                 return ts;
         }
 
         ts->realtime = u;
-
-        delta = (int64_t) now(CLOCK_REALTIME) - (int64_t) u;
-        ts->monotonic = usec_sub_signed(now(CLOCK_MONOTONIC), delta);
-
+        ts->monotonic = map_clock_usec(u, CLOCK_REALTIME, CLOCK_MONOTONIC);
         return ts;
 }
 
 triple_timestamp* triple_timestamp_from_realtime(triple_timestamp *ts, usec_t u) {
-        int64_t delta;
+        usec_t nowr;
 
         assert(ts);
 
-        if (u == USEC_INFINITY || u <= 0) {
+        if (u == USEC_INFINITY || u == 0) {
                 ts->realtime = ts->monotonic = ts->boottime = u;
                 return ts;
         }
 
+        nowr = now(CLOCK_REALTIME);
+
         ts->realtime = u;
-        delta = (int64_t) now(CLOCK_REALTIME) - (int64_t) u;
-        ts->monotonic = usec_sub_signed(now(CLOCK_MONOTONIC), delta);
-        ts->boottime = clock_boottime_supported() ? usec_sub_signed(now(CLOCK_BOOTTIME), delta) : USEC_INFINITY;
+        ts->monotonic = map_clock_usec_internal(u, nowr, now(CLOCK_MONOTONIC));
+        ts->boottime = clock_boottime_supported() ?
+                map_clock_usec_internal(u, nowr, now(CLOCK_BOOTTIME)) :
+                USEC_INFINITY;
 
         return ts;
 }
 
 dual_timestamp* dual_timestamp_from_monotonic(dual_timestamp *ts, usec_t u) {
-        int64_t delta;
         assert(ts);
 
         if (u == USEC_INFINITY) {
@@ -127,25 +166,28 @@ dual_timestamp* dual_timestamp_from_monotonic(dual_timestamp *ts, usec_t u) {
         }
 
         ts->monotonic = u;
-        delta = (int64_t) now(CLOCK_MONOTONIC) - (int64_t) u;
-        ts->realtime = usec_sub_signed(now(CLOCK_REALTIME), delta);
-
+        ts->realtime = map_clock_usec(u, CLOCK_MONOTONIC, CLOCK_REALTIME);
         return ts;
 }
 
 dual_timestamp* dual_timestamp_from_boottime_or_monotonic(dual_timestamp *ts, usec_t u) {
-        int64_t delta;
+        clockid_t cid;
+        usec_t nowm;
 
         if (u == USEC_INFINITY) {
                 ts->realtime = ts->monotonic = USEC_INFINITY;
                 return ts;
         }
 
-        dual_timestamp_get(ts);
-        delta = (int64_t) now(clock_boottime_or_monotonic()) - (int64_t) u;
-        ts->realtime = usec_sub_signed(ts->realtime, delta);
-        ts->monotonic = usec_sub_signed(ts->monotonic, delta);
+        cid = clock_boottime_or_monotonic();
+        nowm = now(cid);
+
+        if (cid == CLOCK_MONOTONIC)
+                ts->monotonic = u;
+        else
+                ts->monotonic = map_clock_usec_internal(u, nowm, now(CLOCK_MONOTONIC));
 
+        ts->realtime = map_clock_usec_internal(u, nowm, now(CLOCK_REALTIME));
         return ts;
 }
 
diff --git a/src/basic/time-util.h b/src/basic/time-util.h
index 4c371257e3..9bbe986306 100644
--- a/src/basic/time-util.h
+++ b/src/basic/time-util.h
@@ -29,8 +29,8 @@ typedef struct triple_timestamp {
         usec_t boottime;
 } triple_timestamp;
 
-#define USEC_INFINITY ((usec_t) -1)
-#define NSEC_INFINITY ((nsec_t) -1)
+#define USEC_INFINITY ((usec_t) UINT64_MAX)
+#define NSEC_INFINITY ((nsec_t) UINT64_MAX)
 
 #define MSEC_PER_SEC  1000ULL
 #define USEC_PER_SEC  ((usec_t) 1000000ULL)
@@ -67,6 +67,8 @@ typedef struct triple_timestamp {
 usec_t now(clockid_t clock);
 nsec_t now_nsec(clockid_t clock);
 
+usec_t map_clock_usec(usec_t from, clockid_t from_clock, clockid_t to_clock);
+
 dual_timestamp* dual_timestamp_get(dual_timestamp *ts);
 dual_timestamp* dual_timestamp_from_realtime(dual_timestamp *ts, usec_t u);
 dual_timestamp* dual_timestamp_from_monotonic(dual_timestamp *ts, usec_t u);
diff --git a/src/core/timer.c b/src/core/timer.c
index 75f1dc1f8b..03a9c14f76 100644
--- a/src/core/timer.c
+++ b/src/core/timer.c
@@ -346,7 +346,6 @@ static void timer_enter_waiting(Timer *t, bool time_change) {
         bool found_monotonic = false, found_realtime = false;
         bool leave_around = false;
         triple_timestamp ts;
-        dual_timestamp dts;
         TimerValue *v;
         Unit *trigger;
         int r;
@@ -368,7 +367,7 @@ static void timer_enter_waiting(Timer *t, bool time_change) {
                         continue;
 
                 if (v->base == TIMER_CALENDAR) {
-                        usec_t b;
+                        usec_t b, rebased;
 
                         /* If we know the last time this was
                          * triggered, schedule the job based relative
@@ -388,12 +387,14 @@ static void timer_enter_waiting(Timer *t, bool time_change) {
                         if (r < 0)
                                 continue;
 
-                        /* To make the delay due to RandomizedDelaySec= work even at boot,
-                         * if the scheduled time has already passed, set the time when systemd
-                         * first started as the scheduled time. */
-                        dual_timestamp_from_monotonic(&dts, UNIT(t)->manager->timestamps[MANAGER_TIMESTAMP_USERSPACE].monotonic);
-                        if (v->next_elapse < dts.realtime)
-                                v->next_elapse = dts.realtime;
+                        /* To make the delay due to RandomizedDelaySec= work even at boot, if the scheduled
+                         * time has already passed, set the time when systemd first started as the scheduled
+                         * time. Note that we base this on the monotonic timestamp of the boot, not the
+                         * realtime one, since the wallclock might have been off during boot. */
+                        rebased = map_clock_usec(UNIT(t)->manager->timestamps[MANAGER_TIMESTAMP_USERSPACE].monotonic,
+                                                 CLOCK_MONOTONIC, CLOCK_REALTIME);
+                        if (v->next_elapse < rebased)
+                                v->next_elapse = rebased;
 
                         if (!found_realtime)
                                 t->next_elapse_realtime = v->next_elapse;
diff --git a/src/test/test-time-util.c b/src/test/test-time-util.c
index e3b1f6f8ca..8826956d10 100644
--- a/src/test/test-time-util.c
+++ b/src/test/test-time-util.c
@@ -483,6 +483,38 @@ static void test_in_utc_timezone(void) {
         assert_se(unsetenv("TZ") >= 0);
 }
 
+static void test_map_clock_usec(void) {
+        usec_t nowr, x, y, z;
+
+        log_info("/* %s */", __func__);
+        nowr = now(CLOCK_REALTIME);
+
+        x = nowr; /* right now */
+        y = map_clock_usec(x, CLOCK_REALTIME, CLOCK_MONOTONIC);
+        z = map_clock_usec(y, CLOCK_MONOTONIC, CLOCK_REALTIME);
+        /* Converting forth and back will introduce inaccuracies, since we cannot query both clocks atomically, but it should be small. Even on the slowest CI smaller than 1h */
+
+        assert_se((z > x ? z - x : x - z) < USEC_PER_HOUR);
+
+        assert_se(nowr < USEC_INFINITY - USEC_PER_DAY*7); /* overflow check */
+        x = nowr + USEC_PER_DAY*7; /* 1 week from now */
+        y = map_clock_usec(x, CLOCK_REALTIME, CLOCK_MONOTONIC);
+        assert_se(y > 0 && y < USEC_INFINITY);
+        z = map_clock_usec(y, CLOCK_MONOTONIC, CLOCK_REALTIME);
+        assert_se(z > 0 && z < USEC_INFINITY);
+        assert_se((z > x ? z - x : x - z) < USEC_PER_HOUR);
+
+        assert_se(nowr > USEC_PER_DAY * 7); /* underflow check */
+        x = nowr - USEC_PER_DAY*7; /* 1 week ago */
+        y = map_clock_usec(x, CLOCK_REALTIME, CLOCK_MONOTONIC);
+        if (y != 0) { /* might underflow if machine is not up long enough for the monotonic clock to be beyond 1w */
+                assert_se(y < USEC_INFINITY);
+                z = map_clock_usec(y, CLOCK_MONOTONIC, CLOCK_REALTIME);
+                assert_se(z > 0 && z < USEC_INFINITY);
+                assert_se((z > x ? z - x : x - z) < USEC_PER_HOUR);
+        }
+}
+
 int main(int argc, char *argv[]) {
         test_setup_logging(LOG_INFO);
 
@@ -511,6 +543,7 @@ int main(int argc, char *argv[]) {
         test_deserialize_dual_timestamp();
         test_usec_shift_clock();
         test_in_utc_timezone();
+        test_map_clock_usec();
 
         /* Ensure time_t is signed */
         assert_cc((time_t) -1 < (time_t) 1);