diff options
author | ulfvonbelow <strilen@tilde.club> | 2023-01-28 16:08:45 -0600 |
---|---|---|
committer | Christian Grothoff <christian@grothoff.org> | 2023-02-06 16:11:06 +0100 |
commit | 763335e08dd7e3feff6184879471198f5852821d (patch) | |
tree | bb834b589b5722cde1f6d12afaa60d5e7897e2af | |
parent | 3f6f1fb44b6b6aa8f70e71d7a9c73ba623f8c9f7 (diff) | |
download | gnunet-763335e08dd7e3feff6184879471198f5852821d.tar.gz gnunet-763335e08dd7e3feff6184879471198f5852821d.zip |
UTIL: use dedicated marker in ready queue.
This inserts a dedicated dummy marker task at the end of the ready queue at
the start of a pass. Because this marker task isn't visible to users of the
scheduler, it can't be canceled while the pass is being run. Additionally,
switching which ready queue is being run partway through by scheduling a
higher-priority task to immediately run also places this dummy marker. This
resolves both erroneous cases by which a pass can accidentally run an
unbounded number of tasks.
This also modifies GNUNET_SCHEDULER_get_load to not be misled by this extra
dummy task, and adds the now-passing test cases to the test suite.
Signed-off-by: Christian Grothoff <christian@grothoff.org>
-rw-r--r-- | src/util/Makefile.am | 4 | ||||
-rw-r--r-- | src/util/scheduler.c | 46 |
2 files changed, 38 insertions, 12 deletions
diff --git a/src/util/Makefile.am b/src/util/Makefile.am index 81b8a93b7..431bd7d0d 100644 --- a/src/util/Makefile.am +++ b/src/util/Makefile.am | |||
@@ -174,8 +174,6 @@ endif | |||
174 | noinst_PROGRAMS = \ | 174 | noinst_PROGRAMS = \ |
175 | gnunet-config-diff \ | 175 | gnunet-config-diff \ |
176 | test_common_logging_dummy \ | 176 | test_common_logging_dummy \ |
177 | test_scheduler_hogging_cancel \ | ||
178 | test_scheduler_hogging_priority \ | ||
179 | gnunet-crypto-tvg | 177 | gnunet-crypto-tvg |
180 | 178 | ||
181 | if ENABLE_TEST_RUN | 179 | if ENABLE_TEST_RUN |
@@ -323,6 +321,8 @@ check_PROGRAMS = \ | |||
323 | test_resolver_api.nc \ | 321 | test_resolver_api.nc \ |
324 | test_scheduler \ | 322 | test_scheduler \ |
325 | test_scheduler_delay \ | 323 | test_scheduler_delay \ |
324 | test_scheduler_hogging_cancel \ | ||
325 | test_scheduler_hogging_priority \ | ||
326 | test_service \ | 326 | test_service \ |
327 | test_strings \ | 327 | test_strings \ |
328 | test_strings_to_data \ | 328 | test_strings_to_data \ |
diff --git a/src/util/scheduler.c b/src/util/scheduler.c index f3b220c4a..b57e2db66 100644 --- a/src/util/scheduler.c +++ b/src/util/scheduler.c | |||
@@ -248,6 +248,13 @@ struct GNUNET_SCHEDULER_Task | |||
248 | struct GNUNET_AsyncScopeSave scope; | 248 | struct GNUNET_AsyncScopeSave scope; |
249 | }; | 249 | }; |
250 | 250 | ||
251 | /** | ||
252 | * Placed at the end of a ready queue to indicate where a scheduler run pass | ||
253 | * ends. The next, prev, in_ready_list and priority fields are the only ones | ||
254 | * that should be used. | ||
255 | */ | ||
256 | static struct GNUNET_SCHEDULER_Task pass_end_marker; | ||
257 | |||
251 | 258 | ||
252 | /** | 259 | /** |
253 | * A struct representing an event the select driver is waiting for | 260 | * A struct representing an event the select driver is waiting for |
@@ -503,6 +510,27 @@ get_timeout () | |||
503 | return timeout; | 510 | return timeout; |
504 | } | 511 | } |
505 | 512 | ||
513 | static void remove_pass_end_marker () | ||
514 | { | ||
515 | if (pass_end_marker.in_ready_list) | ||
516 | { | ||
517 | GNUNET_CONTAINER_DLL_remove (ready_head[pass_end_marker.priority], | ||
518 | ready_tail[pass_end_marker.priority], | ||
519 | &pass_end_marker); | ||
520 | pass_end_marker.in_ready_list = GNUNET_NO; | ||
521 | } | ||
522 | } | ||
523 | |||
524 | static void set_work_priority (enum GNUNET_SCHEDULER_Priority p) | ||
525 | { | ||
526 | remove_pass_end_marker (); | ||
527 | GNUNET_CONTAINER_DLL_insert_tail (ready_head[p], | ||
528 | ready_tail[p], | ||
529 | &pass_end_marker); | ||
530 | pass_end_marker.priority = p; | ||
531 | pass_end_marker.in_ready_list = GNUNET_YES; | ||
532 | work_priority = p; | ||
533 | } | ||
506 | 534 | ||
507 | /** | 535 | /** |
508 | * Put a task that is ready for execution into the ready queue. | 536 | * Put a task that is ready for execution into the ready queue. |
@@ -518,7 +546,7 @@ queue_ready_task (struct GNUNET_SCHEDULER_Task *task) | |||
518 | ready_tail[p], | 546 | ready_tail[p], |
519 | task); | 547 | task); |
520 | if (p > work_priority) | 548 | if (p > work_priority) |
521 | work_priority = p; | 549 | set_work_priority (p); |
522 | task->in_ready_list = GNUNET_YES; | 550 | task->in_ready_list = GNUNET_YES; |
523 | ready_count++; | 551 | ready_count++; |
524 | } | 552 | } |
@@ -752,6 +780,9 @@ GNUNET_SCHEDULER_get_load (enum GNUNET_SCHEDULER_Priority p) | |||
752 | NULL != pos; | 780 | NULL != pos; |
753 | pos = pos->next) | 781 | pos = pos->next) |
754 | ret++; | 782 | ret++; |
783 | if (pass_end_marker.in_ready_list && pass_end_marker.priority == p) | ||
784 | // Don't count the dummy marker | ||
785 | ret--; | ||
755 | return ret; | 786 | return ret; |
756 | } | 787 | } |
757 | 788 | ||
@@ -2050,8 +2081,9 @@ GNUNET_SCHEDULER_do_work (struct GNUNET_SCHEDULER_Handle *sh) | |||
2050 | 2081 | ||
2051 | /* process all *existing* tasks at this priority | 2082 | /* process all *existing* tasks at this priority |
2052 | level, then yield */ | 2083 | level, then yield */ |
2053 | last = ready_tail[work_priority]; | 2084 | set_work_priority (work_priority); |
2054 | while (NULL != (pos = ready_head[work_priority])) | 2085 | while (NULL != (pos = ready_head[work_priority]) |
2086 | && pos != &pass_end_marker) | ||
2055 | { | 2087 | { |
2056 | GNUNET_CONTAINER_DLL_remove (ready_head[work_priority], | 2088 | GNUNET_CONTAINER_DLL_remove (ready_head[work_priority], |
2057 | ready_tail[work_priority], | 2089 | ready_tail[work_priority], |
@@ -2121,14 +2153,8 @@ GNUNET_SCHEDULER_do_work (struct GNUNET_SCHEDULER_Handle *sh) | |||
2121 | active_task = NULL; | 2153 | active_task = NULL; |
2122 | dump_backtrace (pos); | 2154 | dump_backtrace (pos); |
2123 | destroy_task (pos); | 2155 | destroy_task (pos); |
2124 | /* pointer 'pos' was free'd, but we can still safely check for | ||
2125 | pointer equality still. */ | ||
2126 | if (pos == last) | ||
2127 | break; /* All tasks that _were_ ready when we started were | ||
2128 | executed. New tasks may have been added in the | ||
2129 | meantime, but we should check with the OS to | ||
2130 | be sure no higher-priority actions are pending! */ | ||
2131 | } | 2156 | } |
2157 | remove_pass_end_marker (); | ||
2132 | } | 2158 | } |
2133 | shutdown_if_no_lifeness (); | 2159 | shutdown_if_no_lifeness (); |
2134 | if (0 == ready_count) | 2160 | if (0 == ready_count) |