Optimize mutex atomic fast path #16030

jlaitine · 2025-03-21T10:40:10Z

Summary

This further optimizes mutexes:

If priority inheritance or priority protect are enabled in nuttx config, the fast atomic mutex path should be used, if priority adjusting is not enabled for the mutex. The sem->flags can be read/checked without locking
If CONFIG_LIBC_ARCH_ATOMIC is defined, the atomic mutex path should NOT be used, this defaults to spinlock implementation which makes the mutexes heavy
The atomic fast mutexes can be used directly from userspace, so add that logic into the libc. This optimizes the pthread_mutexes heavily in protected builds, as the priority inheritance and priority protection are disabled by default in those, and typically there is no need to do the syscall to the kernel at all.

Impact

Has no functional impact, but increases performance especially for:

pthread_mutexes in CONFIG_BUILD_KERNEL
pthread_mutexes when the CONFIG_PRIORITY_INHERITANCE and/or CONFIG_PRIORITY_PROTECT are enabled
in case CONFIG_LIBC_ARCH_ATOMIC is defined

Testing

rv-virt:nsh: ostest passes
rv-virt64:nsh: ostest passes
- log: "$ qemu-system-riscv64 -semihosting -M virt,aclint=on -cpu rv64 -bios none -kernel nuttx -nographic 2>&1 | tee log.txt"
  log.txt

with TII custom mpfs targets w. application sw, with SMP 4 cores, CONFIG_BUILD_FLAT and CONFIG_BUILD_KERNEL

nuttxpr · 2025-03-21T10:47:01Z

[Experimental Bot, please feedback here]

Yes, this PR appears to meet the NuttX requirements, although some minor improvements could be made.

Strengths:

Clear Summary: The summary clearly explains the "why," "what," and "how" of the changes.
Impact Section Addressed: All impact categories are addressed, and the performance improvements are highlighted.
Testing Provided: Testing logs are included and multiple architectures/configurations are covered. The link to the log file is helpful.

Areas for Improvement:

Missing Issue References: If this PR addresses a specific NuttX or NuttX Apps issue, those should be linked in the Summary. Even if it's a performance optimization without a directly linked issue, consider creating one to track it.
Build Host Details: The Testing section mentions targets but lacks detail on the build host environment (OS, compiler version, etc.). This information is important for reproducibility.
"Before" vs. "After" Logs Clarity: While logs are provided, it's unclear how to directly compare the performance improvement. Consider including specific metrics (e.g., timings for relevant operations) in the logs or providing a summary of the performance gains observed. Simply saying "ostest passes" doesn't quantify the improvement.
Clarity on CONFIG_LIBC_ARCH_ATOMIC: The summary mentions not using the atomic mutex path if CONFIG_LIBC_ARCH_ATOMIC is defined. Explain why this is the case (e.g., "because it defaults to spinlocks, which are less efficient in this context").

Recommendation: Add the missing build host details, clarify the CONFIG_LIBC_ARCH_ATOMIC reasoning, and provide more quantifiable performance data in the testing section. Linking a related issue (even a newly created one for performance tracking) would also be beneficial.

pussuw · 2025-03-21T10:51:50Z

This partially completes the discussion from here #14465
@zyfeier can you take a look?

The next optimization is to move the mutex holder to userspace

jlaitine · 2025-03-21T12:47:08Z

There seems to be some issue with sim:citest target, and of course I can't even build the citest locally ( The same issue as in #12974 ). Need to get the docker build environment and try to resolve that ...

jlaitine · 2025-03-21T14:16:22Z

Changed the way how atomic.h is included, now it should compile. Re-tested on mpfs.

include/nuttx/semaphore.h

jlaitine · 2025-03-24T08:29:44Z

Re-tested on mpfs and qemu targets

jlaitine · 2025-03-24T10:41:43Z

Rebased against current master and cleaned up one comment.

lupyuen · 2025-03-24T10:56:26Z

@nuttxpr test avaota-a1:nsh

