timer: avoid re-arming WakeSystem=yes timer after suspend
There's an edge case where a single-shot timer using WakeSystem=yes
could get re-armed after elapsing. For example, when a timer is created
using:
$ systemd-run --user --on-active="1m" --timer-property=WakeSystem=yes flatpak run io.bassi.Amberol
and the system is then suspended, following sequence of events may
happen:
Jul 08 06:57:25 systemd[2640]: run-p192456-i199160.timer: Installed new job run-p192456-i199160.timer/start as 7675
Jul 08 06:57:25 systemd[2640]: run-p192456-i199160.timer: Enqueued job run-p192456-i199160.timer/start as 7675
Jul 08 06:57:25 systemd[2640]: run-p192456-i199160.timer: Monotonic timer elapses in 59.999999s.
Jul 08 06:57:25 systemd[2640]: run-p192456-i199160.timer: Changed dead -> waiting
Jul 08 06:57:25 systemd[2640]: run-p192456-i199160.timer: Job 7675 run-p192456-i199160.timer/start finished, result=done
Jul 08 06:57:25 systemd[2640]: Started [systemd-run] /usr/bin/flatpak run io.bassi.Amberol.
Jul 08 06:58:13 systemd[2640]: run-p192456-i199160.timer: Time change, recalculating next elapse.
Jul 08 06:58:13 systemd[2640]: run-p192456-i199160.timer: Monotonic timer elapses in 12.785674s.
Jul 08 06:58:26 systemd[2640]: run-p192456-i199160.timer: Timer elapsed.
Jul 08 06:58:26 systemd[2640]: run-p192456-i199160.timer: Changed waiting -> running
Jul 08 06:58:29 systemd[2640]: run-p192456-i199160.timer: Got notified about unit deactivation.
Jul 08 06:58:29 systemd[2640]: run-p192456-i199160.timer: Monotonic timer elapses in 33.544681s.
Jul 08 06:58:29 systemd[2640]: run-p192456-i199160.timer: Changed running -> waiting
Jul 08 06:59:48 systemd[2640]: run-p192456-i199160.timer: Timer elapsed.
1) The timer is armed at 06:57:25. timer_enter_waiting()
calculates following values:
base = M0 (current CLOCK_MONOTONIC)
usec_shift_clock(base, CLOCK_MONOTONIC, CLOCK_BOOTTIME_ALARM) = M0 (no delta yet)
v->next_elapse = M0 + 60s
The timer is on CLOCK_BOOTTIME_ALARM at M0 + 60s
2) System suspends. If the system ran for ~12s and was suspended for
~36s, at resume we'd get:
CLOCK_MONOTONIC = M0 + 12s
CLOCK_BOOTTIME = M0 + 12s + 36s = M0 + 48s
3) timer_time_change() fires and calls timer_enter_waiting(t, true)
Because time_change=true, v->next_elapse is not recalculated and
keeps the original M0 + 60s value. The timer then correctly computes
the remaining time as ~12.8 seconds:
Jul 08 06:58:13 ...: Monotonic timer elapses in 12.785674s.
4) Timer elapses at 06:58:26
Jul 08 06:58:26 ...: Timer elapsed.
timer_enter_running() sets last_trigger.monotonic to CLOCK_MONOTONIC
(which equals to M0 + 12s before suspend + 13s after suspend, thus
+ 25s)
5) Unit deactivates at 06:58:29
timer_trigger_notify() calls timer_enter_waiting(t, false) -
time_change=false; that means that this time v->next_elapse gets
recalculated:
base = inactive_exit_timestamp.monotonic = M0 (i.e. when the timer was originally armed)
usec_shift_clock(M0, CLOCK_MONOTONIC, CLOCK_BOOTTIME_ALARM)
a = now(CLOCK_MONOTONIC) = M0 + 28s
b = now(CLOCK_BOOTTIME_ALARM) = M0 + 64s
result = b - (a - M0) = (M0 + 64s) - 28s = M0 + 36s => the time spent in suspend
Hence v->next_elapse = (M0 + 36s) + 60s = M0 + 96s
This then skips disabling of one-shot timers in the following check,
because the expression
v->next_elapse < triple_timestamp_by_clock(&ts, TIMER_MONOTONIC_CLOCK(t))
is false, because the v->next_elapse (M0 + 96s) is not less than the
current time (M0 + 64s), so the timer is re-armed again:
Jul 08 06:58:29 ...: Monotonic timer elapses in 33.544681s
Let's mitigate this by skipping the next elapse timestamp recalculations
for one-shot timers for which we've already calculated the value in this
activation cycle.
Resolves: #42929
This commit is contained in:
committed by
Luca Boccassi
parent
2e1c8bc3d7
commit
5f2d15abf4
+16
-9
@@ -498,16 +498,23 @@ static void timer_enter_waiting(Timer *t, bool time_change) {
|
||||
assert_not_reached();
|
||||
}
|
||||
|
||||
if (!time_change)
|
||||
v->next_elapse = usec_add(usec_shift_clock(base, CLOCK_MONOTONIC, TIMER_MONOTONIC_CLOCK(t)), v->value);
|
||||
if (!time_change) {
|
||||
bool is_oneshot = IN_SET(v->base, TIMER_ACTIVE, TIMER_BOOT, TIMER_STARTUP);
|
||||
|
||||
if (dual_timestamp_is_set(&t->last_trigger) &&
|
||||
!time_change &&
|
||||
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;
|
||||
/* Skip recalculating the next elapse timestamp for one-shot timers for which
|
||||
* we've already calculated the value in this activation cycle. */
|
||||
if (!(v->next_elapse > 0 &&
|
||||
t->last_trigger.monotonic > base &&
|
||||
is_oneshot))
|
||||
v->next_elapse = usec_add(usec_shift_clock(base, CLOCK_MONOTONIC, TIMER_MONOTONIC_CLOCK(t)), v->value);
|
||||
|
||||
if (dual_timestamp_is_set(&t->last_trigger) &&
|
||||
v->next_elapse < triple_timestamp_by_clock(&ts, TIMER_MONOTONIC_CLOCK(t)) &&
|
||||
is_oneshot) {
|
||||
/* This is a one time trigger, disable it now. */
|
||||
v->disabled = true;
|
||||
continue;
|
||||
}
|
||||
}
|
||||
|
||||
if (!found_monotonic)
|
||||
|
||||
Reference in New Issue
Block a user