Skip to content

core: Make support for multi-tasking optional#11226

Closed
maribu wants to merge 5 commits intoRIOT-OS:masterfrom
maribu:single_tasking
Closed

core: Make support for multi-tasking optional#11226
maribu wants to merge 5 commits intoRIOT-OS:masterfrom
maribu:single_tasking

Conversation

@maribu
Copy link
Member

@maribu maribu commented Mar 21, 2019

Contribution description

Summary

Made multi-tasking support optional via module multi_tasking and enabled it by default. Added alternative implementations of parts of the thread API and the mutex API that are used when multi-tasking is disabled. These implementations heavily exploit the fact that only a single thread is present then.

To avoid resolving complex dependencies between core and architecture depend code in a single PR, this PR adds some workaround code to mock the scheduler API. This is not intended to stay

How to Use

Build with your application using

DISABLE_MODULE="multi_tasking core_msg" make

(core_msg needs to be disabled as well, as it depends on multi-tasking support
and will pull multi_tasking back in if not disabled as well.)

Concept

The idea is that most code not requiring multi-tasking support can be compiled unmodified with multi-tasking disabled. This can only be achieved by providing all RIOT APIs that can be used more or less reasonable in a single-tasking environment. The implementation of these APIs range from total mock versions (e.g. thread_yield()) that simple don't do anything to specialized implementations (e.g. mutex), that exploit the fact that code can only run in two contexts: In the single thread or in ISR context.

Outlook

Currently mock APIs for internal stuff like sched_context_switch_request and sched_task_exit() and sched_run() are also provided, as the platform specific code relies on their existence. By making the platform depended code aware of single-tasking, a lot of code could be elided there as well. This would ultimately allow to drop the mock APIs from sched.h for the single-tasking flavour of RIOT.

Numbers

(OUTDATED!)

Board text data bss
nucleo-f303re -812B -12B -2000B
arduino-uno -1664B +168B -874B

(Negative numbers sizes means disabling multi tasking resulted in lower memory footprint, positive means memory footprint increased. I compilied with BUILD_IN_DOCKER=1.)

Testing procedure

Everything but examples/hello-world should compile and run as nothing ever happened, as multi-tasking support is enabled by default. In examples/hello-world multi tasking support has been disabled, but it hopefully still compiles and runs as before

Issues/PRs references

#11199

Depends on and includes


Update 1: Updated description to match the code changes
Update 2: #11228 is merged, rebase against master is still needed

@maribu maribu added Area: core Area: RIOT kernel. Handle PRs marked with this with care! Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR labels Mar 21, 2019
@maribu
Copy link
Member Author

maribu commented Mar 21, 2019

Rebased to fix merge conflict

@kaspar030
Copy link
Contributor

nucleo-f303re -812B -12B -2000B
arduino-uno -1664B +168B -874B

The bss memory isn't saved, as it is mostly stack memory, which in the single-task case is not displayed.

800b code on arm, is that worth the extra complexity?
Why is it twice as much on avr?

@maribu
Copy link
Member Author

maribu commented Mar 21, 2019

The bss memory isn't saved, as it is mostly stack memory, which in the single-task case is not displayed.

This only partly true. The idle-thread is gone and so is the stack of the idle thread. Also the scheduler requires > 100B RAM with multi-tasking, and 2B / 4B (AVR/ARM) in single-threading.

E.g. on ARM Cortex M THREAD_STACKSIZE_DEFAULT is 1024B, but bss was reduced by 2000B. For ATmega THREAD_STACKSIZE_DEFAULT is 512B, but 706B less RAM (bss+data) is required.

Why is it twice as much on avr?

This is a good question. I didn't touch any platform specific code and did not look into any. If I had to guess, I would bet that the compiler was able to identify unreachable code in AVR in the single-tasked version, that is used by the scheduler in the multi-tasked version.

is that worth the extra complexity?

The stats on this PR look more frightening that the changes actually are. E.g. by splitting up core/include/thread.h into core/include/thread.h, core/multi_tasking/thread.h and core/single_tasking/thread.h the boiler plate code (license, doxygen, C++ compatibility) is thrice now. The actual code change are trivial.

@maribu
Copy link
Member Author

