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.

Sign up for GitHub

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

Jump to bottom

remove single thread support #9808

Closed
nashif opened this issue Sep 5, 2018 · 8 comments
Closed

remove single thread support #9808

nashif opened this issue Sep 5, 2018 · 8 comments
Labels
Enhancement Changes/Updates/Additions to existing features priority: medium Medium impact/importance bug

Comments

@nashif
Copy link
Member

nashif commented Sep 5, 2018

Single thread support was added to reduce the footprint for use in mcuboot which did not require any multithreading or synchronisation.
Meanwhile we are adding subsystems and drivers into mcuboot that depend on multithreading features that makes single-thread support redundant.

Reduce complexity and provide just multi-threading support and focus on reducing footprint for all users.
@nashif nashif added the Enhancement Changes/Updates/Additions to existing features label Sep 5, 2018
@nashif nashif added this to the v1.14.0 milestone Sep 5, 2018
@nashif nashif added the priority: high High impact/importance bug label Sep 5, 2018
nashif added a commit to nashif/zephyr that referenced this issue Sep 6, 2018
@nashif
Revert "kernel: Disable tick handling when !MULTITHREADING"
965e1cf 
This reverts commit 8dcd5f8.

Single thread keep introducing more issues, decided to remove the
feature completely and push any required changes for after 1.13.

See zephyrproject-rtos#9808

Signed-off-by: Anas Nashif <anas.nashif@intel.com>
@nashif nashif mentioned this issue Sep 6, 2018
Merged
nashif added a commit to nashif/zephyr that referenced this issue Sep 6, 2018
@nashif
Revert "kernel: Enable interrupts for MULTITHREADING=n on supported a…
c31d7e0 
…rch's"

This reverts commit 17e9d62.

Single thread keep introducing more issues, decided to remove the
feature completely and push any required changes for after 1.13.

See zephyrproject-rtos#9808

Signed-off-by: Anas Nashif <anas.nashif@intel.com>
nashif added a commit that referenced this issue Sep 6, 2018
@nashif
Revert "kernel: Disable tick handling when !MULTITHREADING"
3d246c0 
This reverts commit 8dcd5f8.

Single thread keep introducing more issues, decided to remove the
feature completely and push any required changes for after 1.13.

See #9808

Signed-off-by: Anas Nashif <anas.nashif@intel.com>
nashif added a commit that referenced this issue Sep 6, 2018
@nashif
Revert "kernel: Enable interrupts for MULTITHREADING=n on supported a…
6b6ecc0 
…rch's"

This reverts commit 17e9d62.

Single thread keep introducing more issues, decided to remove the
feature completely and push any required changes for after 1.13.

See #9808

Signed-off-by: Anas Nashif <anas.nashif@intel.com>
Josh0027 pushed a commit to Josh0027/zephyr that referenced this issue Sep 10, 2018
@nashif @Josh0027
Revert "kernel: Disable tick handling when !MULTITHREADING"
e9e59ce 
This reverts commit 8dcd5f8.

Single thread keep introducing more issues, decided to remove the
feature completely and push any required changes for after 1.13.

See zephyrproject-rtos#9808

Signed-off-by: Anas Nashif <anas.nashif@intel.com>
Josh0027 pushed a commit to Josh0027/zephyr that referenced this issue Sep 10, 2018
@nashif @Josh0027
Revert "kernel: Enable interrupts for MULTITHREADING=n on supported a…
88a620d 
…rch's"

This reverts commit 17e9d62.

Single thread keep introducing more issues, decided to remove the
feature completely and push any required changes for after 1.13.

See zephyrproject-rtos#9808

Signed-off-by: Anas Nashif <anas.nashif@intel.com>
@nashif nashif mentioned this issue Sep 14, 2018
Closed
@nvlsianpu
Copy link
Collaborator

Meanwhile we are adding subsystems and drivers into mcuboot that depend on multithreading features

@nashif Which one? Recently with @carlescufi we eliminated all multithreading dependencies in mcuboot@zephyr - maybe I don't know about something.

cc @utzig @d3zd3z

@Olivier-ProGlove
Copy link
Contributor

