From 8a6ca944841fc626218229d25f7e3ab2c3f68caa Mon Sep 17 00:00:00 2001 From: "sonika.rathi" Date: Thu, 17 Aug 2023 09:36:32 +0200 Subject: [PATCH] fix(vfs/uart): Add support for multi-task call to uart select (v5.1) --- components/vfs/test/test_vfs_select.c | 472 +++++++++++++++----------- components/vfs/vfs_uart.c | 19 +- 2 files changed, 294 insertions(+), 197 deletions(-) diff --git a/components/vfs/test/test_vfs_select.c b/components/vfs/test/test_vfs_select.c index ceab7004f7..69228dee96 100644 --- a/components/vfs/test/test_vfs_select.c +++ b/components/vfs/test/test_vfs_select.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2018-2022 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2018-2023 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -101,6 +101,23 @@ static void uart1_init(void) uart_param_config(UART_NUM_1, &uart_config); } +static void read_task(void *param) +{ + char recv_message[sizeof(message)]; + const test_task_param_t *test_task_param = param; + vTaskDelay(test_task_param->delay_ms / portTICK_PERIOD_MS); + read(test_task_param->fd, recv_message, sizeof(message)); + if (test_task_param->sem) { + xSemaphoreGive(test_task_param->sem); + } + vTaskDelete(NULL); +} + +static inline void start_read_task(const test_task_param_t *test_task_param) +{ + xTaskCreate(read_task, "read_task", 8*1024, (void *) test_task_param, 5, NULL); +} + static void send_task(void *param) { const test_task_param_t *test_task_param = param; @@ -112,7 +129,7 @@ static void send_task(void *param) vTaskDelete(NULL); } -static inline void start_task(const test_task_param_t *test_task_param) +static inline void start_write_task(const test_task_param_t *test_task_param) { xTaskCreate(send_task, "send_task", 8*1024, (void *) test_task_param, 5, NULL); } @@ -141,6 +158,24 @@ static void deinit(int uart_fd, int socket_fd) close(socket_fd); } +static void select_task(void *task_param) +{ + const test_select_task_param_t *param = task_param; + + int s = select(param->maxfds, param->rdfds, param->wrfds, param->errfds, param->tv); + TEST_ASSERT_EQUAL(param->select_ret, s); + + if (param->sem) { + xSemaphoreGive(param->sem); + } + vTaskDelete(NULL); +} + +static void inline start_select_task(test_select_task_param_t *param) +{ + xTaskCreate(select_task, "select_task", 4*1024, (void *) param, 5, NULL); +} + #if !CONFIG_IDF_TARGET_ESP32H2 // IDF-6782 TEST_CASE("UART can do select()", "[vfs]") { @@ -165,7 +200,7 @@ TEST_CASE("UART can do select()", "[vfs]") .sem = xSemaphoreCreateBinary(), }; TEST_ASSERT_NOT_NULL(test_task_param.sem); - start_task(&test_task_param); + start_write_task(&test_task_param); int s = select(uart_fd + 1, &rfds, NULL, NULL, &tv); TEST_ASSERT_EQUAL(s, 1); @@ -182,7 +217,7 @@ TEST_CASE("UART can do select()", "[vfs]") FD_SET(uart_fd, &rfds); FD_SET(socket_fd, &rfds); - start_task(&test_task_param); + start_write_task(&test_task_param); s = select(MAX(uart_fd, socket_fd) + 1, &rfds, NULL, NULL, &tv); TEST_ASSERT_EQUAL(s, 1); @@ -199,6 +234,240 @@ TEST_CASE("UART can do select()", "[vfs]") deinit(uart_fd, socket_fd); } +TEST_CASE("concurrent selects work for UART", "[vfs]") +{ + // This test case initiates two select tasks on the same UART FD, + // One task will wait for a write operation, while the other will wait for a read operation to occur. + // The first task will complete its operation before the second task proceeds with its operation on the same FD + // In this scenario, the write operation will be performed initially, + // followed by the subsequent continuation of the read operation. + + int uart_fd, socket_fd; + init(&uart_fd, &socket_fd); + + const test_task_param_t send_param = { + .fd = uart_fd, + .delay_ms = 0, + .sem = NULL, + }; + + fd_set wrfds1; + FD_ZERO(&wrfds1); + FD_SET(uart_fd, &wrfds1); + + test_select_task_param_t param_write = { + .rdfds = NULL, + .wrfds = &wrfds1, + .errfds = NULL, + .maxfds = uart_fd + 1, + .tv = NULL, + .select_ret = 1, + .sem = xSemaphoreCreateBinary(), + }; + TEST_ASSERT_NOT_NULL(param_write.sem); + //Start first task which will wait on select call for write operation on the UART FD + start_select_task(¶m_write); + + fd_set rdfds2; + FD_ZERO(&rdfds2); + FD_SET(uart_fd, &rdfds2); + + test_select_task_param_t param_read = { + .rdfds = &rdfds2, + .wrfds = NULL, + .errfds = NULL, + .maxfds = uart_fd + 1, + .tv = NULL, + .select_ret = 1, + .sem = xSemaphoreCreateBinary(), + }; + TEST_ASSERT_NOT_NULL(param_read.sem); + //Start second task which will wait on another select call for read operation on the same UART FD + start_select_task(¶m_read); + + //Start writing operation on the UART port + start_write_task(&send_param); + //Confirm the completion of the write operation + TEST_ASSERT_EQUAL(pdTRUE, xSemaphoreTake(param_write.sem, 1000 / portTICK_PERIOD_MS)); + vSemaphoreDelete(param_write.sem); + TEST_ASSERT(FD_ISSET(uart_fd, &wrfds1)); + + //Start reading operation on the same UART port + start_read_task(&send_param); + //Confirm the completion of the read operation + TEST_ASSERT_EQUAL(pdTRUE, xSemaphoreTake(param_read.sem, 1000 / portTICK_PERIOD_MS)); + vSemaphoreDelete(param_read.sem); + TEST_ASSERT(FD_ISSET(uart_fd, &rdfds2)); + + deinit(uart_fd, socket_fd); +} + +TEST_CASE("concurrent selects work", "[vfs]") +{ + int uart_fd, socket_fd; + init(&uart_fd, &socket_fd); + const int dummy_socket_fd = open_dummy_socket(); + + { + // Two tasks will wait for the same UART FD for reading and they will time-out + + struct timeval tv = { + .tv_sec = 0, + .tv_usec = 100000, + }; + + fd_set rdfds1; + FD_ZERO(&rdfds1); + FD_SET(uart_fd, &rdfds1); + + test_select_task_param_t param = { + .rdfds = &rdfds1, + .wrfds = NULL, + .errfds = NULL, + .maxfds = uart_fd + 1, + .tv = &tv, + .select_ret = 0, // expected timeout + .sem = xSemaphoreCreateBinary(), + }; + TEST_ASSERT_NOT_NULL(param.sem); + + fd_set rdfds2; + FD_ZERO(&rdfds2); + FD_SET(uart_fd, &rdfds2); + FD_SET(socket_fd, &rdfds2); + FD_SET(dummy_socket_fd, &rdfds2); + + start_select_task(¶m); + vTaskDelay(10 / portTICK_PERIOD_MS); //make sure the task has started and waits in select() + + int s = select(MAX(MAX(uart_fd, dummy_socket_fd), socket_fd) + 1, &rdfds2, NULL, NULL, &tv); + TEST_ASSERT_EQUAL(0, s); // timeout here as well + + TEST_ASSERT_EQUAL(pdTRUE, xSemaphoreTake(param.sem, 1000 / portTICK_PERIOD_MS)); + vSemaphoreDelete(param.sem); + } + + { + // One tasks waits for UART reading and one for writing. The former will be successful and latter will + // time-out. + + struct timeval tv = { + .tv_sec = 0, + .tv_usec = 100000, + }; + + fd_set wrfds1; + FD_ZERO(&wrfds1); + FD_SET(uart_fd, &wrfds1); + + test_select_task_param_t param = { + .rdfds = NULL, + .wrfds = &wrfds1, + .errfds = NULL, + .maxfds = uart_fd + 1, + .tv = &tv, + .select_ret = 1, + .sem = xSemaphoreCreateBinary(), + }; + TEST_ASSERT_NOT_NULL(param.sem); + + const test_task_param_t send_param = { + .fd = uart_fd, + .delay_ms = 50, + .sem = xSemaphoreCreateBinary(), + }; + TEST_ASSERT_NOT_NULL(send_param.sem); + start_write_task(&send_param); // This task will write to UART which will be detected by select() + start_select_task(¶m); + vTaskDelay(100 / portTICK_PERIOD_MS); //make sure the task has started and waits in select() + + fd_set rdfds2; + FD_ZERO(&rdfds2); + FD_SET(uart_fd, &rdfds2); + FD_SET(socket_fd, &rdfds2); + FD_SET(dummy_socket_fd, &rdfds2); + + int s = select(MAX(MAX(uart_fd, dummy_socket_fd), socket_fd) + 1, &rdfds2, NULL, NULL, &tv); + TEST_ASSERT_EQUAL(1, s); + TEST_ASSERT(FD_ISSET(uart_fd, &rdfds2)); + TEST_ASSERT_UNLESS(FD_ISSET(socket_fd, &rdfds2)); + TEST_ASSERT_UNLESS(FD_ISSET(dummy_socket_fd, &rdfds2)); + + TEST_ASSERT_EQUAL(pdTRUE, xSemaphoreTake(param.sem, 1000 / portTICK_PERIOD_MS)); + vSemaphoreDelete(param.sem); + + TEST_ASSERT_EQUAL(pdTRUE, xSemaphoreTake(send_param.sem, 1000 / portTICK_PERIOD_MS)); + vSemaphoreDelete(send_param.sem); + } + + deinit(uart_fd, socket_fd); + close(dummy_socket_fd); +} +#endif + +TEST_CASE("select() works with concurrent mount", "[vfs][fatfs]") +{ + wl_handle_t test_wl_handle; + int uart_fd, socket_fd; + + init(&uart_fd, &socket_fd); + const int dummy_socket_fd = open_dummy_socket(); + + esp_vfs_fat_sdmmc_mount_config_t mount_config = { + .format_if_mount_failed = true, + .max_files = 2 + }; + + // select() will be waiting for a socket & UART and FATFS mount will occur in parallel + + struct timeval tv = { + .tv_sec = 1, + .tv_usec = 0, + }; + + fd_set rdfds; + FD_ZERO(&rdfds); + FD_SET(uart_fd, &rdfds); + FD_SET(dummy_socket_fd, &rdfds); + + test_select_task_param_t param = { + .rdfds = &rdfds, + .wrfds = NULL, + .errfds = NULL, + .maxfds = MAX(uart_fd, dummy_socket_fd) + 1, + .tv = &tv, + .select_ret = 0, // expected timeout + .sem = xSemaphoreCreateBinary(), + }; + TEST_ASSERT_NOT_NULL(param.sem); + + start_select_task(¶m); + vTaskDelay(10 / portTICK_PERIOD_MS); //make sure the task has started and waits in select() + + TEST_ESP_OK(esp_vfs_fat_spiflash_mount_rw_wl("/spiflash", NULL, &mount_config, &test_wl_handle)); + + TEST_ASSERT_EQUAL(pdTRUE, xSemaphoreTake(param.sem, 1500 / portTICK_PERIOD_MS)); + + // select() will be waiting for a socket & UART and FATFS unmount will occur in parallel + + FD_ZERO(&rdfds); + FD_SET(uart_fd, &rdfds); + FD_SET(dummy_socket_fd, &rdfds); + + start_select_task(¶m); + vTaskDelay(10 / portTICK_PERIOD_MS); //make sure the task has started and waits in select() + + TEST_ESP_OK(esp_vfs_fat_spiflash_unmount_rw_wl("/spiflash", test_wl_handle)); + + TEST_ASSERT_EQUAL(pdTRUE, xSemaphoreTake(param.sem, 1500 / portTICK_PERIOD_MS)); + + vSemaphoreDelete(param.sem); + + deinit(uart_fd, socket_fd); + close(dummy_socket_fd); +} + +#if !CONFIG_IDF_TARGET_ESP32H2 // IDF-6782 TEST_CASE("UART can do poll() with POLLIN event", "[vfs]") { int uart_fd; @@ -223,7 +492,7 @@ TEST_CASE("UART can do poll() with POLLIN event", "[vfs]") .sem = xSemaphoreCreateBinary(), }; TEST_ASSERT_NOT_NULL(test_task_param.sem); - start_task(&test_task_param); + start_write_task(&test_task_param); int s = poll(poll_fds, sizeof(poll_fds)/sizeof(poll_fds[0]), 100); TEST_ASSERT_EQUAL(s, 1); @@ -241,7 +510,7 @@ TEST_CASE("UART can do poll() with POLLIN event", "[vfs]") poll_fds[1].fd = socket_fd; poll_fds[1].events = POLLIN; - start_task(&test_task_param); + start_write_task(&test_task_param); s = poll(poll_fds, sizeof(poll_fds)/sizeof(poll_fds[0]), 100); TEST_ASSERT_EQUAL(s, 1); @@ -284,7 +553,7 @@ TEST_CASE("UART can do poll() with POLLOUT event", "[vfs]") .sem = xSemaphoreCreateBinary(), }; TEST_ASSERT_NOT_NULL(test_task_param.sem); - start_task(&test_task_param); + start_write_task(&test_task_param); poll(poll_fds, sizeof(poll_fds)/sizeof(poll_fds[0]), 100); TEST_ASSERT_EQUAL(uart_fd, poll_fds[0].fd); @@ -302,7 +571,6 @@ TEST_CASE("UART can do poll() with POLLOUT event", "[vfs]") deinit(uart_fd, socket_fd); } - #endif TEST_CASE("socket can do select()", "[vfs]") @@ -330,7 +598,7 @@ TEST_CASE("socket can do select()", "[vfs]") .sem = xSemaphoreCreateBinary(), }; TEST_ASSERT_NOT_NULL(test_task_param.sem); - start_task(&test_task_param); + start_write_task(&test_task_param); const int s = select(MAX(MAX(uart_fd, socket_fd), dummy_socket_fd) + 1, &rfds, NULL, NULL, &tv); TEST_ASSERT_EQUAL(1, s); @@ -379,7 +647,7 @@ TEST_CASE("socket can do poll()", "[vfs]") .sem = xSemaphoreCreateBinary(), }; TEST_ASSERT_NOT_NULL(test_task_param.sem); - start_task(&test_task_param); + start_write_task(&test_task_param); int s = poll(poll_fds, sizeof(poll_fds)/sizeof(poll_fds[0]), 100); TEST_ASSERT_EQUAL(s, 1); @@ -469,187 +737,3 @@ TEST_CASE("poll() timeout", "[vfs]") deinit(uart_fd, socket_fd); } - -static void select_task(void *task_param) -{ - const test_select_task_param_t *param = task_param; - - int s = select(param->maxfds, param->rdfds, param->wrfds, param->errfds, param->tv); - TEST_ASSERT_EQUAL(param->select_ret, s); - - if (param->sem) { - xSemaphoreGive(param->sem); - } - vTaskDelete(NULL); -} - -static void inline start_select_task(test_select_task_param_t *param) -{ - xTaskCreate(select_task, "select_task", 4*1024, (void *) param, 5, NULL); -} - -#if !CONFIG_IDF_TARGET_ESP32H2 // IDF-6782 -TEST_CASE("concurrent selects work", "[vfs]") -{ - int uart_fd, socket_fd; - init(&uart_fd, &socket_fd); - const int dummy_socket_fd = open_dummy_socket(); - - { - // Two tasks will wait for the same UART FD for reading and they will time-out - - struct timeval tv = { - .tv_sec = 0, - .tv_usec = 100000, - }; - - fd_set rdfds1; - FD_ZERO(&rdfds1); - FD_SET(uart_fd, &rdfds1); - - test_select_task_param_t param = { - .rdfds = &rdfds1, - .wrfds = NULL, - .errfds = NULL, - .maxfds = uart_fd + 1, - .tv = &tv, - .select_ret = 0, // expected timeout - .sem = xSemaphoreCreateBinary(), - }; - TEST_ASSERT_NOT_NULL(param.sem); - - fd_set rdfds2; - FD_ZERO(&rdfds2); - FD_SET(uart_fd, &rdfds2); - FD_SET(socket_fd, &rdfds2); - FD_SET(dummy_socket_fd, &rdfds2); - - start_select_task(¶m); - vTaskDelay(10 / portTICK_PERIOD_MS); //make sure the task has started and waits in select() - - int s = select(MAX(MAX(uart_fd, dummy_socket_fd), socket_fd) + 1, &rdfds2, NULL, NULL, &tv); - TEST_ASSERT_EQUAL(0, s); // timeout here as well - - TEST_ASSERT_EQUAL(pdTRUE, xSemaphoreTake(param.sem, 1000 / portTICK_PERIOD_MS)); - vSemaphoreDelete(param.sem); - } - - { - // One tasks waits for UART reading and one for writing. The former will be successful and latter will - // time-out. - - struct timeval tv = { - .tv_sec = 0, - .tv_usec = 100000, - }; - - fd_set wrfds1; - FD_ZERO(&wrfds1); - FD_SET(uart_fd, &wrfds1); - - test_select_task_param_t param = { - .rdfds = NULL, - .wrfds = &wrfds1, - .errfds = NULL, - .maxfds = uart_fd + 1, - .tv = &tv, - .select_ret = 1, - .sem = xSemaphoreCreateBinary(), - }; - TEST_ASSERT_NOT_NULL(param.sem); - - const test_task_param_t send_param = { - .fd = uart_fd, - .delay_ms = 50, - .sem = xSemaphoreCreateBinary(), - }; - TEST_ASSERT_NOT_NULL(send_param.sem); - start_task(&send_param); // This task will write to UART which will be detected by select() - start_select_task(¶m); - vTaskDelay(100 / portTICK_PERIOD_MS); //make sure the task has started and waits in select() - - fd_set rdfds2; - FD_ZERO(&rdfds2); - FD_SET(uart_fd, &rdfds2); - FD_SET(socket_fd, &rdfds2); - FD_SET(dummy_socket_fd, &rdfds2); - - int s = select(MAX(MAX(uart_fd, dummy_socket_fd), socket_fd) + 1, &rdfds2, NULL, NULL, &tv); - TEST_ASSERT_EQUAL(1, s); - TEST_ASSERT(FD_ISSET(uart_fd, &rdfds2)); - TEST_ASSERT_UNLESS(FD_ISSET(socket_fd, &rdfds2)); - TEST_ASSERT_UNLESS(FD_ISSET(dummy_socket_fd, &rdfds2)); - - TEST_ASSERT_EQUAL(pdTRUE, xSemaphoreTake(param.sem, 1000 / portTICK_PERIOD_MS)); - vSemaphoreDelete(param.sem); - - TEST_ASSERT_EQUAL(pdTRUE, xSemaphoreTake(send_param.sem, 1000 / portTICK_PERIOD_MS)); - vSemaphoreDelete(send_param.sem); - } - - deinit(uart_fd, socket_fd); - close(dummy_socket_fd); -} -#endif - -TEST_CASE("select() works with concurrent mount", "[vfs][fatfs]") -{ - wl_handle_t test_wl_handle; - int uart_fd, socket_fd; - - init(&uart_fd, &socket_fd); - const int dummy_socket_fd = open_dummy_socket(); - - esp_vfs_fat_sdmmc_mount_config_t mount_config = { - .format_if_mount_failed = true, - .max_files = 2 - }; - - // select() will be waiting for a socket & UART and FATFS mount will occur in parallel - - struct timeval tv = { - .tv_sec = 1, - .tv_usec = 0, - }; - - fd_set rdfds; - FD_ZERO(&rdfds); - FD_SET(uart_fd, &rdfds); - FD_SET(dummy_socket_fd, &rdfds); - - test_select_task_param_t param = { - .rdfds = &rdfds, - .wrfds = NULL, - .errfds = NULL, - .maxfds = MAX(uart_fd, dummy_socket_fd) + 1, - .tv = &tv, - .select_ret = 0, // expected timeout - .sem = xSemaphoreCreateBinary(), - }; - TEST_ASSERT_NOT_NULL(param.sem); - - start_select_task(¶m); - vTaskDelay(10 / portTICK_PERIOD_MS); //make sure the task has started and waits in select() - - TEST_ESP_OK(esp_vfs_fat_spiflash_mount_rw_wl("/spiflash", NULL, &mount_config, &test_wl_handle)); - - TEST_ASSERT_EQUAL(pdTRUE, xSemaphoreTake(param.sem, 1500 / portTICK_PERIOD_MS)); - - // select() will be waiting for a socket & UART and FATFS unmount will occur in parallel - - FD_ZERO(&rdfds); - FD_SET(uart_fd, &rdfds); - FD_SET(dummy_socket_fd, &rdfds); - - start_select_task(¶m); - vTaskDelay(10 / portTICK_PERIOD_MS); //make sure the task has started and waits in select() - - TEST_ESP_OK(esp_vfs_fat_spiflash_unmount_rw_wl("/spiflash", test_wl_handle)); - - TEST_ASSERT_EQUAL(pdTRUE, xSemaphoreTake(param.sem, 1500 / portTICK_PERIOD_MS)); - - vSemaphoreDelete(param.sem); - - deinit(uart_fd, socket_fd); - close(dummy_socket_fd); -} diff --git a/components/vfs/vfs_uart.c b/components/vfs/vfs_uart.c index 52a2622f0f..8c962280c9 100644 --- a/components/vfs/vfs_uart.c +++ b/components/vfs/vfs_uart.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2021 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2023 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -57,6 +57,10 @@ static int uart_rx_char(int fd); static void uart_tx_char_via_driver(int fd, int c); static int uart_rx_char_via_driver(int fd); +#ifdef CONFIG_VFS_SUPPORT_SELECT +static int s_uart_select_count[UART_NUM] = {0}; +#endif //CONFIG_VFS_SUPPORT_SELECT + typedef struct { // Pointers to UART peripherals uart_dev_t* uart; @@ -470,7 +474,10 @@ static esp_err_t uart_start_select(int nfds, fd_set *readfds, fd_set *writefds, //uart_set_select_notif_callback sets the callbacks in UART ISR for (int i = 0; i < max_fds; ++i) { if (FD_ISSET(i, &args->readfds_orig) || FD_ISSET(i, &args->writefds_orig) || FD_ISSET(i, &args->errorfds_orig)) { - uart_set_select_notif_callback(i, select_notif_callback_isr); + if (s_uart_select_count[i] == 0) { + uart_set_select_notif_callback(i, select_notif_callback_isr); + } + s_uart_select_count[i]++; } } @@ -505,7 +512,13 @@ static esp_err_t uart_end_select(void *end_select_args) portENTER_CRITICAL(uart_get_selectlock()); esp_err_t ret = unregister_select(args); for (int i = 0; i < UART_NUM; ++i) { - uart_set_select_notif_callback(i, NULL); + if (FD_ISSET(i, &args->readfds_orig) || FD_ISSET(i, &args->writefds_orig) || FD_ISSET(i, &args->errorfds_orig)) { + s_uart_select_count[i]--; + if (s_uart_select_count[i] == 0) { + uart_set_select_notif_callback(i, NULL); + } + break; + } } portEXIT_CRITICAL(uart_get_selectlock());