From: Frantisek Sumsal Date: Tue, 7 Jul 2026 15:22:49 +0000 (+0200) Subject: sd-event: use CLOCK_BOOTTIME for rate limits X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=9ae50c78ccf7d36258a6779f34055248e9343efa;p=thirdparty%2Fsystemd.git sd-event: use CLOCK_BOOTTIME for rate limits When a rate limit is armed, for example via StartLimitInterval=, and the machine is suspended, the rate limit's "clock" is suspended as well, since the elapse checks use CLOCK_MONOTONIC. This then causes unexpected situations where a rate limit armed via StartLimitInterval=1h, followed by a 10 hour suspend, would elapse after 11 hours total. Let's avoid this by switching the rate limits to CLOCK_BOOTTIME, which works the same as CLOCK_MONOTONIC, but accounts for the time spent in suspend as well. There's one slight concern when it comes to upgrade path - the old "begin" value of the rate limit is stored as CLOCK_MONOTONIC, but after upgrading systemd and serializing/deserializing the state it will be suddenly compared against now(CLOCK_BOOTTIME), which might cause some rate limits to elapse "prematurely". But this is just a one-time thing, after which the rate limit timers should re-assess themselves. Resolves: #42912 --- diff --git a/src/basic/ratelimit.c b/src/basic/ratelimit.c index 48ad08f3b67..b4ef5470b05 100644 --- a/src/basic/ratelimit.c +++ b/src/basic/ratelimit.c @@ -14,7 +14,7 @@ bool ratelimit_below(RateLimit *rl) { if (!ratelimit_configured(rl)) return true; - ts = now(CLOCK_MONOTONIC); + ts = now(CLOCK_BOOTTIME); if (rl->begin <= 0 || usec_sub_unsigned(ts, rl->begin) > rl->interval) { @@ -54,5 +54,5 @@ usec_t ratelimit_left(const RateLimit *rl) { if (rl->begin == 0) return 0; - return usec_sub_unsigned(ratelimit_end(rl), now(CLOCK_MONOTONIC)); + return usec_sub_unsigned(ratelimit_end(rl), now(CLOCK_BOOTTIME)); } diff --git a/src/core/manager.c b/src/core/manager.c index ae44f87b2a8..27ca84c4e9e 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -1517,7 +1517,7 @@ static int manager_ratelimit_check_and_queue(Unit *u) { r = sd_event_add_time( u->manager->event, &u->auto_start_stop_event_source, - CLOCK_MONOTONIC, + CLOCK_BOOTTIME, ratelimit_end(&u->auto_start_stop_ratelimit), 0, manager_ratelimit_requeue, diff --git a/src/libsystemd/sd-event/sd-event.c b/src/libsystemd/sd-event/sd-event.c index 4935ae1b4aa..590ac06b8e3 100644 --- a/src/libsystemd/sd-event/sd-event.c +++ b/src/libsystemd/sd-event/sd-event.c @@ -900,7 +900,7 @@ static void event_source_time_prioq_reshuffle(sd_event_source *s) { * properly again. */ if (s->ratelimited) - d = &s->event->monotonic; + d = &s->event->boottime; else if (EVENT_SOURCE_IS_TIME(s->type)) assert_se(d = event_get_clock_data(s->event, s->type)); else @@ -950,7 +950,7 @@ static void source_disconnect(sd_event_source *s) { case SOURCE_TIME_BOOTTIME_ALARM: /* Only remove this event source from the time event source here if it is not ratelimited. If * it is ratelimited, we'll remove it below, separately. Why? Because the clock used might - * differ: ratelimiting always uses CLOCK_MONOTONIC, but timer events might use any clock */ + * differ: ratelimiting always uses CLOCK_BOOTTIME, but timer events might use any clock */ if (!s->ratelimited) { struct clock_data *d; @@ -1070,7 +1070,7 @@ static void source_disconnect(sd_event_source *s) { prioq_remove(s->event->prepare, s, &s->prepare_index); if (s->ratelimited) - event_source_time_prioq_remove(s, &s->event->monotonic); + event_source_time_prioq_remove(s, &s->event->boottime); event = TAKE_PTR(s->event); LIST_REMOVE(sources, event->sources, s); @@ -3397,32 +3397,32 @@ static int event_source_enter_ratelimited(sd_event_source *s) { assert(s); - /* When an event source becomes ratelimited, we place it in the CLOCK_MONOTONIC priority queue, with + /* When an event source becomes ratelimited, we place it in the CLOCK_BOOTTIME priority queue, with * the end of the rate limit time window, much as if it was a timer event source. */ if (s->ratelimited) return 0; /* Already ratelimited, this is a NOP hence */ - /* Make sure we can install a CLOCK_MONOTONIC event further down. */ - r = setup_clock_data(s->event, &s->event->monotonic, CLOCK_MONOTONIC); + /* Make sure we can install a CLOCK_BOOTTIME event further down. */ + r = setup_clock_data(s->event, &s->event->boottime, CLOCK_BOOTTIME); if (r < 0) return r; /* Timer event sources are already using the earliest/latest queues for the timer scheduling. Let's * first remove them from the prioq appropriate for their own clock, so that we can use the prioq - * fields of the event source then for adding it to the CLOCK_MONOTONIC prioq instead. */ + * fields of the event source then for adding it to the CLOCK_BOOTTIME prioq instead. */ if (EVENT_SOURCE_IS_TIME(s->type)) event_source_time_prioq_remove(s, event_get_clock_data(s->event, s->type)); - /* Now, let's add the event source to the monotonic clock instead */ - r = event_source_time_prioq_put(s, &s->event->monotonic); + /* Now, let's add the event source to the boottime clock instead */ + r = event_source_time_prioq_put(s, &s->event->boottime); if (r < 0) goto fail; /* And let's take the event source officially offline */ r = event_source_offline(s, s->enabled, /* ratelimited= */ true); if (r < 0) { - event_source_time_prioq_remove(s, &s->event->monotonic); + event_source_time_prioq_remove(s, &s->event->boottime); goto fail; } @@ -3448,8 +3448,8 @@ static int event_source_leave_ratelimit(sd_event_source *s, bool run_callback) { if (!s->ratelimited) return 0; - /* Let's take the event source out of the monotonic prioq first. */ - event_source_time_prioq_remove(s, &s->event->monotonic); + /* Let's take the event source out of the boottime prioq first. */ + event_source_time_prioq_remove(s, &s->event->boottime); /* Let's then add the event source to its native clock prioq again — if this is a timer event source */ if (EVENT_SOURCE_IS_TIME(s->type)) { @@ -3501,7 +3501,7 @@ static int event_source_leave_ratelimit(sd_event_source *s, bool run_callback) { fail: /* Do something somewhat reasonable when we cannot move an event sources out of ratelimited mode: * simply put it back in it, maybe we can then process it more successfully next iteration. */ - assert_se(event_source_time_prioq_put(s, &s->event->monotonic) >= 0); + assert_se(event_source_time_prioq_put(s, &s->event->boottime) >= 0); return r; } @@ -4781,6 +4781,7 @@ static int process_epoll(sd_event *e, usec_t timeout, int64_t threshold, int64_t } _public_ int sd_event_wait(sd_event *e, uint64_t timeout) { + bool ratelimit_expired = false; int r; assert_return(e, -EINVAL); @@ -4845,6 +4846,8 @@ _public_ int sd_event_wait(sd_event *e, uint64_t timeout) { r = process_timer(e, e->timestamp.boottime, &e->boottime); if (r < 0) goto finish; + else if (r == 1) + ratelimit_expired = true; r = process_timer(e, e->timestamp.realtime, &e->realtime_alarm); if (r < 0) @@ -4857,14 +4860,19 @@ _public_ int sd_event_wait(sd_event *e, uint64_t timeout) { r = process_timer(e, e->timestamp.monotonic, &e->monotonic); if (r < 0) goto finish; - else if (r == 1) { - /* Ratelimit expiry callback was called. Let's postpone processing pending sources and - * put loop in the initial state in order to evaluate (in the next iteration) also sources - * there were potentially re-enabled by the callback. + + if (ratelimit_expired) { + /* Ratelimit expiry callback was called. Let's postpone processing pending sources and put + * the loop in the initial state in order to evaluate (in the next iteration) also sources + * that were potentially re-enabled by the callback. + * + * Wondering why we treat only the CLOCK_BOOTTIME invocation of process_timer() differently? + * Once event source is ratelimited we essentially transform it into a CLOCK_BOOTTIME timer + * hence ratelimit expiry callback is never called for any other timer type. * - * Wondering why we treat only this invocation of process_timer() differently? Once event - * source is ratelimited we essentially transform it into CLOCK_MONOTONIC timer hence - * ratelimit expiry callback is never called for any other timer type. */ + * Note that we don't short-circuit earlier because all timer prioqs must be fully processed + * as their timerfds may have already been flushed by process_epoll(), so skipping them would + * leave those sources unprocessed and the timerfds un-rearmed. */ r = 0; goto finish; } diff --git a/src/nsresourced/nsresourced-manager.c b/src/nsresourced/nsresourced-manager.c index 7d5f27bbda8..4e671de8331 100644 --- a/src/nsresourced/nsresourced-manager.c +++ b/src/nsresourced/nsresourced-manager.c @@ -291,7 +291,7 @@ static int start_workers(Manager *m, bool explicit_request) { r = sd_event_add_time( m->event, &m->deferred_start_worker_event_source, - CLOCK_MONOTONIC, + CLOCK_BOOTTIME, ratelimit_end(&m->worker_ratelimit), /* accuracy= */ 0, on_deferred_start_worker, diff --git a/src/userdb/userdbd-manager.c b/src/userdb/userdbd-manager.c index cf9f2f5c6b8..c51ae513f4e 100644 --- a/src/userdb/userdbd-manager.c +++ b/src/userdb/userdbd-manager.c @@ -259,7 +259,7 @@ static int start_workers(Manager *m, bool explicit_request) { r = sd_event_add_time( m->event, &m->deferred_start_worker_event_source, - CLOCK_MONOTONIC, + CLOCK_BOOTTIME, ratelimit_end(&m->worker_ratelimit), /* accuracy= */ 0, on_deferred_start_worker,