Zephyr MCUBoot variant relies on Zephyr drivers. There is no guarantee Zephyr drivers do not use Zephyr kernel API that relies on multithreading support. see my pull-request for the discussion: #8368

We have three solutions:

  • Use MULTITHREADING=y for MCUBoot Zephyr's backend - current direction followed by @nashif
  • Disable Zephyr kernel API that are not suitable for MULTITHREADING=n
  • Add a big warning next to MCUBoot Zephyr's backend to leave the developer to carefully review the Zephyr drivers, he/she uses.

@Olivier-ProGlove Olivier-ProGlove mentioned this issue Sep 17, 2018
Closed
@Olivier-ProGlove
Copy link
Contributor

@nvlsianpu Funnily enough I had this issue today #10028 after rebasing on Zephyr v1.13...

@dcpleung dcpleung mentioned this issue Sep 27, 2018
Closed
dcpleung added a commit to dcpleung/zephyr that referenced this issue Sep 28, 2018
@dcpleung
kernel: remove single thread support
85eb3bb 
This removes single thread supports. Changes are done to
remove CONFIG_MULTITHREADING and #ifdef enclosed by it.

This includes the following reverts which can be trivially
applied:

Revert "drivers: flash: w25qxxdv: Avoid locking when not threaded"
This reverts commit 392da5b.

Revert "drivers: flash: nrf: Avoid locking when not threaded"
This reverts commit 408ea14.

Revert "tests/kernel/threads/no-multithreading: Disable USERSPACE"
This reverts commit 2f95e24.

Revert "tests/kernel: Add a test for CONFIG_MULTITHREADING=n"
This reverts commit 7bbd3a7.

Revert "kernel: Final spin in !MULTITHREADING should be locked"
This reverts commit 8daafd4.

Revert "kernel: Restore CONFIG_MULTITHREADING=n behavior"
This reverts commit 3d14615.

Revert "ext: simplelink: host driver: depend on multithreading"
This reverts commit 4d912b0.

Relates to zephyrproject-rtos#9808

Signed-off-by: Daniel Leung <daniel.leung@intel.com>
@pfalcon
Copy link
Contributor

pfalcon commented Sep 28, 2018 •
edited

Ok, so maybe mcuboot is better off being multithreaded. But why remove this cute feature? What complexity is being talked about? https://github.com/zephyrproject-rtos/zephyr/pull/10275/files hardly simplifies much.

@d3zd3z
Copy link
Collaborator

d3zd3z commented Oct 19, 2018

Also, just because some particular target may have a driver that needs multi threading, doesn't keep people from designing targets that are small, and don't have these requirements. There isn't just one configuration of MCUboot, and it needs to still be able to support very small targets, and work without threading support.

@galak galak added priority: medium Medium impact/importance bug and removed priority: high High impact/importance bug labels Jan 8, 2019
@nashif nashif removed this from the v1.14.0 milestone Mar 4, 2019
@andrewboie andrewboie mentioned this issue Mar 15, 2019
Closed
@pabigot pabigot mentioned this issue Jul 22, 2020
Closed
@pabigot
Copy link
Collaborator

pabigot commented Jul 23, 2020

See also ongoing discussion around #8393 (comment)

@pabigot
Copy link
Collaborator

pabigot commented Jul 23, 2020

After reviewing the history and doing some testing I'm swayed by the point made somewhere that CONFIG_MULTITHREADING=n is essentially an untested configuration of Zephyr that may expose arbitrary problems, both in the existing code and in whatever gets merged in the future. I think this is a risky path to take, so I'm inclined to support removing the capability.

@pabigot pabigot mentioned this issue Aug 3, 2020
Closed
@carlescufi carlescufi mentioned this issue Aug 6, 2020
Closed
4 tasks
@carlescufi
Copy link
Member

Closing in favor of #27415

@nashif nashif mentioned this issue Feb 1, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Enhancement Changes/Updates/Additions to existing features priority: medium Medium impact/importance bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
8 participants
@d3zd3z @nashif @pfalcon @pabigot @carlescufi @galak @nvlsianpu @Olivier-ProGlove