core: make idle thread optional #14224

kaspar030 · 2020-06-08T11:05:18Z

Contribution description

This PR makes the idle thread optional, if the architecture implements support for that.
In order to do that, an architecture needs to implement sched_arch_idle() and disable the module core_idle_thread.

The main benefits are:

saving a full thread context and stack
faster and shorter sleep transitions (no need to schedule idle thread first)

The PR includes an implementation for CortexM3 and higher (M0+ doesn't support dynamically changing IRQ priorities, which the current implementation relies on).

Testing procedure

Any threading application obviously still needs to work. Also, the pm machinery needs to work as before.

Any shell test should confirm that the scheduler is idling (not switching to any thread if there's no runnable) and reacting to ISR (react to uart input and re-schedule shell thread).

I tested power management using tests/periph_pm, confirming that unblocking the right mode for a while makes the shell unresponsive (power mode has been set), and that when the RTC interrupt triggers, shell works again.

edit the commit history looks a bit scary due to #13825 being included. The new logic of this PR is contained in the latest two commits (one for basic support in core, one for the cortexm implementation).

Issues/PRs references

This PR needs #13825, which re-writes the Cortex-M PendSV logic so it can handle the case where there's no thread to save on entry.

bergzand · 2020-06-08T12:21:53Z

This breaks the schedstatistics module 😢

maribu · 2020-06-08T12:22:25Z

disable the module core_idle_thread.

I would like to inverse the logic. Could we instead have a feature (and corresponding module) such as thread_less_idling, which would always be in FEATURS_OPTIONAL?

Providing a feature instead of disabling a module seems more natural to me.

kaspar030 · 2020-06-08T13:17:55Z

Providing a feature instead of disabling a module seems more natural to me.

done

kaspar030 · 2020-06-08T13:40:31Z

This breaks the schedstatistics module cry

This'll need reworking the sched_cb logic: sched_cb(sched_active_pid, next_thread->pid); expects ending of one and starting the next thread to happen at the same time.

kaspar030 · 2020-06-08T13:55:04Z

core/sched.c

@@ -80,11 +80,14 @@ int __attribute__((used)) sched_run(void)
 {
    sched_context_switch_request = 0;

+#ifndef MODULE_CORE_IDLE_THREAD
+    while (!runqueue_bitcache) {


note to self: as is, the compiler must assume that sched_arch_idle() calls a function that modifies runqueue_bitcache (it is static in this compilation unit). With LTO, it might come to a different result. need to make this atomic or volatile.

kaspar030 · 2020-06-08T14:32:48Z

I marked this WIP, as there's some code that expects that there's always an active thread (e.g., most of the IPC code due to sched_switch()).

fjmolinas · 2020-06-16T10:00:19Z

this one needs a rebase now :)

kaspar030 · 2020-06-17T10:11:47Z

I marked this WIP, as there's some code that expects that there's always an active thread (e.g., most of the IPC code due to sched_switch()).

It seems to me that all that code is actually called when within thread context, meaning that having no active thread won't happen. So I've removed WIP.

kaspar030 · 2020-06-25T14:16:46Z

Turns out the unittest hard fault was caused by too a too small stack for main. I've added a commit to increase it slightly. (stack is also small on master, depending on compiler, it hard faults there without increase. This was unnoticed as the crash happens after finishing the tests).

miri64 · 2020-06-26T07:36:46Z

This PR still needs two ACKs to be merged before soft feature freeze (which I will anounce in a few hours) ;-).

bergzand

first ACK!

fjmolinas

ACK!

bergzand · 2020-06-26T09:04:23Z

And go!

kaspar030 · 2020-06-26T09:22:38Z

And go!

Thanks for reviewing!

miri64 · 2020-06-26T09:25:48Z

Now let's rethink making the main thread optional 😜

With #14224 the idle thread became optional, so the main thread can have either 1 or 2 as PID, depending if the idle thread is included or not. As this might also change in the future, it is probably the best to just expect any number.

fjmolinas · 2020-07-15T15:14:43Z

Bisecting I have 0ff9e55 causing tests/xtimer_periodic_wakeup to get stuck on nucleo-l152re.

Testing interval 25... (now=4054509)
Testing interval 24... (now=4057901)
Testing interval 23... (now=4061293)
Testing interval 22... (now=4064686)
Testing interval 21... (now=4068078)
Testing interval 20... (now=4071461)
Testing interval 19... (now=4074844)
Testing interval 18... (now=4078229)
Testing interval 17... (now=4081612)
Testing interval 16... (now=4084996)
# gets stuck here

With RIOT-OS#14224 the idle thread became optional, so the main thread can have either 1 or 2 as PID, depending if the idle thread is included or not. As this might also change in the future, it is probably the best to just expect any number. (cherry picked from commit 5599b06)

bergzand · 2020-07-21T16:58:29Z

Totally not related to #14557, some measurements on the same54-xpro with bench_mutex_pingpong:

	idle thread	no_idle_thread	impact
GCC	262582	251572	4.1%
GCC+LTO	272109	252631	7.1%
LLVM	260303	244897	5.9%

kaspar030 requested review from maribu and bergzand June 8, 2020 11:05

kaspar030 requested review from DipSwitch, haukepetersen and thomaseichinger as code owners June 8, 2020 11:05

kaspar030 mentioned this pull request Jun 8, 2020

cpu/cortexm: "free" SVC #13825

Merged

kaspar030 force-pushed the cortexm_remove_idle_thread branch from 5804c65 to 64a7bbb Compare June 8, 2020 13:15

kaspar030 requested review from cgundogan, jia200x and leandrolanzieri as code owners June 8, 2020 13:15

kaspar030 commented Jun 8, 2020

View reviewed changes

kaspar030 added State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet and removed CI: run tests If set, CI server will run tests on hardware for the labeled PR labels Jun 8, 2020

kaspar030 mentioned this pull request Jun 8, 2020

tests/thread_flood: account for all threads already in use #14181

Merged

kaspar030 removed the State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet label Jun 17, 2020

kaspar030 force-pushed the cortexm_remove_idle_thread branch from d73879e to f922e2b Compare June 17, 2020 10:09

kaspar030 force-pushed the cortexm_remove_idle_thread branch from 34f0915 to 08d4d31 Compare June 25, 2020 14:12

tests/unittests: increase main stacksize to THREAD_STACKSIZE_LARGE

f85f782

kaspar030 force-pushed the cortexm_remove_idle_thread branch from 08d4d31 to f85f782 Compare June 25, 2020 14:13

kaspar030 added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jun 25, 2020

bergzand approved these changes Jun 26, 2020

View reviewed changes

fjmolinas approved these changes Jun 26, 2020

View reviewed changes

bergzand merged commit 279f2ae into RIOT-OS:master Jun 26, 2020

kaspar030 deleted the cortexm_remove_idle_thread branch June 26, 2020 09:22

miri64 mentioned this pull request Jul 15, 2020

examples/micropython: fix checked PID #14518

Merged

miri64 mentioned this pull request Jul 15, 2020

examples/micropython: fix checked PID [backport 2020.07] #14530

Merged

This was referenced Jul 16, 2020

cortexm_common: disable IRQ during thread_sched_idle #14534

Merged

cortexm_common: disable IRQ during thread_sched_idle [backport 2020.07] #14537

Merged

cortexm_common: Make no_thread_idle compatible with cortex-m0 #14557

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

core: make idle thread optional #14224

core: make idle thread optional #14224

kaspar030 commented Jun 8, 2020 •

edited

Loading

bergzand commented Jun 8, 2020

maribu commented Jun 8, 2020

kaspar030 commented Jun 8, 2020

kaspar030 commented Jun 8, 2020

kaspar030 Jun 8, 2020

kaspar030 commented Jun 8, 2020

fjmolinas commented Jun 16, 2020

kaspar030 commented Jun 17, 2020

kaspar030 commented Jun 25, 2020

miri64 commented Jun 26, 2020

bergzand left a comment

fjmolinas left a comment

bergzand commented Jun 26, 2020

kaspar030 commented Jun 26, 2020

miri64 commented Jun 26, 2020 •

edited

Loading

fjmolinas commented Jul 15, 2020 •

edited

Loading

bergzand commented Jul 21, 2020

core: make idle thread optional #14224

core: make idle thread optional #14224

Conversation

kaspar030 commented Jun 8, 2020 • edited Loading

Contribution description

Testing procedure

Issues/PRs references

bergzand commented Jun 8, 2020

maribu commented Jun 8, 2020

kaspar030 commented Jun 8, 2020

kaspar030 commented Jun 8, 2020

kaspar030 Jun 8, 2020

Choose a reason for hiding this comment

kaspar030 commented Jun 8, 2020

fjmolinas commented Jun 16, 2020

kaspar030 commented Jun 17, 2020

kaspar030 commented Jun 25, 2020

miri64 commented Jun 26, 2020

bergzand left a comment

Choose a reason for hiding this comment

fjmolinas left a comment

Choose a reason for hiding this comment

bergzand commented Jun 26, 2020

kaspar030 commented Jun 26, 2020

miri64 commented Jun 26, 2020 • edited Loading

fjmolinas commented Jul 15, 2020 • edited Loading

bergzand commented Jul 21, 2020

kaspar030 commented Jun 8, 2020 •

edited

Loading

miri64 commented Jun 26, 2020 •

edited

Loading

fjmolinas commented Jul 15, 2020 •

edited

Loading