From 8825742c445222d61e27942ed963343876373a6d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 22 Jun 2017 20:48:18 +0200 Subject: [PATCH 1/6] mkosi: order package list alphabetically again --- .mkosi/mkosi.fedora | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.mkosi/mkosi.fedora b/.mkosi/mkosi.fedora index d36d167d94..a32d84ca32 100644 --- a/.mkosi/mkosi.fedora +++ b/.mkosi/mkosi.fedora @@ -32,7 +32,6 @@ RootSize=3G [Packages] BuildPackages= audit-libs-devel - meson bzip2-devel cryptsetup-devel dbus-devel @@ -63,6 +62,7 @@ BuildPackages= libxslt lz4 lz4-devel + meson pam-devel pkgconfig python3-devel From 54d8ef14d8ca92caa80c01bb6ebf09b5edb3df9d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 22 Jun 2017 20:49:12 +0200 Subject: [PATCH 2/6] time-util: rename usec_sub() to usec_sub_signed() and add usec_sub_unsigned() Quite often we just want to subtract two normal usec_t values, hence provide an implementation for that. --- src/basic/time-util.c | 12 +++--- src/basic/time-util.h | 14 ++++--- src/journal-remote/journal-upload-journal.c | 2 +- src/test/test-time.c | 44 +++++++++++++++------ 4 files changed, 49 insertions(+), 23 deletions(-) diff --git a/src/basic/time-util.c b/src/basic/time-util.c index 8d55af8492..51b06698ad 100644 --- a/src/basic/time-util.c +++ b/src/basic/time-util.c @@ -107,7 +107,7 @@ dual_timestamp* dual_timestamp_from_realtime(dual_timestamp *ts, usec_t u) { ts->realtime = u; delta = (int64_t) now(CLOCK_REALTIME) - (int64_t) u; - ts->monotonic = usec_sub(now(CLOCK_MONOTONIC), delta); + ts->monotonic = usec_sub_signed(now(CLOCK_MONOTONIC), delta); return ts; } @@ -124,8 +124,8 @@ triple_timestamp* triple_timestamp_from_realtime(triple_timestamp *ts, usec_t u) ts->realtime = u; delta = (int64_t) now(CLOCK_REALTIME) - (int64_t) u; - ts->monotonic = usec_sub(now(CLOCK_MONOTONIC), delta); - ts->boottime = clock_boottime_supported() ? usec_sub(now(CLOCK_BOOTTIME), delta) : USEC_INFINITY; + ts->monotonic = usec_sub_signed(now(CLOCK_MONOTONIC), delta); + ts->boottime = clock_boottime_supported() ? usec_sub_signed(now(CLOCK_BOOTTIME), delta) : USEC_INFINITY; return ts; } @@ -141,7 +141,7 @@ 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(now(CLOCK_REALTIME), delta); + ts->realtime = usec_sub_signed(now(CLOCK_REALTIME), delta); return ts; } @@ -156,8 +156,8 @@ dual_timestamp* dual_timestamp_from_boottime_or_monotonic(dual_timestamp *ts, us dual_timestamp_get(ts); delta = (int64_t) now(clock_boottime_or_monotonic()) - (int64_t) u; - ts->realtime = usec_sub(ts->realtime, delta); - ts->monotonic = usec_sub(ts->monotonic, delta); + ts->realtime = usec_sub_signed(ts->realtime, delta); + ts->monotonic = usec_sub_signed(ts->monotonic, delta); return ts; } diff --git a/src/basic/time-util.h b/src/basic/time-util.h index 7463507f51..8e2715cf68 100644 --- a/src/basic/time-util.h +++ b/src/basic/time-util.h @@ -169,19 +169,23 @@ static inline usec_t usec_add(usec_t a, usec_t b) { return c; } -static inline usec_t usec_sub(usec_t timestamp, int64_t delta) { - if (delta < 0) - return usec_add(timestamp, (usec_t) (-delta)); +static inline usec_t usec_sub_unsigned(usec_t timestamp, usec_t delta) { if (timestamp == USEC_INFINITY) /* Make sure infinity doesn't degrade */ return USEC_INFINITY; - - if (timestamp < (usec_t) delta) + if (timestamp < delta) return 0; return timestamp - delta; } +static inline usec_t usec_sub_signed(usec_t timestamp, int64_t delta) { + if (delta < 0) + return usec_add(timestamp, (usec_t) (-delta)); + else + return usec_sub_unsigned(timestamp, (usec_t) delta); +} + #if SIZEOF_TIME_T == 8 /* The last second we can format is 31. Dec 9999, 1s before midnight, because otherwise we'd enter 5 digit year * territory. However, since we want to stay away from this in all timezones we take one day off. */ diff --git a/src/journal-remote/journal-upload-journal.c b/src/journal-remote/journal-upload-journal.c index 8ce8e1895e..3a36e46ae0 100644 --- a/src/journal-remote/journal-upload-journal.c +++ b/src/journal-remote/journal-upload-journal.c @@ -251,7 +251,7 @@ static inline void check_update_watchdog(Uploader *u) { return; after = now(CLOCK_MONOTONIC); - elapsed_time = usec_sub(after, u->watchdog_timestamp); + elapsed_time = usec_sub_unsigned(after, u->watchdog_timestamp); if (elapsed_time > u->watchdog_usec / 2) { log_debug("Update watchdog timer"); sd_notify(false, "WATCHDOG=1"); diff --git a/src/test/test-time.c b/src/test/test-time.c index c9e31e90e1..5523263e76 100644 --- a/src/test/test-time.c +++ b/src/test/test-time.c @@ -195,16 +195,37 @@ static void test_usec_add(void) { assert_se(usec_add(USEC_INFINITY, 2) == USEC_INFINITY); } -static void test_usec_sub(void) { - assert_se(usec_sub(0, 0) == 0); - assert_se(usec_sub(4, 1) == 3); - assert_se(usec_sub(4, 4) == 0); - assert_se(usec_sub(4, 5) == 0); - assert_se(usec_sub(USEC_INFINITY-3, -3) == USEC_INFINITY); - assert_se(usec_sub(USEC_INFINITY-3, -3) == USEC_INFINITY); - assert_se(usec_sub(USEC_INFINITY-3, -4) == USEC_INFINITY); - assert_se(usec_sub(USEC_INFINITY-3, -5) == USEC_INFINITY); - assert_se(usec_sub(USEC_INFINITY, 5) == USEC_INFINITY); +static void test_usec_sub_unsigned(void) { + assert_se(usec_sub_unsigned(0, 0) == 0); + assert_se(usec_sub_unsigned(0, 2) == 0); + assert_se(usec_sub_unsigned(0, USEC_INFINITY) == 0); + assert_se(usec_sub_unsigned(1, 0) == 1); + assert_se(usec_sub_unsigned(1, 1) == 0); + assert_se(usec_sub_unsigned(1, 2) == 0); + assert_se(usec_sub_unsigned(1, 3) == 0); + assert_se(usec_sub_unsigned(1, USEC_INFINITY) == 0); + assert_se(usec_sub_unsigned(USEC_INFINITY-1, 0) == USEC_INFINITY-1); + assert_se(usec_sub_unsigned(USEC_INFINITY-1, 1) == USEC_INFINITY-2); + assert_se(usec_sub_unsigned(USEC_INFINITY-1, 2) == USEC_INFINITY-3); + assert_se(usec_sub_unsigned(USEC_INFINITY-1, USEC_INFINITY-2) == 1); + assert_se(usec_sub_unsigned(USEC_INFINITY-1, USEC_INFINITY-1) == 0); + assert_se(usec_sub_unsigned(USEC_INFINITY-1, USEC_INFINITY) == 0); + assert_se(usec_sub_unsigned(USEC_INFINITY, 0) == USEC_INFINITY); + assert_se(usec_sub_unsigned(USEC_INFINITY, 1) == USEC_INFINITY); + assert_se(usec_sub_unsigned(USEC_INFINITY, 2) == USEC_INFINITY); + assert_se(usec_sub_unsigned(USEC_INFINITY, USEC_INFINITY) == USEC_INFINITY); +} + +static void test_usec_sub_signed(void) { + assert_se(usec_sub_signed(0, 0) == 0); + assert_se(usec_sub_signed(4, 1) == 3); + assert_se(usec_sub_signed(4, 4) == 0); + assert_se(usec_sub_signed(4, 5) == 0); + assert_se(usec_sub_signed(USEC_INFINITY-3, -3) == USEC_INFINITY); + assert_se(usec_sub_signed(USEC_INFINITY-3, -3) == USEC_INFINITY); + assert_se(usec_sub_signed(USEC_INFINITY-3, -4) == USEC_INFINITY); + assert_se(usec_sub_signed(USEC_INFINITY-3, -5) == USEC_INFINITY); + assert_se(usec_sub_signed(USEC_INFINITY, 5) == USEC_INFINITY); } static void test_format_timestamp(void) { @@ -322,7 +343,8 @@ int main(int argc, char *argv[]) { test_timezone_is_valid(); test_get_timezones(); test_usec_add(); - test_usec_sub(); + test_usec_sub_signed(); + test_usec_sub_unsigned(); test_format_timestamp(); test_format_timestamp_utc(); test_dual_timestamp_deserialize(); From 1007ec60e664da03b7aea4803c643d991fcf6530 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 22 Jun 2017 20:52:23 +0200 Subject: [PATCH 3/6] time-util: add new call usec_shift_clock() for converting times between clocks We use that quite often, let's implement one clean version of it. --- src/basic/time-util.c | 19 +++++++++++++++++++ src/basic/time-util.h | 2 ++ src/test/test-time.c | 39 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+) diff --git a/src/basic/time-util.c b/src/basic/time-util.c index 51b06698ad..b0b181120a 100644 --- a/src/basic/time-util.c +++ b/src/basic/time-util.c @@ -1351,3 +1351,22 @@ unsigned long usec_to_jiffies(usec_t u) { return DIV_ROUND_UP(u , USEC_PER_SEC / hz); } + +usec_t usec_shift_clock(usec_t x, clockid_t from, clockid_t to) { + usec_t a, b; + + if (x == USEC_INFINITY) + return USEC_INFINITY; + if (map_clock_id(from) == map_clock_id(to)) + return x; + + a = now(from); + b = now(to); + + if (x > a) + /* x lies in the future */ + return usec_add(b, usec_sub_unsigned(x, a)); + else + /* x lies in the past */ + return usec_sub_unsigned(b, usec_sub_unsigned(a, x)); +} diff --git a/src/basic/time-util.h b/src/basic/time-util.h index 8e2715cf68..414995e6af 100644 --- a/src/basic/time-util.h +++ b/src/basic/time-util.h @@ -145,6 +145,8 @@ bool clock_boottime_supported(void); bool clock_supported(clockid_t clock); clockid_t clock_boottime_or_monotonic(void); +usec_t usec_shift_clock(usec_t, clockid_t from, clockid_t to); + #define xstrftime(buf, fmt, tm) \ assert_message_se(strftime(buf, ELEMENTSOF(buf), fmt, tm) > 0, \ "xstrftime: " #buf "[] must be big enough") diff --git a/src/test/test-time.c b/src/test/test-time.c index 5523263e76..d9a6d612f2 100644 --- a/src/test/test-time.c +++ b/src/test/test-time.c @@ -331,6 +331,44 @@ static void test_dual_timestamp_deserialize(void) { assert_se(t.monotonic == 0); } +static void assert_similar(usec_t a, usec_t b) { + usec_t d; + + if (a > b) + d = a - b; + else + d = b - a; + + assert(d < 10*USEC_PER_SEC); +} + +static void test_usec_shift_clock(void) { + usec_t rt, mn, bt; + + rt = now(CLOCK_REALTIME); + mn = now(CLOCK_MONOTONIC); + bt = now(clock_boottime_or_monotonic()); + + assert_se(usec_shift_clock(USEC_INFINITY, CLOCK_REALTIME, CLOCK_MONOTONIC) == USEC_INFINITY); + + assert_similar(usec_shift_clock(rt + USEC_PER_HOUR, CLOCK_REALTIME, CLOCK_MONOTONIC), mn + USEC_PER_HOUR); + assert_similar(usec_shift_clock(rt + 2*USEC_PER_HOUR, CLOCK_REALTIME, clock_boottime_or_monotonic()), bt + 2*USEC_PER_HOUR); + assert_se(usec_shift_clock(rt + 3*USEC_PER_HOUR, CLOCK_REALTIME, CLOCK_REALTIME_ALARM) == rt + 3*USEC_PER_HOUR); + + assert_similar(usec_shift_clock(mn + 4*USEC_PER_HOUR, CLOCK_MONOTONIC, CLOCK_REALTIME_ALARM), rt + 4*USEC_PER_HOUR); + assert_similar(usec_shift_clock(mn + 5*USEC_PER_HOUR, CLOCK_MONOTONIC, clock_boottime_or_monotonic()), bt + 5*USEC_PER_HOUR); + assert_se(usec_shift_clock(mn + 6*USEC_PER_HOUR, CLOCK_MONOTONIC, CLOCK_MONOTONIC) == mn + 6*USEC_PER_HOUR); + + assert_similar(usec_shift_clock(bt + 7*USEC_PER_HOUR, clock_boottime_or_monotonic(), CLOCK_MONOTONIC), mn + 7*USEC_PER_HOUR); + assert_similar(usec_shift_clock(bt + 8*USEC_PER_HOUR, clock_boottime_or_monotonic(), CLOCK_REALTIME_ALARM), rt + 8*USEC_PER_HOUR); + assert_se(usec_shift_clock(bt + 9*USEC_PER_HOUR, clock_boottime_or_monotonic(), clock_boottime_or_monotonic()) == bt + 9*USEC_PER_HOUR); + + if (mn > USEC_PER_MINUTE) { + assert_similar(usec_shift_clock(rt - 30 * USEC_PER_SEC, CLOCK_REALTIME_ALARM, CLOCK_MONOTONIC), mn - 30 * USEC_PER_SEC); + assert_similar(usec_shift_clock(rt - 50 * USEC_PER_SEC, CLOCK_REALTIME, clock_boottime_or_monotonic()), bt - 50 * USEC_PER_SEC); + } +} + int main(int argc, char *argv[]) { uintmax_t x; @@ -348,6 +386,7 @@ int main(int argc, char *argv[]) { test_format_timestamp(); test_format_timestamp_utc(); test_dual_timestamp_deserialize(); + test_usec_shift_clock(); /* Ensure time_t is signed */ assert_cc((time_t) -1 < (time_t) 1); From 79fc8b9623354a98ca81b4d35e576e9c9e3bf687 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 22 Jun 2017 20:59:48 +0200 Subject: [PATCH 4/6] timer: convert property_get_next_elapse_monotonic() to use usec_shift_clock() Let's use the generic clock shifting logic here. --- src/core/dbus-timer.c | 20 +++----------------- src/core/timer.h | 2 ++ 2 files changed, 5 insertions(+), 17 deletions(-) diff --git a/src/core/dbus-timer.c b/src/core/dbus-timer.c index efbb0e8915..c98282a5d4 100644 --- a/src/core/dbus-timer.c +++ b/src/core/dbus-timer.c @@ -144,28 +144,14 @@ static int property_get_next_elapse_monotonic( sd_bus_error *error) { Timer *t = userdata; - usec_t x; assert(bus); assert(reply); assert(t); - if (t->next_elapse_monotonic_or_boottime <= 0) - x = 0; - else if (t->wake_system) { - usec_t a, b; - - a = now(CLOCK_MONOTONIC); - b = now(clock_boottime_or_monotonic()); - - if (t->next_elapse_monotonic_or_boottime + a > b) - x = t->next_elapse_monotonic_or_boottime + a - b; - else - x = 0; - } else - x = t->next_elapse_monotonic_or_boottime; - - return sd_bus_message_append(reply, "t", x); + return sd_bus_message_append(reply, "t", + (uint64_t) usec_shift_clock(t->next_elapse_monotonic_or_boottime, + TIMER_MONOTONIC_CLOCK(t), CLOCK_MONOTONIC)); } const sd_bus_vtable bus_timer_vtable[] = { diff --git a/src/core/timer.h b/src/core/timer.h index 9c4b64f898..546c60d750 100644 --- a/src/core/timer.h +++ b/src/core/timer.h @@ -78,6 +78,8 @@ struct Timer { char *stamp_path; }; +#define TIMER_MONOTONIC_CLOCK(t) ((t)->wake_system && clock_boottime_supported() ? CLOCK_BOOTTIME_ALARM : CLOCK_MONOTONIC) + void timer_free_values(Timer *t); extern const UnitVTable timer_vtable; From c54be90b28cc25663ff52c505956fa3de0bea1fb Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 22 Jun 2017 21:04:20 +0200 Subject: [PATCH 5/6] timer: make sure we use the right monotonic timestamp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- src/core/timer.c | 41 +++++++++++------------------------------ 1 file changed, 11 insertions(+), 30 deletions(-) diff --git a/src/core/timer.c b/src/core/timer.c index af67b7591a..701949fd60 100644 --- 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 = usec_add(usec_shift_clock(base, CLOCK_MONOTONIC, TIMER_MONOTONIC_CLOCK(t)), v->value); - v->next_elapse = base + 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) { From c4834ffaefa8cd4ec18be97395d8a3e8ad1748e6 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 22 Jun 2017 21:10:34 +0200 Subject: [PATCH 6/6] tests: show current monotonic/boottime/realtime clock values in test-time When debugging time issues its kinda handy to have an easy way to query the three clocks, hence let's just output them at the beginning of test-time. --- src/test/test-time.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/test/test-time.c b/src/test/test-time.c index d9a6d612f2..601e835f16 100644 --- a/src/test/test-time.c +++ b/src/test/test-time.c @@ -372,6 +372,13 @@ static void test_usec_shift_clock(void) { int main(int argc, char *argv[]) { uintmax_t x; + log_info("realtime=" USEC_FMT "\n" + "monotonic=" USEC_FMT "\n" + "boottime=" USEC_FMT "\n", + now(CLOCK_REALTIME), + now(CLOCK_MONOTONIC), + now(clock_boottime_or_monotonic())); + test_parse_sec(); test_parse_time(); test_parse_nsec();