kernel: Add mutex wrapper for single thread app #63935

nandojve · 2023-10-13T19:28:16Z

There are subsystems in Zephyr that require synchronization primitives because the default behavior of Zephyr is to build multi-thread applications. This add very small change in the kernel related to mutexes that affects only single thread build. ~~It add a new file only to wrap the API.~~ The change define the k_mutex as an empty struct and the lock/unlock functions are bypassed. This allows few subsystem be used in single thread application without touch the subsystem code.

The change keeps Zephyr flexible and help to stay with very low code and sram profile for the very constrained applications that still require some functionality like Non-Volatile Storage.

With this change is still possible to build a single thread application that uses:

serial
timers
flash driver and layout
Non-Volatile Storage NVS
pinctrl
clocks
ring buffers

Prove of Concept:

Microcontroller: STM32L010X4
Main thread: 512 bytes
Transmission buffers: 2 x 128 bytes
Serial protocol for communication

Memory region         Used Size  Region Size  %age Used
           FLASH:       14060 B        15 KB     91.53%
             RAM:        1480 B         2 KB     72.27%

The proposal start to make sense when the solution is compared with the samples/subsys/nvs in their default configuration using a define from the same family:

west build -b b_l072z_lrwan1 samples/subsys/nvs

Memory region         Used Size  Region Size  %age Used
           FLASH:       27392 B       192 KB     13.93%
             RAM:        4216 B        20 KB     20.59%

dcpleung · 2023-10-13T20:16:08Z

Personally, I think you can roll the changes in mutex_st.c into mutex.c as you can basically #ifdef the bodies of those functions, which should be clean enough.

cfriedt

Just a couple of cosmetic suggestions at the moment.

I wonder if this might let us include a number of other test suites that are filtered out with CONFIG_MULTITHREADING

include/zephyr/kernel.h

kernel/mutex.c

nandojve · 2023-10-15T09:57:28Z

Hi @cfriedt ,

I wonder if this might let us include a number of other test suites that are filtered out with CONFIG_MULTITHREADING

What I have been testing locally was to enable NVS subsystem, which only depends on this sync primitive. I would like know if you are suggesting that we think about a new config that enables this subsystem with the single thread option or your suggestion goes in the direction of more specific tests?

cfriedt · 2023-10-15T10:05:30Z

@nandojve

single thread option or your suggestion goes in the direction of more specific tests?

I don't have any tests in mind yet but a quick search reveals some possibilities

https://github.com/search?q=repo%3Azephyrproject-rtos%2Fzephyr+multithreading+language%3Ayaml&type=code

hongshui3000 · 2023-10-17T02:56:30Z

I think it would be better to change the subsystem to support single-threaded applications rather than changing the mutex. Obviously mutex is generally used in multi-threaded applications. After the change, it can also be used in a single thread, but it has no effect. This is very strange and illogical software behavior

Note that the entire change seems to be changing the behavior of the previous documentation convention

MaureenHelm · 2023-11-02T15:44:19Z

@peter-mitsis can you take a look?

cfriedt · 2023-11-05T11:33:31Z

include/zephyr/kernel.h

