cpu/esp32: critcial section handling changed in FreeRTOS adaptation layer

[gschorcht]

Contribution description

This PR changes the critical section handling of the FreeRTOS adaptation layer that is required by ESP32 SDK libraries for the WiFi interface.

Background

ESP32 SDK libraries for the WiFi interface require a number of FreeRTOS functions to work, e.g., crittical section handling, queues, mutexes and semaphores. To provide the ESP SDK libraries with these required FreeRTOS functions, an adaptation layer module in cpu/esp32/freertos realizes a mapping of these functions to RIOT's mechansims.

Critical section handling

The critical section handling of FreeRTOS for ESP32 realized by the portENTER_CRITICAL, taskENTER_CRITICAL, portEXIT_CRITICAL and taskEXIT_CRITICAL macros/functions uses mutexes. However, since these macros/functions are also called in interrupt context, the mutex based implementation of critical section handling does not work in RIOT.

This led to a number of instability problems when the WiFi interface was used (modules esp_wifi and/or esp_now), e.g., the problem with the multiheap corruption described in issue #11941. Therefore, the former mutex based implementation of critical section handling had to be changed.

Change

The mutex that is given as parameter for critical section handling macros/function is not used any longer. Instead, the basic default FreeRTOS mechanism for critical sections is realized by simply disabling interrupts. Once the interrupt is disabled by calling portENTER_CRITICAL or taskENTER_CRITICAL, the execution in critical section can't be interrupted. Since context switches for the ESP32 are also based on interrupts, there is no possibility that another thread will enter the critical section once the interrupts are disabled, even if an action within the critical section would trigger a thread_yield_higher.

I have tested the changes with ESP32 border router test configuration as described in #10929. While the CORRUPT multiheap error message as described in #11941 occured within several minutes without these changes, the configuration is working with the changes since more than one day without any error message.

Testing procedure

Setup an ESP-NOW network of two ESP32 nodes. For that purpose compile and flash two nodes with:

USEMODULE=esp_now make BOARD=esp32-wroom-32 -C examples/gnrc_networking flash

After that, connect to the nodes with a terminal program and use some commands like ifconfig and ping6 and wait. Without the changes in this PR, a CORRUPT message should come after several minutes. With the changes the same configuration should work stable.

Issues/PRs references

Fixes issue #11941.
Makes PR #11942 obsolete.

[gschorcht]

@JulianHolzwarth May I ask you to take look on this? This might be interesting for your work.

[JulianHolzwarth]

Thank you for letting me know.
It is interesting for my work and this change seems good to me.
Do you intend to change it to a spin lock in the future? (this comment: /* mutex_lock(mux); */ /* TODO should be only a spin lock */)
If not you could remove the parameter mux because it is not used and freertos does not have it in their official code (as far as I can see latest official release FreeRTOSv10.2.1/FreeRTOS/Source/tasks.c line 4220 void vTaskEnterCritical( void )in amazon github: link to line )
same for portENTER_CRITICAL.
(Maybe not this pr because changing all function calls could be a lot of work. What do you think?)

[gschorcht]

Do you intend to change it to a spin lock in the future? (this comment: /* mutex_lock(mux); */ /* TODO should be only a spin lock */)

Probably not, since a spin-lock would not improve the current mechanism and could block an ISR too long. I will remove the comment with the spin lock, but I would prefer to leave the /* mutex_lock ...*/ line and the comment above there for documentation purpose.

If not you could remove the parameter mux because it is not used and freertos does not have it in their official code (as far as I can see latest official release FreeRTOSv10.2.1/FreeRTOS/Source/tasks.c line 4220 void vTaskEnterCritical( void )in amazon github: link to line )
same for portENTER_CRITICAL.

I know, FreeRTOS doesn't define the mutex parameter. Unfortunatly, the ESP32 port of FreeRTOS does. And even more, SDK headers and SDK libraries require this interface. Therefore, I would leave this parameter there for the moment, even it is not used. When you have finished your FreeRTOS adaptation layer, we have to think about it again.

[JulianHolzwarth]

Ok. Thank you.

[benpicco]

vTaskEnterCritical() / vTaskExitCritical() are now just wrappers around irq_disable()/irq_restore().

Tested together with #11997 and ping -f.

[gschorcht]

@benpicco All lights are green 😄

[gschorcht]

@benpicco Thanks again for reviewing, testing and merging 😄