Skip to content

fix(lwip): LWIP memory/performanche heap optimization (IDFGH-16710) #17796

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+44 −42
Open

fix(lwip): LWIP memory/performanche heap optimization (IDFGH-16710) #17796

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ecc0c38
fix(lwip): Avoid dual heap allocations of semaphores by LwIP
jimmyw Nov 29, 2024
b410410
fix(lwip): Use static queue for LwIP mbox
jimmyw Nov 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions components/esp_netif/lwip/esp_netif_lwip.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,8 +207,8 @@ static void netif_unset_garp_flag(struct netif *netif)
#endif // CONFIG_LWIP_GARP_TMR_INTERVAL

#if !LWIP_TCPIP_CORE_LOCKING
static sys_sem_t api_sync_sem = NULL;
static sys_sem_t api_lock_sem = NULL;
static sys_sem_t api_sync_sem = {0};
static sys_sem_t api_lock_sem = {0};
#endif

/**
Expand Down Expand Up @@ -588,14 +588,14 @@ esp_err_t esp_netif_init(void)
}

#if !LWIP_TCPIP_CORE_LOCKING
if (!api_sync_sem) {
if (!sys_sem_valid(&api_sync_sem)) {
if (ERR_OK != sys_sem_new(&api_sync_sem, 0)) {
ESP_LOGE(TAG, "esp netif api sync sem init fail");
return ESP_FAIL;
}
}

if (!api_lock_sem) {
if (!sys_sem_valid(&api_lock_sem)) {
if (ERR_OK != sys_sem_new(&api_lock_sem, 1)) {
ESP_LOGE(TAG, "esp netif api lock sem init fail");
return ESP_FAIL;
Expand Down
12 changes: 6 additions & 6 deletions components/lwip/port/freertos/include/arch/sys_arch.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
*
* SPDX-License-Identifier: BSD-3-Clause
*
* SPDX-FileContributor: 2018-2024 Espressif Systems (Shanghai) CO LTD
* SPDX-FileContributor: 2018-2025 Espressif Systems (Shanghai) CO LTD
*/
#ifndef __SYS_ARCH_H__
#define __SYS_ARCH_H__
Expand All @@ -18,12 +18,13 @@ extern "C" {
#endif


typedef SemaphoreHandle_t sys_sem_t;
typedef StaticSemaphore_t sys_sem_t;
typedef SemaphoreHandle_t sys_mutex_t;
typedef TaskHandle_t sys_thread_t;

typedef struct sys_mbox_s {
QueueHandle_t os_mbox;
StaticQueue_t os_mbox;
uint8_t buffer[0];
}* sys_mbox_t;

/** This is returned by _fromisr() sys functions to tell the outermost function
Expand All @@ -45,9 +46,8 @@ void sys_delay_ms(uint32_t ms);
#define sys_mbox_valid(mbox) (*(mbox) != NULL)
#define sys_mbox_set_invalid(mbox) (*(mbox) = NULL)

#define sys_sem_valid_val(sema) ((sema) != NULL)
#define sys_sem_valid(sema) (((sema) != NULL) && sys_sem_valid_val(*(sema)))
#define sys_sem_set_invalid(sema) ((*(sema)) = NULL)
#define sys_sem_valid(sema) ( ( memcmp((sema), &(StaticSemaphore_t){0}, sizeof(StaticSemaphore_t)) == 0) ? pdFALSE : pdTRUE )
#define sys_sem_set_invalid(sema) memset((sema), 0, sizeof(StaticSemaphore_t))

void sys_delay_ms(uint32_t ms);
sys_sem_t* sys_thread_sem_init(void);
Expand Down
66 changes: 34 additions & 32 deletions components/lwip/port/freertos/sys_arch.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,14 +108,14 @@ sys_sem_new(sys_sem_t *sem, u8_t count)
LWIP_ASSERT("initial_count invalid (neither 0 nor 1)",
(count == 0) || (count == 1));

*sem = xSemaphoreCreateBinary();
if (*sem == NULL) {
LWIP_DEBUGF(SYS_DEBUG, ("sys_sem_new: out of mem\r\n"));
QueueHandle_t res = xSemaphoreCreateBinaryStatic(sem);
if ((sys_sem_t *)res != sem) {
LWIP_DEBUGF(ESP_THREAD_SAFE_DEBUG, ("sys_sem_new: out of mem\r\n"));
return ERR_MEM;
}

if (count == 1) {
BaseType_t ret = xSemaphoreGive(*sem);
BaseType_t ret = xSemaphoreGive(sem);
LWIP_ASSERT("sys_sem_new: initial give failed", ret == pdTRUE);
(void)ret;
}
Expand All @@ -131,7 +131,7 @@ sys_sem_new(sys_sem_t *sem, u8_t count)
void
sys_sem_signal(sys_sem_t *sem)
{
BaseType_t ret = xSemaphoreGive(*sem);
BaseType_t ret = xSemaphoreGive(sem);
/* queue full is OK, this is a signal only... */
LWIP_ASSERT("sys_sem_signal: sane return value",
(ret == pdTRUE) || (ret == errQUEUE_FULL));
Expand All @@ -144,7 +144,7 @@ int
sys_sem_signal_isr(sys_sem_t *sem)
{
BaseType_t woken = pdFALSE;
xSemaphoreGiveFromISR(*sem, &woken);
xSemaphoreGiveFromISR(sem, &woken);
return woken == pdTRUE;
}

Expand All @@ -162,7 +162,7 @@ sys_arch_sem_wait(sys_sem_t *sem, u32_t timeout)

if (!timeout) {
/* wait infinite */
ret = xSemaphoreTake(*sem, portMAX_DELAY);
ret = xSemaphoreTake((QueueHandle_t)sem, portMAX_DELAY);
LWIP_ASSERT("taking semaphore failed", ret == pdTRUE);
} else {
/* Round up the number of ticks.
Expand All @@ -172,7 +172,7 @@ sys_arch_sem_wait(sys_sem_t *sem, u32_t timeout)
* 1 tick before triggering a timeout. Thus, we need to pass 2 ticks as a timeout
* to `xSemaphoreTake`. */
TickType_t timeout_ticks = ((timeout + portTICK_PERIOD_MS - 1) / portTICK_PERIOD_MS) + 1;
ret = xSemaphoreTake(*sem, timeout_ticks);
ret = xSemaphoreTake((QueueHandle_t)sem, timeout_ticks);
if (ret == errQUEUE_EMPTY) {
/* timed out */
return SYS_ARCH_TIMEOUT;
Expand All @@ -191,8 +191,8 @@ sys_arch_sem_wait(sys_sem_t *sem, u32_t timeout)
void
sys_sem_free(sys_sem_t *sem)
{
vSemaphoreDelete(*sem);
*sem = NULL;
vSemaphoreDelete((QueueHandle_t)sem);
memset(sem, 0, sizeof(*sem));
}

/**
Expand All @@ -205,21 +205,21 @@ sys_sem_free(sys_sem_t *sem)
err_t
sys_mbox_new(sys_mbox_t *mbox, int size)
{
*mbox = mem_malloc(sizeof(struct sys_mbox_s));
*mbox = (sys_mbox_t)mem_malloc(sizeof(struct sys_mbox_s) + (size * sizeof(void *)));
if (*mbox == NULL){
LWIP_DEBUGF(SYS_DEBUG, ("fail to new *mbox\n"));
return ERR_MEM;
}

(*mbox)->os_mbox = xQueueCreate(size, sizeof(void *));
QueueHandle_t res = xQueueCreateStatic(size, sizeof(void *), (*mbox)->buffer, &(*mbox)->os_mbox);

if ((*mbox)->os_mbox == NULL) {
if ((QueueHandle_t)&(*mbox)->os_mbox != res) {
LWIP_DEBUGF(SYS_DEBUG, ("fail to new (*mbox)->os_mbox\n"));
mem_free(*mbox);
free(*mbox);
return ERR_MEM;
}

LWIP_DEBUGF(SYS_DEBUG, ("new *mbox ok mbox=%p os_mbox=%p\n", *mbox, (*mbox)->os_mbox));
LWIP_DEBUGF(SYS_DEBUG, ("new *mbox ok mbox=%p os_mbox=%p\n", *mbox, (QueueHandle_t)&(*mbox)->os_mbox));
return ERR_OK;
}

Expand All @@ -232,7 +232,7 @@ sys_mbox_new(sys_mbox_t *mbox, int size)
void
sys_mbox_post(sys_mbox_t *mbox, void *msg)
{
BaseType_t ret = xQueueSendToBack((*mbox)->os_mbox, &msg, portMAX_DELAY);
BaseType_t ret = xQueueSendToBack((QueueHandle_t)&(*mbox)->os_mbox, &msg, portMAX_DELAY);
LWIP_ASSERT("mbox post failed", ret == pdTRUE);
(void)ret;
}
Expand All @@ -249,10 +249,10 @@ sys_mbox_trypost(sys_mbox_t *mbox, void *msg)
{
err_t xReturn;

if (xQueueSend((*mbox)->os_mbox, &msg, 0) == pdTRUE) {
if (xQueueSend((QueueHandle_t)&(*mbox)->os_mbox, &msg, 0) == pdTRUE) {
xReturn = ERR_OK;
} else {
LWIP_DEBUGF(SYS_DEBUG, ("trypost mbox=%p fail\n", (*mbox)->os_mbox));
LWIP_DEBUGF(SYS_DEBUG, ("trypost mbox=%p fail\n", (QueueHandle_t)&(*mbox)->os_mbox));
xReturn = ERR_MEM;
}

Expand All @@ -274,7 +274,7 @@ sys_mbox_trypost_fromisr(sys_mbox_t *mbox, void *msg)
BaseType_t ret;
BaseType_t xHigherPriorityTaskWoken = pdFALSE;

ret = xQueueSendFromISR((*mbox)->os_mbox, &msg, &xHigherPriorityTaskWoken);
ret = xQueueSendFromISR((QueueHandle_t)&(*mbox)->os_mbox, &msg, &xHigherPriorityTaskWoken);
if (ret == pdTRUE) {
if (xHigherPriorityTaskWoken == pdTRUE) {
return ERR_NEED_SCHED;
Expand Down Expand Up @@ -306,11 +306,11 @@ sys_arch_mbox_fetch(sys_mbox_t *mbox, void **msg, u32_t timeout)

if (timeout == 0) {
/* wait infinite */
ret = xQueueReceive((*mbox)->os_mbox, &(*msg), portMAX_DELAY);
ret = xQueueReceive((QueueHandle_t)&(*mbox)->os_mbox, &(*msg), portMAX_DELAY);
LWIP_ASSERT("mbox fetch failed", ret == pdTRUE);
} else {
TickType_t timeout_ticks = timeout / portTICK_PERIOD_MS;
ret = xQueueReceive((*mbox)->os_mbox, &(*msg), timeout_ticks);
ret = xQueueReceive((QueueHandle_t)&(*mbox)->os_mbox, &(*msg), timeout_ticks);
if (ret == errQUEUE_EMPTY) {
/* timed out */
*msg = NULL;
Expand Down Expand Up @@ -338,7 +338,7 @@ sys_arch_mbox_tryfetch(sys_mbox_t *mbox, void **msg)
if (msg == NULL) {
msg = &msg_dummy;
}
ret = xQueueReceive((*mbox)->os_mbox, &(*msg), 0);
ret = xQueueReceive((QueueHandle_t)&(*mbox)->os_mbox, &(*msg), 0);
if (ret == errQUEUE_EMPTY) {
*msg = NULL;
return SYS_MBOX_EMPTY;
Expand All @@ -359,10 +359,10 @@ sys_mbox_free(sys_mbox_t *mbox)
if ((NULL == mbox) || (NULL == *mbox)) {
return;
}
UBaseType_t msgs_waiting = uxQueueMessagesWaiting((*mbox)->os_mbox);
UBaseType_t msgs_waiting = uxQueueMessagesWaiting((QueueHandle_t)&(*mbox)->os_mbox);
LWIP_ASSERT("mbox quence not empty", msgs_waiting == 0);

vQueueDelete((*mbox)->os_mbox);
vQueueDelete((QueueHandle_t)&(*mbox)->os_mbox);
mem_free(*mbox);
*mbox = NULL;

Expand Down Expand Up @@ -490,31 +490,33 @@ sys_thread_sem_free(void* data) // destructor for TLS semaphore
{
sys_sem_t *sem = (sys_sem_t*)(data);

if (sem && *sem){
LWIP_DEBUGF(SYS_DEBUG, ("sem del, sem=%p\n", *sem));
vSemaphoreDelete(*sem);
if (sem){
LWIP_DEBUGF(ESP_THREAD_SAFE_DEBUG, ("sem del, sem=%p\n", sem));
vSemaphoreDelete((QueueHandle_t)sem);
}

if (sem) {
LWIP_DEBUGF(SYS_DEBUG, ("sem pointer del, sem_p=%p\n", sem));
LWIP_DEBUGF(ESP_THREAD_SAFE_DEBUG, ("sem pointer del, sem_p=%p\n", sem));
mem_free(sem);
}
}

sys_sem_t*
sys_thread_sem_init(void)
{
sys_sem_t *sem = (sys_sem_t*)mem_malloc(sizeof(sys_sem_t*));
sys_sem_t *sem = (sys_sem_t*)mem_malloc(sizeof(sys_sem_t));

if (!sem){
ESP_LOGE(TAG, "thread_sem_init: out of memory");
return 0;
}

*sem = xSemaphoreCreateBinary();
if (!(*sem)){
// If successful, create a binary semaphore the pointer to the semaphore is returned
QueueHandle_t res = xSemaphoreCreateBinaryStatic(sem);

if (res != (QueueHandle_t)sem){
mem_free(sem);
ESP_LOGE(TAG, "thread_sem_init: out of memory");
ESP_LOGE(TAG, "thread_sem_init: unable to create semaphore %p != %p", res, sem);
return 0;
}

Expand Down