From 7c0eb30e329940a1bed6fe1df8c9bc122385cbe4 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 21 Jul 2020 17:30:49 +0200 Subject: [PATCH 1/3] time-util: rework clock conversion logic Let's split this out into its own helper function we can reuse at various places. Also, let's avoid signed values where we can so that we can cover more of the available time range. --- src/basic/time-util.c | 82 ++++++++++++++++++++++++++++++++----------- src/basic/time-util.h | 6 ++-- 2 files changed, 66 insertions(+), 22 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); From d3926f9a465c735054d645c288935e8851fc12b4 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 21 Jul 2020 17:33:36 +0200 Subject: [PATCH 2/3] test: add basic test for clock mapping --- src/test/test-time-util.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) 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); From 58afc4f8e41176c890b171f422b988fd83887605 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 21 Jul 2020 17:33:19 +0200 Subject: [PATCH 3/3] core: don't acquire dual timestamp needlessly if we don't need it in .timer handling Follow-up for: 26698337f3842842af51cd007485f1dcd7c43cf2 --- src/core/timer.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) 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;