maribu commented Mar 21, 2019

I think it makes more sense to also have sched.h split into a common sched.h and two sched_backend.h. (This might increase the lines of text due to additional boilerplate, but the lines of code will be less.)

@maribu
Copy link
Member Author

maribu commented Mar 21, 2019

OK, I did so now and added a few additional workarounds to get MSP430 and PIC32 compiling. If this approach is considered for including, the platform specific code can be adapted to no longer need the workarounds, which could yield some additional bytes of ROM/RAM - or none if the compiler already elides such features as functions to save and restore context.

{
sched_active_thread = sched_threads[0];
/* FIXME: This line seems to be unneeded. Is this true? */
/* sched_active_thread = sched_threads[0]; */
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: Does this line of code have a purpose? sched_run() will set sched_active_thread anyway, right?

@maribu maribu requested a review from gschorcht March 21, 2019 12:32
@kaspar030
Copy link
Contributor

The stats on this PR look more frightening that the changes actually are.

Well, they look like most of core/ has two versions now.

@maribu, as is, I'll NACK this PR. Let's please first remove the (need for the) idle thread, then see from there.

@maribu
Copy link
Member Author

maribu commented Mar 21, 2019

Update: Added some #ifdef MODULE_CORE_MSG to xtimer to allow using it without core_msg.

@maribu
Copy link
Member Author

maribu commented Mar 21, 2019

Well, they look like most of core/ has two versions now.

You miscounted ;-) There are two versions of thread, mutex and rmutex now, with rmutex just being a wrapper for mutex in the single task case. The rest (sched) is a work around that is not intended to stay.

The things not having two versions are: assert, bitarithm, byteorder, cib, clist, debug, irq, kernel*, lifo, list, log, panic, priority_queue, ringbuffer, thread_flags, cond, mbox, and msg. (The last four are missing in single tasking mode.)

@maribu maribu changed the title Single tasking core: Make support for multi-tasking optional Mar 21, 2019
maribu added 4 commits March 21, 2019 20:04
`void cpu_switch_context_exit(void)` assigns `sched_active_thread` just before
calling `sched_run()`. This is unneeded, as `sched_run()` will updated that
anyway. Also generally speaking, changing internal scheduler data from outside
the scheduler is a risky thing to do.
@maribu
Copy link
Member Author

maribu commented Mar 21, 2019

Fore reference: The old state is backed up here: https://github.com/maribu/RIOT/tree/single_tasking_bak

@maribu
Copy link
Member Author

maribu commented Mar 21, 2019

@kaspar030

Let's please first remove the (need for the) idle thread, then see from there.

I agree that this is clearly beneficial I'll sure help with that. But I don't see the relation to this PR. Removing the idle thread will lower resource requirements in the multi-tasking scenario. But this PR tries to introduce a single-task scenario. Both ideas have the common goal of reducing resource requirements, but I do not see any dependency between them.

I changed this PR to keep the folder structure as is. I had to add a bit of preprocessor magic to mutex.h to allow the different implementations, but apart from that now all headers are at the same place and unmodified.

The mock scheduler stuff is also still in place to allow compiling on ARM/AVR/MSP430/MIPS. (For ESP the linker is fine with unresolved symbols, so I did not add workarounds there.) If this approach is considered for merging, the workarounds can be dropped once the architecture specific code is prepared for non-multi-tasking mode, which likely will shrink the firmware a bit more.

Having riotboot as use case in mind, there seem to be a need for this. Allowing a "single-threaded" mode with official APIs instead of overwriting kernel_init will make the code of riotboot more readable. This will also allow using additional RIOT modules out of the box, that are broken when simply overwriting kernel_init. Also, once the architecture depended context saving, context switching, thread initialization code and the code handling the termination of threads is changed to only be included with module multi_tasking added, riotboot will likely be even smaller than now.

@miri64: Could you have a look if this approach could be an alternative for #10971? The current implementation allows for void main(void) to exit and will go to the lowest power mode in an endless loop, while still serving interrupts.

@gschorcht
Copy link
Contributor

