Skip to content
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.

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

Parametrize some FreeRTOS config fields (configIDLE_SHOULD_YIELD and configUSE_PREEMPTION) (IDFGH-6716) #8345

Closed
carlessole opened this issue Feb 4, 2022 · 11 comments
Labels
Resolution: Won't Do This will not be worked on Status: Done Issue is done internally Type: Feature Request Feature request for IDF

Comments

@carlessole
Copy link

Module or chip used: ESP32-WROVER (With ESP_PROG)
IDF version: v4.3
Build System: CMake/idf.py (usingEclipse Plugin)
Compiler version: esp-2020r3-8.4.0
Operating System: Windows
(Windows only) environment type: -.
Using an IDE?: Yes. Eclipse + ESP-IDF plugin
Power Supply: USB

Hello,
I have seen that almost all the freeRTOS config fields (defined in FreeRTOSConfig.h) are hardcoded and can not be modified (not by sdkconfig and as far as I know, this file is recommended to not modify it, right?).

I see that in master branch is the same: https://github.com/espressif/esp-idf/blob/master/components/freertos/esp_additions/include/freertos/FreeRTOSConfig.h

Is there any idea to support/let modify some config fields? I would like to be able to modify the:

As for example:

#define configTICK_RATE_HZ ( CONFIG_FREERTOS_HZ )

Thanks

@carlessole carlessole added the Type: Feature Request Feature request for IDF label Feb 4, 2022
@espressif-bot espressif-bot added the Status: Opened Issue is new label Feb 4, 2022
@github-actions github-actions bot changed the title Parametrize some FreeRTOS config fields (configIDLE_SHOULD_YIELD and configUSE_PREEMPTION) Parametrize some FreeRTOS config fields (configIDLE_SHOULD_YIELD and configUSE_PREEMPTION) (IDFGH-6716) Feb 4, 2022
@Dazza0
Copy link
Contributor

Dazza0 commented Feb 7, 2022

@carlessole

There are some IDF system level considerations that force most (if not all) FreeRTOS configurations to be hardcoded. We've already made menuconfigurable what we can.

  • configIDLE_SHOULD_YIELD will be tricky. I'll need to check how it interacts with configUSE_TICKLESS_IDLE as it may prevent the IDLE task from putting the system into light sleep.
  • configUSE_PREEMPTION 0 might break a lot of higher level components if they are written with the assumption that preemption is always enabled. It's rare to disable preemption in FreeRTOS as it basically makes the scheduler no longer Real Time.

@carlessole
Copy link
Author

Thanks for the answer @Dazza0 .

I understand the configUSE_PREEMPTION dependency on many higher level code.

Regarding the configIDLE_SHOULD_YIELD for my needs it would be great to have that possibility. Without it what is happening is that once an IDLE priority TASK will not be executed after one FreeRTOS tick once sched_yield has been called on it.

I have tried (in order to avoid changing the configIDLE_SHOULD_YIELD) to use an IDLE hook an perform an sched_yield from there, but as it is normal, the behaviour of my code was quite undefined.

imatge

imatge

https://docs.espressif.com/projects/esp-idf/en/release-v3.0/api-reference/system/hooks.html#_CPPv431esp_register_freertos_idle_hook22esp_freertos_idle_cb_t

I'm assuming that the warnings are already preventing to use 'sched_yields' and similars in this hooks.

Carles

@carlessole
Copy link
Author

This issue is related with: #7256

I don't know if the fix that you have introduced in version 4.4

imatge

has already solving it (configIDLE_SHOULD_YIELD is still not configurable):

Thanks

@carlessole
Copy link
Author

Any news on that?
Thanks

@o-marshmallow
Copy link
Collaborator

Hi @carlessole ,

There is a way to force the macros that you want when compiling ESP-IDF without modifying ESP-IDF. Let's take the example hello-world in esp-idf/example/get-started/hello_world/.

Edit its CMakeLists.txt to add the following line:

add_compile_options(-include ${CMAKE_CURRENT_SOURCE_DIR}/main/override.h)

Right after include, before project(hello_world)

This will force the inclusion of the file main/override.h inside all the source files compiled in IDF. This file will be included at first, so before any other includes the source file may contain.