nuttxpr · 2025-03-24T11:00:44Z

[Experimental Bot, please feedback here]

Build and Test Successful (avaota-a1:nsh)
https://gitlab.com/lupyuen/nuttx-build-log/-/snippets/4828942

$ git clone https://github.com/tiiuae/nuttx nuttx --branch optimze_mutex_atomic_fast_path
$ git clone https://github.com/apache/nuttx-apps apps --branch master
$ pushd nuttx
$ git reset --hard HEAD
HEAD is now at 5908cdc578 libc/semaphore: Add fast mutex wait/post paths to libc
$ popd
$ pushd apps
$ git reset --hard HEAD
HEAD is now at e336ebad5 cmake: fix cmake build for FreeModBus
$ popd
NuttX Source: https://github.com/apache/nuttx/tree/5908cdc578e8d515bc7183d7ae558973c08e6023
NuttX Apps: https://github.com/apache/nuttx-apps/tree/e336ebad5e4da1cfd9d99ac04bccaf15cc09249f
$ cd nuttx
$ tools/configure.sh avaota-a1:nsh
$ make -j
$ make -j export
$ pushd ../apps
$ ./tools/mkimport.sh -z -x ../nuttx/nuttx-export-10.4.0.tar.gz
$ make -j import
$ popd
$ genromfs -f initrd -d ../apps/bin -V NuttXBootVol
$ head -c 65536 /dev/zero
$ cat nuttx.bin /tmp/nuttx.pad initrd
$ scp Image thinkcentre:/tmp/Image
$ ssh thinkcentre sudo /home/user/copy-image.sh
$ sd-mux-ctrl --device-serial=sd-wire_02-09 --ts
$ mkdir -p /tmp/sda1
$ mount /dev/sda1 /tmp/sda1
$ cp /tmp/Image /tmp/sda1/
$ rm /tmp/Image
$ umount /tmp/sda1
$ sd-mux-ctrl --device-serial=sd-wire_02-09 --dut
$ cd /home/luppy/nuttx-build-farm
$ ssh thinkcentre
nsh> uname -a
NuttX 10.4.0 5908cdc578 Mar 24 2025 18:58:32 arm64 avaota-a1
nsh> ostest
arena        a000    28000
ordblks         2        4
mxordblk     5ff8    1bff8
uordblks     27e8     6700
fordblks     7818    21900
user_main: Exiting
ostest_main: Exiting with status 0
Now running https://github.com/lupyuen/nuttx-build-farm/blob/main/avaota-power.sh off
----- Power off Avaota-A1
[]

lupyuen · 2025-03-24T11:01:03Z

@nuttxpr test milkv_duos:nsh

nuttxpr · 2025-03-24T11:05:06Z

[Experimental Bot, please feedback here]

Build and Test Successful (milkv_duos:nsh)
https://gitlab.com/lupyuen/nuttx-build-log/-/snippets/4828943

$ git clone https://github.com/tiiuae/nuttx nuttx --branch optimze_mutex_atomic_fast_path
$ git clone https://github.com/apache/nuttx-apps apps --branch master
$ pushd nuttx
$ git reset --hard HEAD
HEAD is now at 5908cdc578 libc/semaphore: Add fast mutex wait/post paths to libc
$ popd
$ pushd apps
$ git reset --hard HEAD
HEAD is now at e336ebad5 cmake: fix cmake build for FreeModBus
$ popd
NuttX Source: https://github.com/apache/nuttx/tree/5908cdc578e8d515bc7183d7ae558973c08e6023
NuttX Apps: https://github.com/apache/nuttx-apps/tree/e336ebad5e4da1cfd9d99ac04bccaf15cc09249f
$ cd nuttx
$ tools/configure.sh milkv_duos:nsh
$ make -j
$ make -j export
$ pushd ../apps
$ ./tools/mkimport.sh -z -x ../nuttx/nuttx-export-10.4.0.tar.gz
$ make -j import
$ popd
$ genromfs -f initrd -d ../apps/bin -V NuttXBootVol
$ head -c 65536 /dev/zero
$ cat nuttx.bin /tmp/nuttx.pad initrd
$ scp Image tftpserver:/tftpboot/Image-sg2000
$ cd /home/luppy/nuttx-build-farm
$ ssh tftpserver
OpenSBI v0.9
nsh> uname -a
NuttX 10.4.0 5908cdc578 Mar 24 2025 19:02:56 risc-v milkv_duos
nsh> ostest
arena       81000    81000
ordblks         2        3
mxordblk    7cff8    78ff8
uordblks     2660     4570
fordblks    7e9a0    7ca90
user_main: Exiting
ostest_main: Exiting with status 0
nsh> Now running https://github.com/lupyuen/nuttx-build-farm/blob/main/oz64-power.sh off
----- Power off Oz64
[]

