Merge branch 'bugfix/select_init_sem' into 'master'

VFS: Allocate socket select semaphore outside ISR

See merge request idf/esp-idf!4591
This commit is contained in:
Ivan Grokhotkov 2019-04-25 19:10:08 +08:00
commit 89798b328b
4 changed files with 67 additions and 45 deletions

View file

@ -27,18 +27,26 @@
_Static_assert(MAX_FDS >= CONFIG_LWIP_MAX_SOCKETS, "MAX_FDS < CONFIG_LWIP_MAX_SOCKETS");
static void lwip_stop_socket_select()
static void lwip_stop_socket_select(void *sem)
{
sys_sem_signal(sys_thread_sem_get()); //socket_select will return
sys_sem_signal(sem); //socket_select will return
}
static void lwip_stop_socket_select_isr(BaseType_t *woken)
static void lwip_stop_socket_select_isr(void *sem, BaseType_t *woken)
{
if (sys_sem_signal_isr(sys_thread_sem_get()) && woken) {
if (sys_sem_signal_isr(sem) && woken) {
*woken = pdTRUE;
}
}
static void *lwip_get_socket_select_semaphore()
{
/* Calling this from the same process as select() will ensure that the semaphore won't be allocated from
* ISR (lwip_stop_socket_select_isr).
*/
return (void *) sys_thread_sem_get();
}
static int lwip_fcntl_r_wrapper(int fd, int cmd, int arg)
{
return lwip_fcntl_r(fd, cmd, arg);
@ -61,6 +69,7 @@ void esp_vfs_lwip_sockets_register()
.fcntl = &lwip_fcntl_r_wrapper,
.ioctl = &lwip_ioctl_r_wrapper,
.socket_select = &lwip_select,
.get_socket_select_semaphore = &lwip_get_socket_select_semaphore,
.stop_socket_select = &lwip_stop_socket_select,
.stop_socket_select_isr = &lwip_stop_socket_select_isr,
};

View file

@ -66,6 +66,16 @@ extern "C" {
*/
typedef int esp_vfs_id_t;
/**
* @brief VFS semaphore type for select()
*
*/
typedef struct
{
bool is_sem_local; /*!< type of "sem" is SemaphoreHandle_t when true, defined by socket driver otherwise */
void *sem; /*!< semaphore instance */
} esp_vfs_select_sem_t;
/**
* @brief VFS definition structure
*
@ -218,14 +228,16 @@ typedef struct
#endif // CONFIG_SUPPORT_TERMIOS
/** start_select is called for setting up synchronous I/O multiplexing of the desired file descriptors in the given VFS */
esp_err_t (*start_select)(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds, SemaphoreHandle_t *signal_sem);
esp_err_t (*start_select)(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds, esp_vfs_select_sem_t sem);
/** socket select function for socket FDs with the functionality of POSIX select(); this should be set only for the socket VFS */
int (*socket_select)(int nfds, fd_set *readfds, fd_set *writefds, fd_set *errorfds, struct timeval *timeout);
/** called by VFS to interrupt the socket_select call when select is activated from a non-socket VFS driver; set only for the socket driver */
void (*stop_socket_select)();
void (*stop_socket_select)(void *sem);
/** stop_socket_select which can be called from ISR; set only for the socket driver */
void (*stop_socket_select_isr)(BaseType_t *woken);
void (*stop_socket_select_isr)(void *sem, BaseType_t *woken);
/** end_select is called to stop the I/O multiplexing and deinitialize the environment created by start_select for the given VFS */
void* (*get_socket_select_semaphore)();
/** get_socket_select_semaphore returns semaphore allocated in the socket driver; set only for the socket driver */
void (*end_select)();
} esp_vfs_t;
@ -371,9 +383,9 @@ int esp_vfs_select(int nfds, fd_set *readfds, fd_set *writefds, fd_set *errorfds
* This function is called when the VFS driver detects a read/write/error
* condition as it was requested by the previous call to start_select.
*
* @param signal_sem semaphore handle which was passed to the driver by the start_select call
* @param sem semaphore structure which was passed to the driver by the start_select call
*/
void esp_vfs_select_triggered(SemaphoreHandle_t *signal_sem);
void esp_vfs_select_triggered(esp_vfs_select_sem_t sem);
/**
* @brief Notification from a VFS driver about a read/write/error condition (ISR version)
@ -381,10 +393,10 @@ void esp_vfs_select_triggered(SemaphoreHandle_t *signal_sem);
* This function is called when the VFS driver detects a read/write/error
* condition as it was requested by the previous call to start_select.
*
* @param signal_sem semaphore handle which was passed to the driver by the start_select call
* @param sem semaphore structure which was passed to the driver by the start_select call
* @param woken is set to pdTRUE if the function wakes up a task with higher priority
*/
void esp_vfs_select_triggered_isr(SemaphoreHandle_t *signal_sem, BaseType_t *woken);
void esp_vfs_select_triggered_isr(esp_vfs_select_sem_t sem, BaseType_t *woken);
/**
* @brief Implements the VFS layer for synchronous I/O multiplexing by poll()

View file

@ -824,6 +824,11 @@ int esp_vfs_select(int nfds, fd_set *readfds, fd_set *writefds, fd_set *errorfds
return -1;
}
esp_vfs_select_sem_t sel_sem = {
.is_sem_local = false,
.sem = NULL,
};
int (*socket_select)(int, fd_set *, fd_set *, fd_set *, struct timeval *) = NULL;
for (int fd = 0; fd < nfds; ++fd) {
_lock_acquire(&s_fd_table_lock);
@ -844,6 +849,7 @@ int esp_vfs_select(int nfds, fd_set *readfds, fd_set *writefds, fd_set *errorfds
esp_vfs_safe_fd_isset(fd, errorfds)) {
const vfs_entry_t *vfs = s_vfs[vfs_index];
socket_select = vfs->vfs.socket_select;
sel_sem.sem = vfs->vfs.get_socket_select_semaphore();
}
}
continue;
@ -874,19 +880,14 @@ int esp_vfs_select(int nfds, fd_set *readfds, fd_set *writefds, fd_set *errorfds
// the global readfds, writefds and errorfds contain only socket FDs (if
// there any)
/* Semaphore used for waiting select events from other VFS drivers when socket
* select is not used (not registered or socket FDs are not observed by the
* given call of select)
*/
SemaphoreHandle_t select_sem = NULL;
if (!socket_select) {
// There is no socket VFS registered or select() wasn't called for
// any socket. Therefore, we will use our own signalization.
if ((select_sem = xSemaphoreCreateBinary()) == NULL) {
sel_sem.is_sem_local = true;
if ((sel_sem.sem = xSemaphoreCreateBinary()) == NULL) {
free(vfs_fds_triple);
__errno_r(r) = ENOMEM;
ESP_LOGD(TAG, "cannot create select_sem");
ESP_LOGD(TAG, "cannot create select semaphore");
return -1;
}
}
@ -902,18 +903,18 @@ int esp_vfs_select(int nfds, fd_set *readfds, fd_set *writefds, fd_set *errorfds
esp_vfs_log_fd_set("readfds", &item->readfds);
esp_vfs_log_fd_set("writefds", &item->writefds);
esp_vfs_log_fd_set("errorfds", &item->errorfds);
esp_err_t err = vfs->vfs.start_select(nfds, &item->readfds, &item->writefds, &item->errorfds, &select_sem);
esp_err_t err = vfs->vfs.start_select(nfds, &item->readfds, &item->writefds, &item->errorfds, sel_sem);
if (err != ESP_OK) {
call_end_selects(i, vfs_fds_triple);
(void) set_global_fd_sets(vfs_fds_triple, s_vfs_count, readfds, writefds, errorfds);
if (select_sem) {
vSemaphoreDelete(select_sem);
select_sem = NULL;
if (sel_sem.is_sem_local && sel_sem.sem) {
vSemaphoreDelete(sel_sem.sem);
sel_sem.sem = NULL;
}
free(vfs_fds_triple);
__errno_r(r) = EINTR;
ESP_LOGD(TAG, "start_select failed");
ESP_LOGD(TAG, "start_select failed: %s", esp_err_to_name(err));
return -1;
}
}
@ -947,16 +948,16 @@ int esp_vfs_select(int nfds, fd_set *readfds, fd_set *writefds, fd_set *errorfds
ESP_LOGD(TAG, "timeout is %dms", timeout_ms);
}
ESP_LOGD(TAG, "waiting without calling socket_select");
xSemaphoreTake(select_sem, ticks_to_wait);
xSemaphoreTake(sel_sem.sem, ticks_to_wait);
}
call_end_selects(s_vfs_count, vfs_fds_triple); // for VFSs for start_select was called before
if (ret >= 0) {
ret += set_global_fd_sets(vfs_fds_triple, s_vfs_count, readfds, writefds, errorfds);
}
if (select_sem) {
vSemaphoreDelete(select_sem);
select_sem = NULL;
if (sel_sem.is_sem_local && sel_sem.sem) {
vSemaphoreDelete(sel_sem.sem);
sel_sem.sem = NULL;
}
free(vfs_fds_triple);
@ -967,10 +968,10 @@ int esp_vfs_select(int nfds, fd_set *readfds, fd_set *writefds, fd_set *errorfds
return ret;
}
void esp_vfs_select_triggered(SemaphoreHandle_t *signal_sem)
void esp_vfs_select_triggered(esp_vfs_select_sem_t sem)
{
if (signal_sem && (*signal_sem)) {
xSemaphoreGive(*signal_sem);
if (sem.is_sem_local) {
xSemaphoreGive(sem.sem);
} else {
// Another way would be to go through s_fd_table and find the VFS
// which has a permanent FD. But in order to avoid to lock
@ -978,17 +979,17 @@ void esp_vfs_select_triggered(SemaphoreHandle_t *signal_sem)
for (int i = 0; i < s_vfs_count; ++i) {
const vfs_entry_t *vfs = s_vfs[i];
if (vfs != NULL && vfs->vfs.stop_socket_select != NULL) {
vfs->vfs.stop_socket_select();
vfs->vfs.stop_socket_select(sem.sem);
break;
}
}
}
}
void esp_vfs_select_triggered_isr(SemaphoreHandle_t *signal_sem, BaseType_t *woken)
void esp_vfs_select_triggered_isr(esp_vfs_select_sem_t sem, BaseType_t *woken)
{
if (signal_sem && (*signal_sem)) {
xSemaphoreGiveFromISR(*signal_sem, woken);
if (sem.is_sem_local) {
xSemaphoreGiveFromISR(sem.sem, woken);
} else {
// Another way would be to go through s_fd_table and find the VFS
// which has a permanent FD. But in order to avoid to lock
@ -996,7 +997,7 @@ void esp_vfs_select_triggered_isr(SemaphoreHandle_t *signal_sem, BaseType_t *wok
for (int i = 0; i < s_vfs_count; ++i) {
const vfs_entry_t *vfs = s_vfs[i];
if (vfs != NULL && vfs->vfs.stop_socket_select_isr != NULL) {
vfs->vfs.stop_socket_select_isr(woken);
vfs->vfs.stop_socket_select_isr(sem.sem, woken);
break;
}
}

View file

@ -62,7 +62,7 @@ static bool s_non_blocking[UART_NUM];
/* Lock ensuring that uart_select is used from only one task at the time */
static _lock_t s_one_select_lock;
static SemaphoreHandle_t *_signal_sem = NULL;
static esp_vfs_select_sem_t _select_sem = {.sem = NULL};
static fd_set *_readfds = NULL;
static fd_set *_writefds = NULL;
static fd_set *_errorfds = NULL;
@ -319,25 +319,25 @@ static void select_notif_callback(uart_port_t uart_num, uart_select_notif_t uart
case UART_SELECT_READ_NOTIF:
if (FD_ISSET(uart_num, _readfds_orig)) {
FD_SET(uart_num, _readfds);
esp_vfs_select_triggered_isr(_signal_sem, task_woken);
esp_vfs_select_triggered_isr(_select_sem, task_woken);
}
break;
case UART_SELECT_WRITE_NOTIF:
if (FD_ISSET(uart_num, _writefds_orig)) {
FD_SET(uart_num, _writefds);
esp_vfs_select_triggered_isr(_signal_sem, task_woken);
esp_vfs_select_triggered_isr(_select_sem, task_woken);
}
break;
case UART_SELECT_ERROR_NOTIF:
if (FD_ISSET(uart_num, _errorfds_orig)) {
FD_SET(uart_num, _errorfds);
esp_vfs_select_triggered_isr(_signal_sem, task_woken);
esp_vfs_select_triggered_isr(_select_sem, task_woken);
}
break;
}
}
static esp_err_t uart_start_select(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds, SemaphoreHandle_t *signal_sem)
static esp_err_t uart_start_select(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds, esp_vfs_select_sem_t select_sem)
{
if (_lock_try_acquire(&s_one_select_lock)) {
return ESP_ERR_INVALID_STATE;
@ -347,7 +347,7 @@ static esp_err_t uart_start_select(int nfds, fd_set *readfds, fd_set *writefds,
portENTER_CRITICAL(uart_get_selectlock());
if (_readfds || _writefds || _errorfds || _readfds_orig || _writefds_orig || _errorfds_orig || _signal_sem) {
if (_readfds || _writefds || _errorfds || _readfds_orig || _writefds_orig || _errorfds_orig || _select_sem.sem) {
portEXIT_CRITICAL(uart_get_selectlock());
uart_end_select();
return ESP_ERR_INVALID_STATE;
@ -378,7 +378,7 @@ static esp_err_t uart_start_select(int nfds, fd_set *readfds, fd_set *writefds,
}
}
_signal_sem = signal_sem;
_select_sem = select_sem;
_readfds = readfds;
_writefds = writefds;
@ -398,7 +398,7 @@ static esp_err_t uart_start_select(int nfds, fd_set *readfds, fd_set *writefds,
if (uart_get_buffered_data_len(i, &buffered_size) == ESP_OK && buffered_size > 0) {
// signalize immediately when data is buffered
FD_SET(i, _readfds);
esp_vfs_select_triggered(_signal_sem);
esp_vfs_select_triggered(_select_sem);
}
}
}
@ -417,7 +417,7 @@ static void uart_end_select()
uart_set_select_notif_callback(i, NULL);
}
_signal_sem = NULL;
_select_sem.sem = NULL;
_readfds = NULL;
_writefds = NULL;