Thus, we need to create that file, main/override.h, with the following content inside:

#pragma once

// Force the inclusion of FreeRTOS config to make sure we can redefine any macro right after
#include "freertos/FreeRTOSConfig.h"

// New value for configIDLE_SHOULD_YIELD
#undef configIDLE_SHOULD_YIELD
#define configIDLE_SHOULD_YIELD                         1

Now you can compile IDF, your new macro will be taken into account for all sources, including FreeRTOS'

@Dazza0
Copy link
Contributor

Dazza0 commented Sep 28, 2022

@carlessole I'm still hesitant to expose configUSE_PREEMPTION and configIDLE_SHOULD_YIELD as Kconfig options due to the reasons mentioned above (i.e., lack of preemption and idle yielding could cause other higher level components to break). Could you elaborate further as to your use case for wanting to set configUSE_PREEMPTION = 0 and configIDLE_SHOULD_YIELD = 1. Maybe there's another way to achieve the same thing.

I'm also open to allowing users to override FreeRTOS config (but not exposing them as Kconfig options). One method is described above by @o-marshmallow. But, adding something like user provided #include "FreeRTOSConfigOverrides.h" into FreeRTOSConfig.h so that users can undef/define is also an option.

@carlessole
Copy link
Author

@o-marshmallow Thanks!! It is really a nice way to override some symbols in ESP-IDF without modifying its source code :). But anyway, I need to be sure that if 'configIDLE_SHOULD_YIELD' is '1', all the architecture is readyy to support that.

@carlessole
Copy link
Author

carlessole commented Sep 28, 2022

@Dazza0 I understand it.

I will try to expose my reasons to wanting that.

Main reason is that we are migrating many existing projects from another microcontroller to ESP32. Then all the code is already done and it is expecting a certain scheduler behavior. They are big projects that involves many and many files.

Then, regarding 'configUSE_PREEMPTION': All the existing code supposes that if no 'delay(n)' or 'sched_yield()' is called, no one can take the control of a running task. Then many data structures shared between tasks are not well protected with 'mutexes' or 'semaphores' as 'configUSE_PREEMPTION = 0' was a precondition.

That has been solved protecting them with 'mutexes' and 'semaphores', but anyway keeping the same behavior as the old scheduler (which one has been taken as reference to develop ALL the existing code) should be very nice for us. As I said I understand that many ESP-IDF components have been created with 'configUSE_PREEMPTION = 1' in mind, so, not big deal about not being able to change this option ('configUSE_PREEMPTION')

Regarding 'configIDLE_SHOULD_YIELD': The existing projects have many tasks running (about 20). None of them perform a 'delay(n)', they just perform and 'sched_yield()', that means that once a tasks gives away the uP control, the task is immediately ready again to enter (it is placed in the scheduler queue). To achieve that all the tasks have the same priority (the IDLE task priority). The refresh time of each task has been calculated with oscilloscope and in the old platform it was OK as there were no tasks that keeps running too much time.
But when 'configIDLE_SHOULD_YIELD = 0' (like it is in ESP-IDF), then each time all our tasks have performed its job and they have been placed in the scheduler queue to enter again ASAP, it takes at least a freeRTOS tick (fastest configuration is 1ms) to be running again as IDLE tasks are keeping the execution until a freeRTOS tick . See this issue for more details.
For example, we have many tasks that are reading the UART input with polling it and they have the requirement to be able to answer with an ACK very very fast (less than 3 ms). With current ''configIDLE_SHOULD_YIELD = 0' we see that the responsiveness of our code is much more slower and in some cases we are not matching the needs of the sytem where our devices are installed.

So it is very important that IDLE tasks can yield once they have finished their job and not waiting until a freeRTOS tick happens (see here).

@carlessole
Copy link
Author

@Dazza0 Any feedback on that?

Thanks!

@espressif-bot espressif-bot added Status: Selected for Development Issue is selected for development and removed Status: Opened Issue is new labels Apr 13, 2023
@RichFalk
Copy link
Contributor

RichFalk commented Jan 10, 2025