nuttxpr · 2025-03-26T08:13:59Z

[Experimental Bot, please feedback here]

Build and Test Successful (milkv_duos:nsh)
https://gitlab.com/lupyuen/nuttx-build-log/-/snippets/4829490

$ git clone https://github.com/tiiuae/nuttx nuttx --branch optimze_mutex_atomic_fast_path
$ git clone https://github.com/apache/nuttx-apps apps --branch master
$ pushd nuttx
$ git reset --hard HEAD
HEAD is now at 603b3c82b3 libc/semaphore: Add fast mutex wait/post paths to libc
$ popd
$ pushd apps
$ git reset --hard HEAD
HEAD is now at 7b65b2bb4 drivertest_uart: Pass in the specified parameters to test
$ popd
NuttX Source: https://github.com/apache/nuttx/tree/603b3c82b3a3da6281016ef48348b07192bb5a8e
NuttX Apps: https://github.com/apache/nuttx-apps/tree/7b65b2bb41763207dcfb965c3eb2223595986779
$ cd nuttx
$ tools/configure.sh milkv_duos:nsh
$ make -j
$ make -j export
$ pushd ../apps
$ ./tools/mkimport.sh -z -x ../nuttx/nuttx-export-10.4.0.tar.gz
$ make -j import
$ popd
$ genromfs -f initrd -d ../apps/bin -V NuttXBootVol
$ head -c 65536 /dev/zero
$ cat nuttx.bin /tmp/nuttx.pad initrd
$ scp Image tftpserver:/tftpboot/Image-sg2000
$ cd /home/luppy/nuttx-build-farm
$ ssh tftpserver
OpenSBI v0.9
nsh> uname -a
NuttX 10.4.0 603b3c82b3 Mar 26 2025 16:11:52 risc-v milkv_duos
nsh> ostest
arena       81000    81000
ordblks         2        3
mxordblk    7cff8    78ff8
uordblks     2660     4570
fordblks    7e9a0    7ca90
user_main: Exiting
ostest_main: Exiting with status 0
nsh> Now running https://github.com/lupyuen/nuttx-build-farm/blob/main/oz64-power.sh off
----- Power off Oz64
[]

lupyuen

Retested OK with OSTest on Real Hardware. Thanks :-)

…TECT or CONFIG_PRIORITY_INHERITANCE When the semaphore priority flags is set to NONE, and the semaphore is a mutex, the fast locking path can be used, even when priority inheritance or priority protect are enabled globally. Signed-off-by: Jukka Laitinen <jukka.laitinen@tii.ae>

jlaitine · 2025-03-28T11:36:09Z

Modified the PR acc. to the review comments, re-tested on rv-virt:nsh64 (ostest) and on multiple real hw & build configurations (MPFS, CONFIG_BUILD_KERNEL and CONFIG_BUILD_FLAT, SMP and non SMP).

This avoids unnecessary syscalls in memory protected builds, when mutex lock/unlock can be done with only atomic counter access Signed-off-by: Jukka Laitinen <jukka.laitinen@tii.ae>

jlaitine · 2025-03-28T11:52:09Z

And had to modify still a bit to pass CI with debug assertions....

