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

Decide if we keep a single thread support (CONFIG_MULTITHREADING=n) in Zephyr #27415

Closed
3 of 4 tasks
carlescufi opened this issue Aug 6, 2020 · 16 comments
Closed
3 of 4 tasks
Assignees
Labels
Meta A collection of features, enhancements or bugs

Comments

@carlescufi
Copy link
Member

carlescufi commented Aug 6, 2020

This issue contains the information required in order for us to make an informed decision on whether we should keep supporting a non-multithreaded mode (CONFIG_MULTITHREADING=n) in Zephyr for the foreseeable future.

If we decide to keep it, then the scope of the non-multithreaded mode shall be:

  • Build system
  • Boot to main()
  • Board support (for booting)
  • Interrupt handling (including irq_lock() and irq_unlock()
  • k_busy_wait() and k_uptime_get()
  • A subset of driver classes, and within them only those that have been written with non-multithreaded mode in mind:
    • flash
    • uart
    • gpio

The requirements for maintaining support for this mode in the tree are:

Related issues:

Bugs:

Related PRs:

@carlescufi
Copy link
Member Author

carlescufi commented Aug 6, 2020

Measurements

All measurements taken using HEAD in this branch (includes the fix in #27343)

Only booting

  • No multithreading: west build -b reel_board -t r{a,o}m_report samples/basic/minimal/ -- -DCONF_FILE="common.conf no-mt.conf no-timers.conf arm.conf"
  • Multithreading: west build -b reel_board -t r{a,o}m_report samples/basic/minimal/ -- -DCONF_FILE="common.conf mt.conf no-timers.conf arm.conf"
RAM ROM
NO MT 3553 2508
MT 3989 3722
Difference +436 +1214

MCUBoot

  • No multithreading: west build -t r{a,o}m_report -- -DCONF_FILE="prj.conf no-mt.conf"
  • Multithreading: west build -t r{a,o}m_report -- -DCONF_FILE="prj.conf mt.conf"
RAM ROM
NO MT 17497 14126
MT 17925 15768*
Difference +428 +1642*

*around 504 bytes of those are caused by the addition of k_sem operations in the Nordic flash driver when multithreading is enabled, but those could be removed for MCUBoot even with multithreading enabled, because MCUBoot is only ever using main() and no additional threads. If we disabled those semaphore ops based on some IS_BOOTLOADER option, then the difference would be exactly 1100 bytes.

@MaureenHelm MaureenHelm added the Meta A collection of features, enhancements or bugs label Aug 6, 2020
@andrewboie
Copy link
Contributor

andrewboie commented Aug 10, 2020

Passing comment about semaphores, as this has been a pain point and the subject of a few discussions at least in the past.

More than a few drivers have their ISR give a semaphore on some event, taken by something running in thread context waiting for the event to occur. Or if the driver doesn't do it, an installed callback would.

We could possibly do something that would work in a lot of cases if there's no threading or scheduling:

  • An alternate implementation of k_sem_take() which simply spins on the semaphore count with interrupts unlocked until it is nonzero with something like atomic_cas(), and atomically decrements it.
  • An alternate implementation of k_sem_give() which simply atomically increments the semaphore count

This would allow for the main execution context to block until we get event that triggers an ISR. I think this would allow more (definitely not all) drivers to be used in this configuration.

@carlescufi
Copy link
Member Author

Passing comment about semaphores, as this has been a pain point and the subject of a few discussions at least in the past.

While your comment is indeed valuable if we decide to keep CONFIG_MULTITHREADING, the reference to semaphores in this comment is related to using them as lightweight mutexes, something that was discussed also here

@andrewboie
Copy link
Contributor

Passing comment about semaphores, as this has been a pain point and the subject of a few discussions at least in the past.

While your comment is indeed valuable if we decide to keep CONFIG_MULTITHREADING, the reference to semaphores in this comment is related to using them as lightweight mutexes, something that was discussed also here

I think what I'm discussing is more or less orthogonal to that. Currently I don't think you can use semaphores at all if multi threading is disabled. This would allow that (or something similar) using atomic operations for the specific case where we need to block the main execution until we get some external event from an interrupt.

@carlescufi
Copy link
Member Author

Passing comment about semaphores, as this has been a pain point and the subject of a few discussions at least in the past.

While your comment is indeed valuable if we decide to keep CONFIG_MULTITHREADING, the reference to semaphores in this comment is related to using them as lightweight mutexes, something that was discussed also here

I think what I'm discussing is more or less orthogonal to that. Currently I don't think you can use semaphores at all if multi threading is disabled. This would allow that (or something similar) using atomic operations for the specific case where we need to block the main execution until we get some external event from an interrupt.

Agreed, it is orthogonal. The question is whether it's worthwhile going down the road you propose given that raw multithreading today is barely 1.1KB of ROM.

@andrewboie
Copy link
Contributor

Agreed, it is orthogonal. The question is whether it's worthwhile going down the road you propose given that raw multithreading today is barely 1.1KB of ROM.

Ah, now I see. Yes that is the real question! My feeling is "no" if that's all we save and we are confident we're not pulling in any code we don't need.

@nashif
Copy link
Member

nashif commented Aug 11, 2020

Agreed, it is orthogonal. The question is whether it's worthwhile going down the road you propose given that raw multithreading today is barely 1.1KB of ROM.

need to move forward with this, @MaureenHelm should the finding above be presented to the TSC? Do we need to vote on removing this feature and shift focus on improving footprint in general?

@ruuddw
Copy link
Member

ruuddw commented Aug 12, 2020

As just mentioned in the TSC meeting: another use-case that we are considering for ARC is use the no-multithreading version for a simple Secure binary (handling boot and secure services like Crypto). Multi-threading on the secure side could make things more complex than required for most cases. And simple typically also means more secure...

@gregshue
Copy link

gregshue commented Aug 12, 2020

@ruuddw, what latencies are introduced to non-secure handling (ISRs, meta-ISRs, and threads) while executing a secure service (e.g. Crypto)?

@ruuddw
Copy link
Member

ruuddw commented Aug 13, 2020

@ruuddw, what latencies are introduced to non-secure handling (ISRs, meta-ISRs, and threads) while executing a secure service (e.g. Crypto)?

Good question - don't have a complete answer yet. But, I think we would like the Secure services to be preemptible/cooperative: no multithreading within Secure mode, but the Secure 'context' can be swapped out to Normal, e.g. also while Secure context is waiting for a HW crypto engine result.

The additional footprint is probably less of an issue, it's the complexity with scheduling on both sides why I'd like to stick to a single thread on the secure side.

@carlescufi
Copy link
Member Author

Decision in the TSC: Deprecate in the 2.4 release for this to be removed in the 2.6 release unless someone steps up to maintain the feature.

@raveslave
Copy link

great to see the ram/rom footprint comparison

anyone done any performance tests with a "single threaded multi thread" app vs a non multithreaded one, basicaly doing the same thing, ie hw timer firing an irq -> process stuff -> write to uart -> wait

carlescufi added a commit to carlescufi/zephyr that referenced this issue Sep 22, 2020
Deprecate the Kconfig option for the time being. Unless a contributor
volunteers to take over the work to maintain the option, it will be
removed after 2 releases.

Relates to zephyrproject-rtos#27415.

Signed-off-by: Carles Cufi <carles.cufi@nordicsemi.no>
MaureenHelm pushed a commit that referenced this issue Sep 23, 2020
Deprecate the Kconfig option for the time being. Unless a contributor
volunteers to take over the work to maintain the option, it will be
removed after 2 releases.

Relates to #27415.

Signed-off-by: Carles Cufi <carles.cufi@nordicsemi.no>
@nashif nashif changed the title Decide if we keep a CONFIG_MULTITHREADING mode in Zephyr Decide if we keep a single thread support (CONFIG_MULTITHREADING=n) in Zephyr Oct 1, 2020
@nvlsianpu
Copy link
Collaborator

If Single Thread mode (CONFIG_MULTITHREADING=n) will be deprecated then:
Can be introduced a mode without synchronization mechanism like semaphores, mutexes etc?
Such mode will be useful for small footprint application (like mcuboot), where application will use only one thread anyway.

@pabigot pabigot self-assigned this Oct 5, 2020
@carlocaione
Copy link
Collaborator

Just my 2 cents here. CONFIG_MULTITHREADING=n was very useful to me when first porting Zephyr to ARM64 to reduce complexity and to start "small".

@nordic-krch
Copy link
Contributor

I would like to keep single thread support in zephyr. I started to work towards it (see #34279).

@carlescufi
Copy link
Member Author

Closing this since this is now complete.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Meta A collection of features, enhancements or bugs
Projects
None yet
Development

No branches or pull requests