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

Allow the FreeRTOS config configIDLE_SHOULD_YIELD to be configurable (IDFGH-14431) #15211

Open
ESP-Marius opened this issue Jan 15, 2025 · 2 comments
Assignees
Labels
Status: Opened Issue is new Type: Feature Request Feature request for IDF

Comments

@ESP-Marius
Copy link
Collaborator

Is your feature request related to a problem?

If creating tasks with the same priority as the IDLE task then these will run in a round-robin fashion together with the IDLE task. But since configIDLE_SHOULD_YIELD=0 in IDF this wont be very efficient as the IDLE task will occupy its share of the round-robin ticks, even though in many cases we would would rather make the idle task yield and prioritize these other tasks at the same priority.

Describe the solution you'd like.

Make the internal freertos config configIDLE_SHOULD_YIELD configurable as a kconfig config.

Describe alternatives you've considered.

No response

Additional context.

Split from #8345

Comment from @RichFalk #8345 (comment)

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 ESP-Marius added the Type: Feature Request Feature request for IDF label Jan 15, 2025
@github-actions github-actions bot changed the title Allow the FreeRTOS config configIDLE_SHOULD_YIELD to be configurable Allow the FreeRTOS config configIDLE_SHOULD_YIELD to be configurable (IDFGH-14431) Jan 15, 2025
@espressif-bot espressif-bot added the Status: Opened Issue is new label Jan 15, 2025
@RichFalk
Copy link
Contributor

@ESP-Marius Thank you for splitting this off from my comment on the other issue. Should I create separate issues/requests for the other RTOS-related items, namely 1) ESP32-C3 default serial/JTAG driver being nonblocking instead of blocking, 2) changing RF cert test code to use blocking/interrupts, 3) having Flash erase use blocking/interrupts (if possible; might be that hardware doesn't support this)?

I have a slew of bugs when moving from ESP-IDF v4 to v5 and will be opening issues for each. They are mostly related to BLE or interactions of BLE with Wi-Fi. Leftover BLE timers that won't go away even with deinit and cause excessive idle current (though interestingly go away if Wi-Fi connects), excessive interrupt delays killing our LEDC-driven motor code, out-of-order data and connection events, and generally excessively long BLE wake time (though that latter one was in v4 as well). I'll create reproducible cases run on the ESP32-C3-DevKitM-1U Development Board for each of these.

@ESP-Marius
Copy link
Collaborator Author

@RichFalk Yeah, anything you would want a response on/fix to is better to open a separate issue for.

Areas are handled by separate teams and engineers, so by listing a lot of issues in a single issue we are unfortunately indeed more likely to miss something.

Thanks again for your initial comment, your point of view does indeed make sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Opened Issue is new Type: Feature Request Feature request for IDF
Projects
None yet
Development

No branches or pull requests

4 participants