lupyuen · 2025-03-28T11:54:51Z

~~Hmmm wonder why Avaota-A1 Arm64 SBC is failing...~~
https://gist.github.com/lupyuen/75bee0db56674f2a90c4be99a3baa15a

dump_assert_info: Current Version: NuttX  10.4.0 427d51e07e Mar 28 2025 19:51:10 arm64
dump_assert_info: Assertion failed ret > 0: at file: init/nx_bringup.c:380 task: AppBringUp process: Kernel 0x408065c8

Update: Seems OK now :-)

lupyuen · 2025-03-28T12:10:37Z

@nuttxpr test avaota-a1:nsh

jlaitine · 2025-03-28T12:15:47Z

~~Hmmm wonder why Avaota-A1 Arm64 SBC is failing...~~ https://gist.github.com/lupyuen/75bee0db56674f2a90c4be99a3baa15a
dump_assert_info: Current Version: NuttX  10.4.0 427d51e07e Mar 28 2025 19:51:10 arm64
dump_assert_info: Assertion failed ret > 0: at file: init/nx_bringup.c:380 task: AppBringUp process: Kernel 0x408065c8
Update: Seems OK now :-)

Ok, what happened? I re-checked the PR, and did some extra tests on i.MX93 (arm64) CONFIG_BUILD_KERNEL, but didn't find anything...

nuttxpr · 2025-03-28T12:16:47Z

[Experimental Bot, please feedback here]

Build and Test Successful (avaota-a1:nsh)
https://gitlab.com/lupyuen/nuttx-build-log/-/snippets/4830105

$ git clone https://github.com/tiiuae/nuttx nuttx --branch optimze_mutex_atomic_fast_path
$ git clone https://github.com/apache/nuttx-apps apps --branch master
$ pushd nuttx
$ git reset --hard HEAD
HEAD is now at 2c8f0cf574 libc/semaphore: Move fast mutex wait/post paths to libc
$ popd
$ pushd apps
$ git reset --hard HEAD
HEAD is now at 369729eea testing/drivers/drivertest: Uniform test case name
$ popd
NuttX Source: https://github.com/apache/nuttx/tree/2c8f0cf57465f7751eac07e5aade0cb5ae2c3b01
NuttX Apps: https://github.com/apache/nuttx-apps/tree/369729eea4f18b61ab899f84948acfa4820d0518
$ cd nuttx
$ tools/configure.sh avaota-a1:nsh
$ make -j
$ make -j export
$ pushd ../apps
$ ./tools/mkimport.sh -z -x ../nuttx/nuttx-export-10.4.0.tar.gz
$ make -j import
$ popd
$ genromfs -f initrd -d ../apps/bin -V NuttXBootVol
$ head -c 65536 /dev/zero
$ cat nuttx.bin /tmp/nuttx.pad initrd
$ scp Image thinkcentre:/tmp/Image
$ ssh thinkcentre sudo /home/user/copy-image.sh
$ sd-mux-ctrl --device-serial=sd-wire_02-09 --ts
$ mkdir -p /tmp/sda1
$ mount /dev/sda1 /tmp/sda1
$ cp /tmp/Image /tmp/sda1/
$ rm /tmp/Image
$ umount /tmp/sda1
$ sd-mux-ctrl --device-serial=sd-wire_02-09 --dut
$ cd /home/luppy/nuttx-build-farm
$ ssh thinkcentre
nsh> uname -a
NuttX 10.4.0 2c8f0cf574 Mar 28 2025 20:14:34 arm64 avaota-a1
nsh> ostest
arena        a000    28000
ordblks         2        4
mxordblk     5ff8    1bff8
uordblks     27e8     6700
fordblks     7818    21900
user_main: Exiting
ostest_main: Exiting with status 0
nsh> Now running https://github.com/lupyuen/nuttx-build-farm/blob/main/avaota-power.sh off
----- Power off Avaota-A1
[]

lupyuen · 2025-03-28T12:17:05Z

@nuttxpr test milkv_duos:nsh