I'm wondering what someone's intention might be to use a multi-threaded system like RIOT if it does not require multitasking. Realizing a single-task system which can be integrated in a RIOT network can't be the intention since networking requires multi-threading. So, the only intention I see is that someone could want to use the pool of drivers. But is this really a use case which is worth to restructure the kernel and to adapt all MCU implementations?

At least for ESPs I can say, that they will never work as single task systems since SDK libraries already require multi-tasking.

@kaspar030
Copy link
Contributor

I'm wondering what someone's intention might be to use a multi-threaded system like RIOT if it does not require multitasking.

I think this would go further: the PR tries to make it possible to remove the multithreading parts.
I find the benefits becoming quite academic at some point.
The overhead of unused but present multitasking is <1k code, <50b RAM, and almost no runtime overhead.
Being able to remove that is, to me, a nice to have, but I wouldn't sacrifice much in terms of how core is laid out.

I think of it as in "what applications would benefit". none comes to mind.

@kaspar030
Copy link
Contributor

I agree that this is clearly beneficial I'll sure help with that. But I don't see the relation to this PR. Removing the idle thread will lower resource requirements in the multi-tasking scenario. But this PR tries to introduce a single-task scenario. Both ideas have the common goal of reducing resource requirements, but I do not see any dependency between them.

The idle thread is the single largest unneeded RAM consumer when multithreading is enabled but only a single thread is used, having it removed benefits all deployments and makes disabling multithreading less attractive.

@maribu
Copy link
Member Author

maribu commented Mar 22, 2019

@gschorcht: I agree that on the ESP platform it is impossible and also unreasonable, as even the ESP8266 have plenty of flash and RAM.

The idea is to allow specialized single purpose applications to be written with RIOT with minimal resource requirements. One use case is riotboot, as users will want to spend as little flash as possible for the boot loader. ~1KiB flash saving is really nice to have there.

@kaspar030
Copy link
Contributor

One use case is riotboot, as users will want to spend as little flash as possible for the boot loader. ~1KiB flash saving is really nice to have there.

riotboot is already using only a fraction of the multithreading code. It would benefit much more from much simpler changes which would also benefit a wider range of deployments. E.g., #10767 reduces its size from ~2.6K flash to 1.2k.

@gschorcht
Copy link
Contributor

gschorcht commented Mar 22, 2019

@maribu This PR might affect PRs #10934, #10943, and #10944.

They all rely on the fact that system is always operating on thread stacks or on the ISR stack, and the standard stack is no longer used once kernel_init is executed and the system is working in multi-threading mode. Therefore, they simply define the top of the heap as the current stack pointer minus some margin when calling kernel_init or cpu_switch_context_exit.

@maribu
Copy link
Member Author

maribu commented Mar 22, 2019

This PR might affect PRs #10934, #10943, and #10944.

I personally wouldn't take this PR into account for others, regardless if it will be considered for inclusion. If single threaded mode will be added, it will only be used in very specific scenarios where the user has to expect things to break. I personally would just add to the documentation, that malloc() cannot be used when multi-tasking is disabled. User of this mode don't want to use malloc() anyway...

@maribu maribu added the State: waiting for other PR State: The PR requires another PR to be merged first label Mar 25, 2019
@maribu
Copy link
Member Author

maribu commented Mar 25, 2019

It would benefit much more from much simpler changes which would also benefit a wider range of deployments.

Agreed. Approaches that can yield more benefit and apply to more use cases should get priority.

@maribu
Copy link
Member Author

maribu commented Jul 23, 2019

Let's close this PR for now. If there is still a reason to add this once other approaches to reduce RIOT's footprint for simple tasks have been merge, it can be opened again.

@maribu maribu closed this Jul 23, 2019
@maribu maribu deleted the single_tasking branch May 11, 2020 13:36
@benpicco
Copy link
Contributor

Now that no_idle_thread is in, would this be easier to achieve?
Just trying to squeeze down riotboot a bit.

@maribu
Copy link
Member Author

maribu commented Jan 27, 2021

Not really. With only a single thread, idling becomes rather trivial: Just call pm_set_lowest() where previously a scheduler run would take place.

If you want to push for this route, a series of smaller and less scary PRs might be what is needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: core Area: RIOT kernel. Handle PRs marked with this with care! Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR State: waiting for other PR State: The PR requires another PR to be merged first

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants