From 955b1f5be8582c7703e69b3f9184fb357397c7e1 Mon Sep 17 00:00:00 2001 From: Marian Buschsieweke Date: Wed, 18 Nov 2020 20:43:36 +0100 Subject: [PATCH 1/6] tests/event_threads: spice up test a bit By posting a higher priority event from the event handlers prior to printing, we can verify that an event handler is indeed preempted by a higher priority event. --- tests/event_threads/main.c | 4 ++-- tests/event_threads/tests/01-run.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/event_threads/main.c b/tests/event_threads/main.c index 88920d1c55..9b0b2f60f2 100644 --- a/tests/event_threads/main.c +++ b/tests/event_threads/main.c @@ -34,6 +34,7 @@ static event_t event_high = { .handler=_handler_high }; static void _handler_medium(event_t *event) { (void)event; + event_post(EVENT_PRIO_HIGHEST, &event_high); puts("medium"); } @@ -41,6 +42,7 @@ static event_t event_medium = { .handler=_handler_medium }; static void _handler_low(event_t *event) { (void)event; + event_post(EVENT_PRIO_MEDIUM, &event_medium); puts("low"); } @@ -49,8 +51,6 @@ static event_t event_low = { .handler=_handler_low }; int main(void) { event_post(EVENT_PRIO_LOWEST, &event_low); - event_post(EVENT_PRIO_MEDIUM, &event_medium); - event_post(EVENT_PRIO_HIGHEST, &event_high); puts("main done"); diff --git a/tests/event_threads/tests/01-run.py b/tests/event_threads/tests/01-run.py index 3a731093d0..d9632bec58 100755 --- a/tests/event_threads/tests/01-run.py +++ b/tests/event_threads/tests/01-run.py @@ -5,9 +5,9 @@ from testrunner import run def testfunc(child): - child.expect_exact('medium\r\n') - child.expect_exact('high\r\n') child.expect_exact('main done\r\n') + child.expect_exact('high\r\n') + child.expect_exact('medium\r\n') child.expect_exact('low\r\n') From c6211cc6c24fbde2968871845b42f00287101708 Mon Sep 17 00:00:00 2001 From: Marian Buschsieweke Date: Wed, 18 Nov 2020 20:48:16 +0100 Subject: [PATCH 2/6] sys/event: Allow shared thread for multiple queues Allow using `event_loop_multi()` to handle event queues of multiple priorities in an single thread. In the extreme case, all three event queues are handled by a single thread (thus saving two stacks). This comes for the price of increased worst case latency, as already running event handlers will no longer be preempted by higher priority events. With this, all three event queue priorities are always provided. Using modules, the old behavior of one thread per event queue can be restored for better worst case latency at the expense of additional thread size. --- sys/event/thread.c | 100 ++++++++++++++++++++----------------- sys/include/event.h | 6 +-- sys/include/event/thread.h | 95 +++++++++++++++++++++++++++++------ 3 files changed, 138 insertions(+), 63 deletions(-) diff --git a/sys/event/thread.c b/sys/event/thread.c index 999ff1371e..2c192620b2 100644 --- a/sys/event/thread.c +++ b/sys/event/thread.c @@ -21,21 +21,31 @@ * @} */ +#include "architecture.h" #include "thread.h" #include "event.h" #include "event/thread.h" -static void *_handler(void *event_queue) +struct event_queue_and_size { + event_queue_t *q; + size_t q_numof; +}; + +static void *_handler_thread(void *tagged_ptr) { - event_queue_claim(event_queue); - event_loop(event_queue); + event_queue_t *qs = ptrtag_ptr(tagged_ptr); + /* number of queues is encoded in lower pointer bits */ + size_t n = ptrtag_tag(tagged_ptr) + 1; + event_queues_claim(qs, n); + /* start event loop */ + event_loop_multi(qs, n); /* should be never reached */ return NULL; } -void event_thread_init(event_queue_t *queue, char *stack, size_t stack_size, - unsigned priority) +void event_thread_init_multi(event_queue_t *queues, size_t queues_numof, + char *stack, size_t stack_size, unsigned priority) { /* For the auto_init use case, this will be called before main gets * started. main might already use the queues, so they need to be @@ -43,9 +53,12 @@ void event_thread_init(event_queue_t *queue, char *stack, size_t stack_size, * * They will be claimed within the handler thread. */ - event_queue_init_detached(queue); + event_queues_init_detached(queues, queues_numof); - thread_create(stack, stack_size, priority, 0, _handler, queue, "event"); + void *tagged_ptr = ptrtag(queues, queues_numof - 1); + + thread_create(stack, stack_size, priority, 0, _handler_thread, tagged_ptr, + "event"); } #ifndef EVENT_THREAD_STACKSIZE_DEFAULT @@ -78,48 +91,43 @@ void event_thread_init(event_queue_t *queue, char *stack, size_t stack_size, #define EVENT_THREAD_LOWEST_PRIO (THREAD_PRIORITY_IDLE - 1) #endif -#ifdef MODULE_EVENT_THREAD_HIGHEST -event_queue_t event_queue_highest; -static char _evq_highest_stack[EVENT_THREAD_HIGHEST_STACKSIZE]; -#endif +/* rely on compiler / linker to garbage collect unused stacks */ +static char WORD_ALIGNED _evq_highest_stack[EVENT_THREAD_HIGHEST_STACKSIZE]; +static char WORD_ALIGNED _evq_medium_stack[EVENT_THREAD_MEDIUM_STACKSIZE]; +static char WORD_ALIGNED _evq_lowest_stack[EVENT_THREAD_LOWEST_STACKSIZE]; -#ifdef MODULE_EVENT_THREAD_MEDIUM -event_queue_t event_queue_medium; -static char _evq_medium_stack[EVENT_THREAD_MEDIUM_STACKSIZE]; -#endif - -#ifdef MODULE_EVENT_THREAD_LOWEST -event_queue_t event_queue_lowest; -static char _evq_lowest_stack[EVENT_THREAD_LOWEST_STACKSIZE]; -#endif - -typedef struct { - event_queue_t *queue; - char *stack; - size_t stack_size; - unsigned priority; -} event_threads_t; - -const event_threads_t _event_threads[] = { -#ifdef MODULE_EVENT_THREAD_HIGHEST - { &event_queue_highest, _evq_highest_stack, sizeof(_evq_highest_stack), - EVENT_THREAD_HIGHEST_PRIO }, -#endif -#ifdef MODULE_EVENT_THREAD_MEDIUM - { &event_queue_medium, _evq_medium_stack, sizeof(_evq_medium_stack), - EVENT_THREAD_MEDIUM_PRIO }, -#endif -#ifdef MODULE_EVENT_THREAD_LOWEST - { &event_queue_lowest, _evq_lowest_stack, sizeof(_evq_lowest_stack), - EVENT_THREAD_LOWEST_PRIO }, -#endif -}; +event_queue_t event_thread_queues[EVENT_QUEUE_PRIO_NUMOF]; void auto_init_event_thread(void) { - for (unsigned i = 0; i < ARRAY_SIZE(_event_threads); i++) { - event_thread_init(_event_threads[i].queue, - _event_threads[i].stack, _event_threads[i].stack_size, - _event_threads[i].priority); + if (IS_USED(MODULE_EVENT_THREAD_HIGHEST)) { + /* In order to allow highest priority events to preempt all others, + * high priority events must be run in their own thread. This thread + * can preempt than preempt the other event thread(s). */ + event_thread_init(EVENT_PRIO_HIGHEST, + _evq_highest_stack, sizeof(_evq_highest_stack), + EVENT_THREAD_HIGHEST_PRIO); } + if (IS_USED(MODULE_EVENT_THREAD_MEDIUM)) { + /* In order to allow medium priority events to preempt low priority + * events, we need to move the low priority events into their own + * thread. The always existing medium priority event thread can then + * preempt the lowest priority event thread. */ + event_thread_init(EVENT_PRIO_LOWEST, + _evq_lowest_stack, sizeof(_evq_lowest_stack), + EVENT_THREAD_LOWEST_PRIO); + } + + event_queue_t *qs = EVENT_PRIO_MEDIUM; + size_t qs_numof = 1; + if (!IS_USED(MODULE_EVENT_THREAD_HIGHEST)) { + qs = EVENT_PRIO_HIGHEST; + qs_numof = 2; + } + if (!IS_USED(MODULE_EVENT_THREAD_MEDIUM)) { + qs_numof++; + } + event_thread_init_multi(qs, qs_numof, + _evq_medium_stack, sizeof(_evq_medium_stack), + EVENT_THREAD_MEDIUM_PRIO); } diff --git a/sys/include/event.h b/sys/include/event.h index fb06d27f19..01ec3966fd 100644 --- a/sys/include/event.h +++ b/sys/include/event.h @@ -104,6 +104,7 @@ #include "irq.h" #include "thread.h" #include "thread_flags.h" +#include "ptrtag.h" #ifdef __cplusplus extern "C" { @@ -147,12 +148,11 @@ struct event { /** * @brief event queue structure */ -typedef struct { +typedef struct PTRTAG { clist_node_t event_list; /**< list of queued events */ - thread_t *waiter; /**< thread ownning event queue */ + thread_t *waiter; /**< thread owning event queue */ } event_queue_t; - /** * @brief Initialize an array of event queues * diff --git a/sys/include/event/thread.h b/sys/include/event/thread.h index 251ee92ce3..940553c576 100644 --- a/sys/include/event/thread.h +++ b/sys/include/event/thread.h @@ -12,6 +12,39 @@ * @ingroup sys_event * @brief Provides utility functions for event handler threads * + * Usage + * ===== + * + * By using the module `event_thread`, the event queues + * @ref EVENT_PRIO_HIGHEST, @ref EVENT_PRIO_MEDIUM, and + * @ref EVENT_PRIO_LOWEST are provided and declared in the header + * `event/thread.h`. With default settings, the `auto_init` module will + * automatically start one or more threads to handle these + * queues. + * + * By default, a single thread with priority `EVENT_THREAD_MEDIUM_PRIO` + * will handle all three event queues according to their priority. + * An already started event handler will not be preempted in this case by an + * incoming higher priority event. Still, lower priority event queues will only + * be worked on once the higher priority queues are empty. Hence, the worst case + * latency is increased by the worst case duration of any possible lower + * priority event in this configuration. + * + * By using module `event_thread_highest`, the highest priority queue gets its + * own thread. With this, events of the highest priority can preempt already + * running event handlers of medium and lowest priority. + * + * By using module `event_thread_medium`, the lowest priority events are handled + * in their own thread. With this, events of at least medium priority can + * preempt already running events of the lowest priority. + * + * By using both module `event_thread_highest` and `event_thread_medium`, each + * event queue gets its own thread. So higher priority events will always + * preempt events of lower priority in this case. + * + * Finally, the module `event_thread_lowest` is provided for backward + * compatibility and has no effect. + * * @{ * * @file @@ -31,6 +64,22 @@ extern "C" { #endif +/** + * @brief Convenience function for initializing an event queue thread + * handling multiple queues + * + * @param[in] queues array of the preallocated queue objects + * @param[in] queues_numof number of elements in @p queues + * @param[in] stack ptr to stack space + * @param[in] stack_size size of stack + * @param[in] priority priority to use + * + * @pre @p queues_numof is at most 4 + */ +void event_thread_init_multi(event_queue_t *queues, size_t queues_numof, + char *stack, size_t stack_size, + unsigned priority); + /** * @brief Convenience function for initializing an event queue thread * @@ -39,23 +88,41 @@ extern "C" { * @param[in] stack_size size of stack * @param[in] priority priority to use */ -void event_thread_init(event_queue_t *queue, char *stack, size_t stack_size, - unsigned priority); +static inline void event_thread_init(event_queue_t *queue, + char *stack, size_t stack_size, + unsigned priority) +{ + event_thread_init_multi(queue, 1, stack, stack_size, priority); +} -#ifdef MODULE_EVENT_THREAD_HIGHEST -extern event_queue_t event_queue_highest; -#define EVENT_PRIO_HIGHEST (&event_queue_highest) -#endif +/** + * @brief Event queue priorities + * + * @details The lower the numeric value, the higher the priority. The highest + * priority is 0, so that these priorities can be used as index to + * access arrays. + */ +enum { + EVENT_QUEUE_PRIO_HIGHEST, /**< Highest event queue priority */ + EVENT_QUEUE_PRIO_MEDIUM, /**< Medium event queue priority */ + EVENT_QUEUE_PRIO_LOWEST, /**< Lowest event queue priority */ + EVENT_QUEUE_PRIO_NUMOF /**< Number of event queue priorities */ +}; -#ifdef MODULE_EVENT_THREAD_MEDIUM -extern event_queue_t event_queue_medium; -#define EVENT_PRIO_MEDIUM (&event_queue_medium) -#endif +extern event_queue_t event_thread_queues[EVENT_QUEUE_PRIO_NUMOF]; -#ifdef MODULE_EVENT_THREAD_LOWEST -extern event_queue_t event_queue_lowest; -#define EVENT_PRIO_LOWEST (&event_queue_lowest) -#endif +/** + * @brief Pointer to the event queue handling highest priority events + */ +#define EVENT_PRIO_HIGHEST (&event_thread_queues[EVENT_QUEUE_PRIO_HIGHEST]) +/** + * @brief Pointer to the event queue handling medium priority events + */ +#define EVENT_PRIO_MEDIUM (&event_thread_queues[EVENT_QUEUE_PRIO_MEDIUM]) +/** + * @brief Pointer to the event queue handling lowest priority events + */ +#define EVENT_PRIO_LOWEST (&event_thread_queues[EVENT_QUEUE_PRIO_LOWEST]) #ifdef __cplusplus } From 10471a33b337c636f18f9680d1ffa6ee06107ba0 Mon Sep 17 00:00:00 2001 From: Marian Buschsieweke Date: Wed, 18 Nov 2020 20:55:11 +0100 Subject: [PATCH 3/6] tests: Add event_thread tests for shared thread --- tests/event_thread_shared/Makefile | 5 ++ tests/event_thread_shared/Makefile.ci | 3 ++ tests/event_thread_shared/main.c | 59 +++++++++++++++++++++++ tests/event_thread_shared/tests/01-run.py | 16 ++++++ 4 files changed, 83 insertions(+) create mode 100644 tests/event_thread_shared/Makefile create mode 100644 tests/event_thread_shared/Makefile.ci create mode 100644 tests/event_thread_shared/main.c create mode 100755 tests/event_thread_shared/tests/01-run.py diff --git a/tests/event_thread_shared/Makefile b/tests/event_thread_shared/Makefile new file mode 100644 index 0000000000..52eb82bb30 --- /dev/null +++ b/tests/event_thread_shared/Makefile @@ -0,0 +1,5 @@ +include ../Makefile.tests_common + +USEMODULE += event_thread + +include $(RIOTBASE)/Makefile.include diff --git a/tests/event_thread_shared/Makefile.ci b/tests/event_thread_shared/Makefile.ci new file mode 100644 index 0000000000..b9ff275375 --- /dev/null +++ b/tests/event_thread_shared/Makefile.ci @@ -0,0 +1,3 @@ +BOARD_INSUFFICIENT_MEMORY := \ + nucleo-l011k4 \ + # diff --git a/tests/event_thread_shared/main.c b/tests/event_thread_shared/main.c new file mode 100644 index 0000000000..380e342778 --- /dev/null +++ b/tests/event_thread_shared/main.c @@ -0,0 +1,59 @@ +/* + * Copyright (C) 2020 Kaspar Schleiser + * 2020 Freie Universität Berlin + * 2020 Inria + * + * This file is subject to the terms and conditions of the GNU Lesser + * General Public License v2.1. See the file LICENSE in the top level + * directory for more details. + */ + +/** + * @ingroup tests + * @{ + * + * @file + * @brief Event threads test application + * + * @author Kaspar Schleiser + * + * @} + */ + +#include + +#include "thread.h" +#include "event/thread.h" + +static void _handler_high(event_t *event) { + (void)event; + puts("high"); +} + +static event_t event_high = { .handler=_handler_high }; + +static void _handler_medium(event_t *event) { + (void)event; + event_post(EVENT_PRIO_HIGHEST, &event_high); + puts("medium"); +} + +static event_t event_medium = { .handler=_handler_medium }; + +static void _handler_low(event_t *event) { + (void)event; + event_post(EVENT_PRIO_MEDIUM, &event_medium); + event_post(EVENT_PRIO_HIGHEST, &event_high); + puts("low"); +} + +static event_t event_low = { .handler=_handler_low }; + +int main(void) +{ + event_post(EVENT_PRIO_LOWEST, &event_low); + + puts("main done"); + + return 0; +} diff --git a/tests/event_thread_shared/tests/01-run.py b/tests/event_thread_shared/tests/01-run.py new file mode 100755 index 0000000000..6ed171eb8f --- /dev/null +++ b/tests/event_thread_shared/tests/01-run.py @@ -0,0 +1,16 @@ +#!/usr/bin/env python3 + +import sys +from testrunner import run + + +def testfunc(child): + child.expect_exact('low\r\n') + child.expect_exact('high\r\n') + child.expect_exact('medium\r\n') + child.expect_exact('high\r\n') + child.expect_exact('main done\r\n') + + +if __name__ == "__main__": + sys.exit(run(testfunc)) From 06c87dfe97dd84b6e4e9be715e782aa56937e8da Mon Sep 17 00:00:00 2001 From: Marian Buschsieweke Date: Wed, 2 Dec 2020 09:47:06 +0100 Subject: [PATCH 4/6] makefiles: add mechanism to deprecate modules Add event_thread_lowest as first deprecated module --- makefiles/dependency_resolution.inc.mk | 8 ++++++++ makefiles/deprecated_modules.inc.mk | 3 +++ 2 files changed, 11 insertions(+) create mode 100644 makefiles/deprecated_modules.inc.mk diff --git a/makefiles/dependency_resolution.inc.mk b/makefiles/dependency_resolution.inc.mk index 827a1e17a6..7b15ea2ef1 100644 --- a/makefiles/dependency_resolution.inc.mk +++ b/makefiles/dependency_resolution.inc.mk @@ -38,4 +38,12 @@ else # Sort and de-duplicate used modules and default modules for readability USEMODULE := $(sort $(USEMODULE)) DEFAULT_MODULE := $(sort $(DEFAULT_MODULE)) + + # Warn about used deprecated modules + include $(RIOTMAKE)/deprecated_modules.inc.mk + DEPRECATED_MODULES_USED := $(sort $(filter $(DEPRECATED_MODULES),$(USEMODULE))) + ifneq (,$(DEPRECATED_MODULES_USED)) + $(shell $(COLOR_ECHO) "$(COLOR_RED)Deprecated modules are in use:$(COLOR_RESET)"\ + "$(DEPRECATED_MODULES_USED)" 1>&2) + endif endif diff --git a/makefiles/deprecated_modules.inc.mk b/makefiles/deprecated_modules.inc.mk new file mode 100644 index 0000000000..05d49d2083 --- /dev/null +++ b/makefiles/deprecated_modules.inc.mk @@ -0,0 +1,3 @@ +# Add deprecated modules here +# Keep this list ALPHABETICALLY SORTED!!!!111elven +DEPRECATED_MODULES += event_thread_lowest From ae5845c372f04fb6f731eb0205432873330dd9b7 Mon Sep 17 00:00:00 2001 From: Marian Buschsieweke Date: Wed, 2 Dec 2020 09:48:19 +0100 Subject: [PATCH 5/6] tests/event_threads: drop use of deprecated module --- tests/event_threads/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/event_threads/Makefile b/tests/event_threads/Makefile index 00f7eab2d7..cb838a7a48 100644 --- a/tests/event_threads/Makefile +++ b/tests/event_threads/Makefile @@ -1,5 +1,5 @@ include ../Makefile.tests_common -USEMODULE += event_thread_highest event_thread_medium event_thread_lowest +USEMODULE += event_thread_highest event_thread_medium include $(RIOTBASE)/Makefile.include From 78aec672ddfdf1c9327b81ec23aae7aaa22f7ac3 Mon Sep 17 00:00:00 2001 From: Marian Buschsieweke Date: Wed, 2 Dec 2020 17:46:52 +0100 Subject: [PATCH 6/6] tests/pkg_tinydtls_sock_async: fix use of internals event_queue_medium was never part of the public API, instead of using `&event_queue_medium` one should use `EVENT_PRIO_MEDIUM`. --- tests/pkg_tinydtls_sock_async/dtls-client.c | 4 ++-- tests/pkg_tinydtls_sock_async/dtls-server.c | 5 ++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/tests/pkg_tinydtls_sock_async/dtls-client.c b/tests/pkg_tinydtls_sock_async/dtls-client.c index 2cafecc18d..2aefc75856 100644 --- a/tests/pkg_tinydtls_sock_async/dtls-client.c +++ b/tests/pkg_tinydtls_sock_async/dtls-client.c @@ -171,7 +171,7 @@ static int client_send(char *addr_str, char *data) if (_sending) { puts("Already in the process of sending"); } - event_timeout_init(&_timeouter, &event_queue_medium, &_timeout.super); + event_timeout_init(&_timeouter, EVENT_PRIO_MEDIUM, &_timeout.super); /* get interface */ char* iface = ipv6_addr_split_iface(addr_str); if (iface) { @@ -208,7 +208,7 @@ static int client_send(char *addr_str, char *data) return -1; } - sock_dtls_event_init(&_dtls_sock, &event_queue_medium, _dtls_handler, + sock_dtls_event_init(&_dtls_sock, EVENT_PRIO_MEDIUM, _dtls_handler, data); res = credman_add(&credential); if (res < 0 && res != CREDMAN_EXIST) { diff --git a/tests/pkg_tinydtls_sock_async/dtls-server.c b/tests/pkg_tinydtls_sock_async/dtls-server.c index 13ac2bce8d..476235706d 100644 --- a/tests/pkg_tinydtls_sock_async/dtls-server.c +++ b/tests/pkg_tinydtls_sock_async/dtls-server.c @@ -166,8 +166,7 @@ static int start_server(void) } _active = true; - sock_dtls_event_init(&_dtls_sock, &event_queue_medium, _dtls_handler, - NULL); + sock_dtls_event_init(&_dtls_sock, EVENT_PRIO_MEDIUM, _dtls_handler, NULL); return 0; } @@ -179,7 +178,7 @@ static int stop_server(void) return 1; } - event_post(&event_queue_medium, &_close.super); + event_post(EVENT_PRIO_MEDIUM, &_close.super); return 0; }