nuttxpr · 2025-03-28T12:21:52Z

[Experimental Bot, please feedback here]

Build and Test Successful (milkv_duos:nsh)
https://gitlab.com/lupyuen/nuttx-build-log/-/snippets/4830107

$ git clone https://github.com/tiiuae/nuttx nuttx --branch optimze_mutex_atomic_fast_path
$ git clone https://github.com/apache/nuttx-apps apps --branch master
$ pushd nuttx
$ git reset --hard HEAD
HEAD is now at 2c8f0cf574 libc/semaphore: Move fast mutex wait/post paths to libc
$ popd
$ pushd apps
$ git reset --hard HEAD
HEAD is now at 369729eea testing/drivers/drivertest: Uniform test case name
$ popd
NuttX Source: https://github.com/apache/nuttx/tree/2c8f0cf57465f7751eac07e5aade0cb5ae2c3b01
NuttX Apps: https://github.com/apache/nuttx-apps/tree/369729eea4f18b61ab899f84948acfa4820d0518
$ cd nuttx
$ tools/configure.sh milkv_duos:nsh
$ make -j
$ make -j export
$ pushd ../apps
$ ./tools/mkimport.sh -z -x ../nuttx/nuttx-export-10.4.0.tar.gz
$ make -j import
$ popd
$ genromfs -f initrd -d ../apps/bin -V NuttXBootVol
$ head -c 65536 /dev/zero
$ cat nuttx.bin /tmp/nuttx.pad initrd
$ scp Image tftpserver:/tftpboot/Image-sg2000
$ cd /home/luppy/nuttx-build-farm
$ ssh tftpserver
OpenSBI v0.9
nsh> uname -a
NuttX 10.4.0 2c8f0cf574 Mar 28 2025 20:19:48 risc-v milkv_duos
nsh> ostest
arena       81000    81000
ordblks         2        3
mxordblk    7cff8    78ff8
uordblks     2660     4570
fordblks    7e9a0    7ca90
user_main: Exiting
ostest_main: Exiting with status 0
nsh> Now running https://github.com/lupyuen/nuttx-build-farm/blob/main/oz64-power.sh off
----- Power off Oz64
[]

lupyuen

Retested OK with OSTest on Real Hardware. Thanks :-)

libs/libc/semaphore/sem_wait.c

jlaitine · 2025-03-31T09:55:36Z

Sorry, clicked "close by accident" :)

jlaitine · 2025-03-31T10:00:37Z

I will still do the following chages based on comments from @pussuw :

Will have separate functions for non-cancellation or errno in libc without the nx-prefix and functions in kernel with the nx-prefix
Have the common code (counter handling) as before as an inline function and include that in both
replace all the existing calls of nxsem* inside libc with calls to the functions without the nx-prefix

include/nuttx/sem_fast.h

github-actions bot added Area: OS Components OS Components issues Size: M The size of the change in this PR is medium labels Mar 21, 2025

pussuw requested review from pussuw and xiaoxiang781216 March 21, 2025 10:50

pussuw approved these changes Mar 21, 2025

View reviewed changes

jlaitine force-pushed the optimze_mutex_atomic_fast_path branch from 0362941 to 7f733ba Compare March 21, 2025 12:31

jlaitine force-pushed the optimze_mutex_atomic_fast_path branch from 7f733ba to 2ba7c1b Compare March 21, 2025 14:15

jlaitine force-pushed the optimze_mutex_atomic_fast_path branch from 2ba7c1b to 50b2e31 Compare March 21, 2025 14:56

pussuw reviewed Mar 21, 2025

View reviewed changes

include/nuttx/semaphore.h Outdated Show resolved Hide resolved

pussuw approved these changes Mar 21, 2025

View reviewed changes

xiaoxiang781216 reviewed Mar 21, 2025

View reviewed changes

include/nuttx/semaphore.h Outdated Show resolved Hide resolved

include/nuttx/semaphore.h Outdated Show resolved Hide resolved