@@ -2927,12 +2929,12 @@ struct k_mutex {
 * @cond INTERNAL_HIDDEN
 */
 #define Z_MUTEX_INITIALIZER(obj) \
-	{ \
+	IF_ENABLED(CONFIG_MULTITHREADING, ({ \


Suggested change

IF_ENABLED(CONFIG_MULTITHREADING, ({ \

{IF_ENABLED(CONFIG_MULTITHREADING, ( \

might want to use COND_CODE_1() and set the "else" clause to (0) so that the initializer becomes {0}, which is possibly more pedantically-correct or something like that.

include/zephyr/kernel.h

cfriedt

2 minor changes to get around the 1 error in CI.

Otherwise, just a rebase to resolve conflicts and LGTM.

peter-mitsis · 2023-11-06T17:18:38Z

Adding my 2 cents ... I agree with the assessment from @cfriedt .

andyross

Style: this is a lot of #if'ery to add to a (somewhat fragile!) C file just to make it act like a noop.

Can't you do this at a header level somewhere just by changing the struct definition and making the calls empty inlines?

There are subsystems in Zephyr that require synchronization primitives because the default behavior of Zephyr is to build multi-thread applications. This add very small change in the kernel related to mutexes that affects only single thread build. The change define the k_mutex as an empty struct and the lock/unlock functions are bypassed. This allows few subsystem be used in single thread application without touch the subsystem code. The change keeps Zephyr flexible and help to stay with very low code and sram profile for the very constrained applications that still require some functionality like Non-Volatile Storage. Signed-off-by: Gerson Fernando Budke <gerson.budke@ossystems.com.br>

nandojve · 2024-01-09T17:02:53Z

Hi @andyross ,

Can you revisit and let me know if something is missing? It important to us have this on v3.6.

cfriedt

Still looks ok to me.

nandojve · 2024-01-29T12:27:41Z

Hi @andyross ,

Can you revisit and let me know if something is missing? It important to us have this on v3.6.

ping

nandojve · 2024-02-01T19:17:14Z

friendly ping

npitre · 2024-02-01T19:34:00Z

My previous comment still stands. If this were under my responsibility, I'd ask you to create a mutex_singlethread.c alternative to put dummies in there and leave mutex.c alone.

nandojve · 2024-02-01T19:53:28Z

My previous comment still stands. If this were under my responsibility, I'd ask you to create a mutex_singlethread.c alternative to put dummies in there and leave mutex.c alone.

Hi @npitre ,

This was my initial proposal. Originally posted by @dcpleung in #63935 (comment) So, there are many tastes and it has been difficult to adjust contradictory expectations.

npitre · 2024-02-01T20:46:32Z

Well... I usually agree with @dcpleung but not in this particular case. Mutexes may have their share of implementation subtleties, and e.g. @andyross has a "zync" implementation replacement on the backburner. No need to complicate things with a dummy "single-thread" variant woven into the real variant making maintenance for both variants more difficult. It is not like if any potential addition/improvement to the real mutex would also benefit the dummy one either. For those reasons I think it is best to keep a special, and not quite mutex-like behaving version only for the purpose of satisfying the build in a single-threaded environment separate. Who knows what condition you might encounter, like actually having to wait on an IRQ to fire and unlock a mutex you're waiting for. That might happen even in a single-thread scenario, in which case your dummy mutex won't be as dummy anymore (i.e. k_mutex_lock() would have to spin calling the idle thread function while the coundt != 0 and have k_mutex_unlock() simply decrement the count from the ISR). Do you see yourself trying to fit that behavior in mutex.c without turning it into a mess?

nashif

The project, long time ago, decided not do any of that and to refrain from changing kernel premitives to support single thread and the documentation does reflect that clearly. I will find the relevant history and the decision made, until then....

nashif · 2024-02-01T21:08:07Z

#8393 (comment)
#9808
#27415

This is basically similar to this: #8368

otavio · 2024-02-01T23:04:11Z

The support for a single thread is currently available, but it is not yet complete. This feature allows drivers to function in applications that require a single thread, and depending on the driver used, it can consume a significant amount of RAM and ROM. In some cases, like ours, utilizing Zephyr with the single thread option is the only viable solution.

We used this feature in our project to avoid using multiple RTOS in the same project, which helped us share code and knowledge more efficiently.

dcpleung · 2024-02-01T23:16:37Z

If it is for drivers, wouldn't it be better to amend the drivers for single thread operation? (e.g. by not using mutex when CONFIG_MULTITHREADING is n). It avoids the headache of having multiple mutex implementations.

otavio · 2024-02-01T23:34:36Z

@dcpleung, many drivers would need to be changed, while the mutex wrapper allows multiple drivers to work out of the box.

npitre · 2024-02-01T23:46:37Z

Such wrappers have to be discrete, unobstrusive, isolated and ignorable by those who don't care for such contraptions. Hence the separate file. Do you still have that version in a separate file? I'd like to see it. I'm pretty sure mixing this in mutex.c will simply raise objections.

dcpleung · 2024-02-02T00:25:33Z

Thinking about it more... maybe it would be better to explicitly have a kconfig to enable such a feature. So that it would be obvious when looking at .config, and that it would not be hiding behind CONFIG_MULTITHREADING. Then you will also need another kconfig to turn OFF k_mutex (or rather have a default enabled kconfig for mutex). Having separate source file for this would work with this scheme. Though, it still conflicts with the idea that we shouldn't be do single threaded version of kernel objects.

github-actions · 2024-04-11T00:29:43Z

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

github-actions · 2024-06-11T00:30:14Z

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

github-actions · 2024-08-11T00:32:37Z

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

github-actions · 2024-10-11T00:32:57Z

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

nandojve requested review from dcpleung, nashif and andyross as code owners October 13, 2023 19:28

zephyrbot added the area: Kernel label Oct 13, 2023

zephyrbot requested review from ceolin, cfriedt and peter-mitsis October 13, 2023 19:28

zephyrbot assigned andyross Oct 13, 2023

otavio approved these changes Oct 13, 2023

View reviewed changes

nandojve force-pushed the single_thread/add_mutex_wrap branch 2 times, most recently from 6b5f0a3 to 1ad1ae9 Compare October 15, 2023 08:38

cfriedt reviewed Oct 15, 2023

View reviewed changes

include/zephyr/kernel.h Outdated Show resolved Hide resolved

kernel/mutex.c Outdated Show resolved Hide resolved

cfriedt reviewed Oct 15, 2023

View reviewed changes

kernel/mutex.c Outdated Show resolved Hide resolved

nandojve force-pushed the single_thread/add_mutex_wrap branch from 1ad1ae9 to 64183fc Compare October 15, 2023 10:11

nandojve requested a review from cfriedt October 15, 2023 10:16

nandojve mentioned this pull request Oct 16, 2023

soc: st: Add all stm32l010 missing SoCs #63956

Merged

nandojve force-pushed the single_thread/add_mutex_wrap branch from 64183fc to 3cc31e0 Compare October 17, 2023 10:11

cfriedt reviewed Nov 5, 2023

View reviewed changes

include/zephyr/kernel.h Outdated Show resolved Hide resolved

cfriedt reviewed Nov 5, 2023

View reviewed changes

andyross reviewed Nov 6, 2023

View reviewed changes

nandojve force-pushed the single_thread/add_mutex_wrap branch from 3cc31e0 to d533d25 Compare December 31, 2023 13:58

zephyrbot requested a review from npitre December 31, 2023 13:58

cfriedt approved these changes Jan 10, 2024

View reviewed changes

nandojve added this to the v3.6.0 milestone Feb 1, 2024

dcpleung approved these changes Feb 1, 2024

View reviewed changes

nashif requested changes Feb 1, 2024

View reviewed changes

dcpleung self-requested a review February 2, 2024 00:21

henrikbrixandersen removed this from the v3.6.0 milestone Feb 10, 2024

github-actions bot added the Stale label Apr 11, 2024

nandojve removed the Stale label Apr 11, 2024

github-actions bot added the Stale label Jun 11, 2024

nandojve removed the Stale label Jun 11, 2024

github-actions bot added the Stale label Aug 11, 2024

cfriedt removed the Stale label Aug 11, 2024

github-actions bot added the Stale label Oct 11, 2024

github-actions bot closed this Oct 25, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

kernel: Add mutex wrapper for single thread app #63935

kernel: Add mutex wrapper for single thread app #63935

nandojve commented Oct 13, 2023 •

edited

Loading

dcpleung commented Oct 13, 2023

cfriedt left a comment

nandojve commented Oct 15, 2023

cfriedt commented Oct 15, 2023 •

edited

Loading

hongshui3000 commented Oct 17, 2023 •

edited

Loading

MaureenHelm commented Nov 2, 2023

cfriedt Nov 5, 2023

cfriedt Nov 5, 2023 •

edited

Loading

nandojve Dec 31, 2023

cfriedt left a comment

peter-mitsis commented Nov 6, 2023

andyross left a comment

nandojve commented Jan 9, 2024

cfriedt left a comment

nandojve commented Jan 29, 2024

nandojve commented Feb 1, 2024

npitre commented Feb 1, 2024 via email

nandojve commented Feb 1, 2024

npitre commented Feb 1, 2024 via email

nashif left a comment

nashif commented Feb 1, 2024

otavio commented Feb 1, 2024

dcpleung commented Feb 1, 2024

otavio commented Feb 1, 2024

npitre commented Feb 1, 2024 via email

dcpleung commented Feb 2, 2024

github-actions bot commented Apr 11, 2024

github-actions bot commented Jun 11, 2024

github-actions bot commented Aug 11, 2024

github-actions bot commented Oct 11, 2024

	IF_ENABLED(CONFIG_MULTITHREADING, ({ \
	{IF_ENABLED(CONFIG_MULTITHREADING, ( \

kernel: Add mutex wrapper for single thread app #63935

kernel: Add mutex wrapper for single thread app #63935

Conversation

nandojve commented Oct 13, 2023 • edited Loading

dcpleung commented Oct 13, 2023

cfriedt left a comment

Choose a reason for hiding this comment

nandojve commented Oct 15, 2023

cfriedt commented Oct 15, 2023 • edited Loading

hongshui3000 commented Oct 17, 2023 • edited Loading

MaureenHelm commented Nov 2, 2023

cfriedt Nov 5, 2023

Choose a reason for hiding this comment

cfriedt Nov 5, 2023 • edited Loading

Choose a reason for hiding this comment

nandojve Dec 31, 2023

Choose a reason for hiding this comment

cfriedt left a comment

Choose a reason for hiding this comment

peter-mitsis commented Nov 6, 2023

andyross left a comment

Choose a reason for hiding this comment

nandojve commented Jan 9, 2024

cfriedt left a comment

Choose a reason for hiding this comment

nandojve commented Jan 29, 2024

nandojve commented Feb 1, 2024

npitre commented Feb 1, 2024 via email

nandojve commented Feb 1, 2024

npitre commented Feb 1, 2024 via email

nashif left a comment

Choose a reason for hiding this comment

nashif commented Feb 1, 2024

otavio commented Feb 1, 2024

dcpleung commented Feb 1, 2024

otavio commented Feb 1, 2024

npitre commented Feb 1, 2024 via email

dcpleung commented Feb 2, 2024

github-actions bot commented Apr 11, 2024

github-actions bot commented Jun 11, 2024

github-actions bot commented Aug 11, 2024

github-actions bot commented Oct 11, 2024

nandojve commented Oct 13, 2023 •

edited

Loading

cfriedt commented Oct 15, 2023 •

edited

Loading

hongshui3000 commented Oct 17, 2023 •

edited

Loading

cfriedt Nov 5, 2023 •

edited

Loading