| From 81107b8419c39f726fd2805517a5b9faab204e59 Mon Sep 17 00:00:00 2001 |
| From: Lennart Poettering <lennart@poettering.net> |
| Date: Tue, 8 Jun 2021 00:07:51 -0700 |
| Subject: [PATCH] sd-event: change ordering of pending/ratelimited events |
| |
| Instead of ordering non-pending before pending we should order |
| "non-pending OR ratelimited" before "pending AND not-ratelimited". |
| This fixes a bug where ratelimited events were ordered at the end of the |
| priority queue and could be stuck there for an indeterminate amount of |
| time. |
| --- |
| src/libsystemd/sd-event/sd-event.c | 45 ++++++++++++++---------------- |
| 1 file changed, 21 insertions(+), 24 deletions(-) |
| |
| diff --git a/src/libsystemd/sd-event/sd-event.c b/src/libsystemd/sd-event/sd-event.c |
| index 611af45293ac..489878a3e967 100644 |
| --- a/src/libsystemd/sd-event/sd-event.c |
| +++ b/src/libsystemd/sd-event/sd-event.c |
| @@ -238,25 +238,6 @@ static usec_t time_event_source_next(const sd_event_source *s) { |
| return USEC_INFINITY; |
| } |
| |
| -static int earliest_time_prioq_compare(const void *a, const void *b) { |
| - const sd_event_source *x = a, *y = b; |
| - |
| - /* Enabled ones first */ |
| - if (x->enabled != SD_EVENT_OFF && y->enabled == SD_EVENT_OFF) |
| - return -1; |
| - if (x->enabled == SD_EVENT_OFF && y->enabled != SD_EVENT_OFF) |
| - return 1; |
| - |
| - /* Move the pending ones to the end */ |
| - if (!x->pending && y->pending) |
| - return -1; |
| - if (x->pending && !y->pending) |
| - return 1; |
| - |
| - /* Order by time */ |
| - return CMP(time_event_source_next(x), time_event_source_next(y)); |
| -} |
| - |
| static usec_t time_event_source_latest(const sd_event_source *s) { |
| assert(s); |
| |
| @@ -275,7 +256,15 @@ static usec_t time_event_source_latest(const sd_event_source *s) { |
| return USEC_INFINITY; |
| } |
| |
| -static int latest_time_prioq_compare(const void *a, const void *b) { |
| +static bool event_source_timer_candidate(const sd_event_source *s) { |
| + assert(s); |
| + |
| + /* Returns true for event sources that either are not pending yet (i.e. where it's worth to mark them pending) |
| + * or which are currently ratelimited (i.e. where it's worth leaving the ratelimited state) */ |
| + return !s->pending || s->ratelimited; |
| +} |
| + |
| +static int time_prioq_compare(const void *a, const void *b, usec_t (*time_func)(const sd_event_source *s)) { |
| const sd_event_source *x = a, *y = b; |
| |
| /* Enabled ones first */ |
| @@ -284,14 +273,22 @@ static int latest_time_prioq_compare(const void *a, const void *b) { |
| if (x->enabled == SD_EVENT_OFF && y->enabled != SD_EVENT_OFF) |
| return 1; |
| |
| - /* Move the pending ones to the end */ |
| - if (!x->pending && y->pending) |
| + /* Order "non-pending OR ratelimited" before "pending AND not-ratelimited" */ |
| + if (event_source_timer_candidate(x) && !event_source_timer_candidate(y)) |
| return -1; |
| - if (x->pending && !y->pending) |
| + if (!event_source_timer_candidate(x) && event_source_timer_candidate(y)) |
| return 1; |
| |
| /* Order by time */ |
| - return CMP(time_event_source_latest(x), time_event_source_latest(y)); |
| + return CMP(time_func(x), time_func(y)); |
| +} |
| + |
| +static int earliest_time_prioq_compare(const void *a, const void *b) { |
| + return time_prioq_compare(a, b, time_event_source_next); |
| +} |
| + |
| +static int latest_time_prioq_compare(const void *a, const void *b) { |
| + return time_prioq_compare(a, b, time_event_source_latest); |
| } |
| |
| static int exit_prioq_compare(const void *a, const void *b) { |
| @@ -778,14 +775,15 @@ static void event_source_time_prioq_reshuffle(sd_event_source *s) { |
| assert(s); |
| |
| /* Called whenever the event source's timer ordering properties changed, i.e. time, accuracy, |
| - * pending, enable state. Makes sure the two prioq's are ordered properly again. */ |
| + * pending, enable state, and ratelimiting state. Makes sure the two prioq's are ordered |
| + * properly again. */ |
| |
| if (s->ratelimited) |
| d = &s->event->monotonic; |
| - else { |
| - assert(EVENT_SOURCE_IS_TIME(s->type)); |
| + else if (EVENT_SOURCE_IS_TIME(s->type)) |
| assert_se(d = event_get_clock_data(s->event, s->type)); |
| - } |
| + else |
| + return; /* no-op for an event source which is neither a timer nor ratelimited. */ |
| |
| prioq_reshuffle(d->earliest, s, &s->earliest_index); |
| prioq_reshuffle(d->latest, s, &s->latest_index); |
| @@ -2376,14 +2374,6 @@ static int event_source_offline( |
| source_io_unregister(s); |
| break; |
| |
| - case SOURCE_TIME_REALTIME: |
| - case SOURCE_TIME_BOOTTIME: |
| - case SOURCE_TIME_MONOTONIC: |
| - case SOURCE_TIME_REALTIME_ALARM: |
| - case SOURCE_TIME_BOOTTIME_ALARM: |
| - event_source_time_prioq_reshuffle(s); |
| - break; |
| - |
| case SOURCE_SIGNAL: |
| event_gc_signal_data(s->event, &s->priority, s->signal.sig); |
| break; |
| @@ -2404,6 +2394,11 @@ static int event_source_offline( |
| prioq_reshuffle(s->event->exit, s, &s->exit.prioq_index); |
| break; |
| |
| + case SOURCE_TIME_REALTIME: |
| + case SOURCE_TIME_BOOTTIME: |
| + case SOURCE_TIME_MONOTONIC: |
| + case SOURCE_TIME_REALTIME_ALARM: |
| + case SOURCE_TIME_BOOTTIME_ALARM: |
| case SOURCE_DEFER: |
| case SOURCE_POST: |
| case SOURCE_INOTIFY: |
| @@ -2413,6 +2408,9 @@ static int event_source_offline( |
| assert_not_reached("Wut? I shouldn't exist."); |
| } |
| |
| + /* Always reshuffle time prioq, as the ratelimited flag may be changed. */ |
| + event_source_time_prioq_reshuffle(s); |
| + |
| return 1; |
| } |
| |
| @@ -2502,22 +2500,11 @@ static int event_source_online( |
| s->ratelimited = ratelimited; |
| |
| /* Non-failing operations below */ |
| - switch (s->type) { |
| - case SOURCE_TIME_REALTIME: |
| - case SOURCE_TIME_BOOTTIME: |
| - case SOURCE_TIME_MONOTONIC: |
| - case SOURCE_TIME_REALTIME_ALARM: |
| - case SOURCE_TIME_BOOTTIME_ALARM: |
| - event_source_time_prioq_reshuffle(s); |
| - break; |
| - |
| - case SOURCE_EXIT: |
| + if (s->type == SOURCE_EXIT) |
| prioq_reshuffle(s->event->exit, s, &s->exit.prioq_index); |
| - break; |
| |
| - default: |
| - break; |
| - } |
| + /* Always reshuffle time prioq, as the ratelimited flag may be changed. */ |
| + event_source_time_prioq_reshuffle(s); |
| |
| return 1; |
| } |