include/nuttx/semaphore.h Outdated Show resolved Hide resolved

jlaitine force-pushed the optimze_mutex_atomic_fast_path branch 3 times, most recently from ac60716 to bc30151 Compare March 24, 2025 08:09

jlaitine requested review from acassis, anchao, pussuw, raiden00pl and xiaoxiang781216 March 24, 2025 08:12

jlaitine force-pushed the optimze_mutex_atomic_fast_path branch from bc30151 to 5908cdc Compare March 24, 2025 10:41

lupyuen approved these changes Mar 26, 2025

View reviewed changes

jerpelea approved these changes Mar 26, 2025

View reviewed changes

jlaitine force-pushed the optimze_mutex_atomic_fast_path branch 3 times, most recently from 3854169 to 427d51e Compare March 28, 2025 11:17

libc/semaphore: Move fast mutex wait/post paths to libc

2c8f0cf

This avoids unnecessary syscalls in memory protected builds, when mutex lock/unlock can be done with only atomic counter access Signed-off-by: Jukka Laitinen <jukka.laitinen@tii.ae>

jlaitine force-pushed the optimze_mutex_atomic_fast_path branch from 427d51e to 2c8f0cf Compare March 28, 2025 11:49

lupyuen approved these changes Mar 28, 2025

View reviewed changes

pussuw reviewed Mar 28, 2025

View reviewed changes

libs/libc/semaphore/sem_wait.c Show resolved Hide resolved

xiaoxiang781216 approved these changes Mar 30, 2025

View reviewed changes

jlaitine closed this Mar 31, 2025

jlaitine reopened this Mar 31, 2025

jerpelea approved these changes Mar 31, 2025

View reviewed changes

anchao approved these changes Apr 1, 2025

View reviewed changes

include/nuttx/sem_fast.h Outdated Show resolved Hide resolved

pussuw approved these changes Apr 1, 2025

View reviewed changes

xiaoxiang781216 merged commit 19a8e24 into apache:master Apr 1, 2025
78 checks passed

tmedicci mentioned this pull request Apr 11, 2025

[BUG] esp32-devkitc:smp fails on ostest after #16030 #16193

Closed

1 task

tmedicci mentioned this pull request Apr 22, 2025

[BUG] RP2350 ostest stuck when SMP enabled #16133

Open

1 task

Optimize mutex atomic fast path #16030

Optimize mutex atomic fast path #16030

Uh oh!

Conversation

jlaitine commented Mar 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Impact

Testing

Uh oh!

nuttxpr commented Mar 21, 2025

Uh oh!

pussuw commented Mar 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jlaitine commented Mar 21, 2025

Uh oh!

jlaitine commented Mar 21, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jlaitine commented Mar 24, 2025

Uh oh!

jlaitine commented Mar 24, 2025

Uh oh!

lupyuen commented Mar 24, 2025

Uh oh!

nuttxpr commented Mar 24, 2025

Uh oh!

lupyuen commented Mar 24, 2025

Uh oh!

nuttxpr commented Mar 24, 2025

Uh oh!

nuttxpr commented Mar 26, 2025

Uh oh!

lupyuen left a comment

Choose a reason for hiding this comment

Uh oh!

jlaitine commented Mar 28, 2025

Uh oh!

jlaitine commented Mar 28, 2025

Uh oh!

lupyuen commented Mar 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lupyuen commented Mar 28, 2025

Uh oh!

jlaitine commented Mar 28, 2025

Uh oh!

nuttxpr commented Mar 28, 2025

Uh oh!

lupyuen commented Mar 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nuttxpr commented Mar 28, 2025

Uh oh!

lupyuen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jlaitine commented Mar 31, 2025

Uh oh!

jlaitine commented Mar 31, 2025

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

jlaitine commented Mar 21, 2025 •

edited

Loading

pussuw commented Mar 21, 2025 •

edited

Loading

lupyuen commented Mar 28, 2025 •

edited

Loading

lupyuen commented Mar 28, 2025 •

edited

Loading