From c8e1c44c0ebb6739f046fe355ba3ade05d581128 Mon Sep 17 00:00:00 2001 From: Johny Mattsson Date: Mon, 26 Jul 2021 15:36:57 +1000 Subject: [PATCH] Sort out task posting behaviour. --- components/driver_console/console.c | 2 +- components/modules/gpio.c | 2 +- components/modules/pulsecnt.c | 2 +- components/modules/touch.c | 2 +- components/task/include/task/task.h | 17 +++++++++++++++++ components/task/task.c | 23 ++++++++++++++++++++--- 6 files changed, 41 insertions(+), 7 deletions(-) diff --git a/components/driver_console/console.c b/components/driver_console/console.c index 2f656c5d..9125eaf0 100644 --- a/components/driver_console/console.c +++ b/components/driver_console/console.c @@ -131,7 +131,7 @@ static void uart0_rx_intr_handler (void *arg) status = READ_PERI_REG(UART_INT_ST_REG(CONSOLE_UART)); } if (received && input_task) - task_post_low (input_task, false); + task_post_isr_low (input_task, false); } diff --git a/components/modules/gpio.c b/components/modules/gpio.c index 05229dab..3c1bf7d1 100644 --- a/components/modules/gpio.c +++ b/components/modules/gpio.c @@ -62,7 +62,7 @@ static void single_pin_isr (void *p) { gpio_num_t gpio_num = (gpio_num_t)p; gpio_intr_disable (gpio_num); - task_post_low (cb_task, (gpio_num) | (gpio_get_level (gpio_num) << 8)); + task_post_isr_low (cb_task, (gpio_num) | (gpio_get_level (gpio_num) << 8)); } diff --git a/components/modules/pulsecnt.c b/components/modules/pulsecnt.c index bcaf5f90..df031df6 100644 --- a/components/modules/pulsecnt.c +++ b/components/modules/pulsecnt.c @@ -96,7 +96,7 @@ static void IRAM_ATTR pulsecnt_intr_handler(void *arg) // on lua_open we set pulsecnt_task_id as a method which gets called // by Lua after task_post_high with reference to this self object and then we can steal the // callback_ref and then it gets called by lua_call where we get to add our args - task_post_high(pulsecnt_task_id, (evt.status << 8) | evt.unit ); + task_post_isr_high(pulsecnt_task_id, (evt.status << 8) | evt.unit ); } } } diff --git a/components/modules/touch.c b/components/modules/touch.c index 9e8afdbe..f429fa0e 100644 --- a/components/modules/touch.c +++ b/components/modules/touch.c @@ -91,7 +91,7 @@ static void IRAM_ATTR touch_intr_handler(void *arg) // on lua_open we set touch_task_id as a method which gets called // by Lua after task_post_high with reference to this self object and then we can steal the // callback_ref and then it gets called by lua_call where we get to add our args - task_post_high(touch_task_id, pad_intr ); + task_post_isr_high(touch_task_id, pad_intr ); } diff --git a/components/task/include/task/task.h b/components/task/include/task/task.h index b7763516..55013c36 100644 --- a/components/task/include/task/task.h +++ b/components/task/include/task/task.h @@ -23,10 +23,27 @@ typedef intptr_t task_param_t; */ bool task_post(task_prio_t priority, task_handle_t handle, task_param_t param); +/* When posting NodeMCU tasks from an ISR, this version MUST be used, + * and vice versa. + * Doing otherwise breaks assumptions made by the FreeRTOS kernel in terms of + * concurrency safety, and may lead to obscure and hard-to-pinpoint bugs. + * While a specific hardware port may be implemented in a manner which makes + * it concurrency safe to call FreeRTOS xxxFromISR functions from any context, + * doing so would still lead to scheduling oddities as the xxxFromISR + * functions do not perform task switching whereas the non-FromISR versions + * do. In practical terms, that means that even if a higher priority FreeRTOS + * task was unblocked, it would not get scheduled at that point. + */ +bool task_post_isr(task_prio_t priority, task_handle_t handle, task_param_t param); + #define task_post_low(handle,param) task_post(TASK_PRIORITY_LOW, handle, param) #define task_post_medium(handle,param) task_post(TASK_PRIORITY_MEDIUM, handle, param) #define task_post_high(handle,param) task_post(TASK_PRIORITY_HIGH, handle, param) +#define task_post_isr_low(handle,param) task_post_isr(TASK_PRIORITY_LOW, handle, param) +#define task_post_isr_medium(handle,param) task_post_isr(TASK_PRIORITY_MEDIUM, handle, param) +#define task_post_isr_high(handle,param) task_post_isr(TASK_PRIORITY_HIGH, handle, param) + typedef void (*task_callback_t)(task_param_t param, task_prio_t prio); task_handle_t task_get_id(task_callback_t t); diff --git a/components/task/task.c b/components/task/task.c index 9fa69e6f..24084912 100644 --- a/components/task/task.c +++ b/components/task/task.c @@ -55,16 +55,33 @@ task_handle_t task_get_id(task_callback_t t) { } -bool IRAM_ATTR task_post (task_prio_t priority, task_handle_t handle, task_param_t param) +bool IRAM_ATTR task_post(task_prio_t priority, task_handle_t handle, task_param_t param) { if (priority >= TASK_PRIORITY_COUNT || (handle & TASK_HANDLE_MASK) != TASK_HANDLE_MONIKER) return false; task_event_t ev = { handle, param }; - bool res = pdPASS == xQueueSendToBackFromISR (task_Q[priority], &ev, NULL); + bool res = (pdPASS == xQueueSendToBack(task_Q[priority], &ev, 0)); - xSemaphoreGiveFromISR (pending, NULL); + xSemaphoreGive(pending); + + return res; +} + + +bool IRAM_ATTR task_post_isr(task_prio_t priority, task_handle_t handle, task_param_t param) +{ + if (priority >= TASK_PRIORITY_COUNT || + (handle & TASK_HANDLE_MASK) != TASK_HANDLE_MONIKER) + return false; + + task_event_t ev = { handle, param }; + bool res = (pdPASS == xQueueSendToBackFromISR (task_Q[priority], &ev, NULL)); + + BaseType_t woken = pdFALSE; + xSemaphoreGiveFromISR (pending, &woken); + portYIELD_FROM_ISR(woken); return res; }