Why has this not been worked on and is now unassigned?
Especially for the configIDLE_SHOULD_YIELD setting which will have no problem being a configurable parameter via menuconfig and really should have its default be 1 which is the FreeRTOS default.
The concern that it may "prevent the IDLE task from putting the system into light sleep" when using configUSE_TICKLESS_IDLE is a misplaced one. All the yield parameter does when set to 0 is to not call yield in the idle task. It is not related to tickless idle nor going to sleep.

Specifically the tickless idle will only go to sleep when "The Idle task is the only task able to run because all the application tasks are either in the Blocked state or in the Suspended state" and the sleep will exceed the minimum based on configEXPECTED_IDLE_TIME_BEFORE_SLEEP. Since sleep is only done when all application tasks are blocked or suspended, this means that a yield will do nothing so the setting of whether or not the idle task does a yield is irrelevant.

ESP-IDF changing the value of configIDLE_SHOULD_YIELD from the FreeRTOS value of 1 to the ESP-IDF setting of 0 was a very bad decision. It breaks the cooperative multi-tasking fallback RTOS rule which is that tasks that don't follow RTOS convention by using interrupts/blocking and instead use yield should all be placed at priority 0 since yield will not give any time to lower priority tasks.

There are parts of ESP-IDF code that are not RTOS compliant yet cannot currently be run as priority 0 tasks because of this wrong configuration setting for idle yield. One example is in examples/phy/cert_test where some RF commands require running and do a frequent yield but should not be delayed in running or they will not work properly. The cert_test code sets the priority of tasks to 2 but that requires the task watchdog timer to be disabled (which that example project says to do) which is the wrong way to address this. If the RF task were at priority 0, it yields to the idle task properly so the watchdog timer gets tickled/fed but the ESP-IDF setting of configIDLE_SHOULD_YIELD to 0 has the idle task run until the next tick so hogs much too much time causing the RF output to be wobbly. Of course, the RF code should really be using calls that suspend/block the task until woken by an interrupt such as a timer for when they need to run again (say, to handle the next RF packet when sending them at some delay interval), but that's a separate issue.

A related problem with ESP-IDF not being RTOS compliant is for the ESP32-C3 where for some reason the USB serial/JTAG is using a non-blocking driver instead of using a blocking driver. That leads to bad loop code polling serial if one is parsing for such input. Yet the console component installs a blocking driver which is the right thing to do where one would use a blocked read in a task (so would get blocked) so why isn't this the default so that developers are given strong incentive to write RTOS compliant code?

FYI, the ONLY reason to set configIDLE_SHOULD_YIELD to 0 is if one has tasks that do not yield but rather need/want an equal amount of time for each task, specifically 1 tick, going in round-robin. But this is not at all the normal way to run and is most certainly not RTOS-compliant. It's fine to have this as a menuconfig option in case one has pure time-slicing code that is not at all cooperative, but otherwise the idle task should yield which will support cooperative multi-tasking at priority 0. And, of course, the right thing to do is to have all code be RTOS-compliant which means no yielding at all and only doing suspending or blocking.

Also FYI, the only other place I found (so far) in ESP-IDF code that is not RTOS-compliant is in flash erase that uses a vTaskDelay(1) call in a polling loop looking at a status register. Here again there should be an interrupt when Flash is done erasing so one should be blocking until that interrupt. Nevertheless, this vTaskDelay is still better than a yield that won't give lower priority tasks any time. It's just an inefficient forced blocking for a 1 tick interval.

@ESP-Marius
Copy link
Collaborator

After some discussion we agree that having configIDLE_SHOULD_YIELD as a configurable option sounds reasonable, i've split this out into a separate feature-request in !15211 to avoid entangling these two separate features/discussions.

For the reasons already stated, we do not at this moment plan to support turning of preemption, but please feel free to continue leaving comments/thumbsups in this issue if you feel this is an important feature. If there is a big demand for this then we can always reconsider the request at a later date.

@espressif-bot espressif-bot added Status: Done Issue is done internally Resolution: Won't Do This will not be worked on and removed Status: Selected for Development Issue is selected for development labels Jan 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: Won't Do This will not be worked on Status: Done Issue is done internally Type: Feature Request Feature request for IDF
Projects
None yet
Development

No branches or pull requests

6 participants