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
Add newlib retargetable locking implementation and tests #36201
Add newlib retargetable locking implementation and tests #36201
Conversation
246d3a1
to
f282bc4
Compare
f282bc4
to
484d1c2
Compare
UPDATE:
|
484d1c2
to
c59d1c2
Compare
Marking it as "ready for review" since the PR itself is ready. Please note that the CI failure will be gone as soon as the SDK 0.13.0 is mainlined. |
This commit adds the newlib retargetable locking interface function implementations in order to make newlib functions thread safe. The newlib retargetable locking interface is internally called by the standard C library functions provided by newlib to synchronise access to the internal shared resources. By default, the retargetable locking interface functions defined within the newlib library are no-op. When multi-threading is enabled (i.e. `CONFIG_MULTITHREADING=y`), the Zephyr-side retargetable locking interface implementations override the default newlib implementation and provide locking mechanism. The retargetable locking interface may be called with either a static (`__lock__...`) or a dynamic lock. The static locks are statically allocated and initialised immediately after kernel initialisation by `newlib_locks_prepare`. The dynamic locks are allocated and de-allocated through the `__retargetable_lock_init[_recursive]` and `__retarget_lock_close_[recurisve]` functions as necessary by the newlib functions. These locks are allocated in the newlib heap using the `malloc` function when userspace is not enabled -- this is safe because the internal multi-threaded malloc lock implementations (`__malloc_lock` and `__malloc_unlock`) call the retargetable locking interface with a static lock (`__lock__malloc_recursive_mutex`). When userspace is enabled, the dynamic locks are allocated and freed through `k_object_alloc` and `k_object_release`. Note that the lock implementations used here are `k_mutex` and `k_sem` instead of `sys_mutex` and `sys_sem` because the Zephyr kernel does not currently support dynamic allocation of the latter. These locks should be updated to use `sys_mutex` and `sys_sem` when the Zephyr becomes capable of dynamically allocating them in the future. Signed-off-by: Stephanos Ioannidis <root@stephanos.io>
This commit adds the `static` keyword to the test functions that are not intended to be globally available. Signed-off-by: Stephanos Ioannidis <root@stephanos.io>
e0adc1e
to
cb4be75
Compare
SDK 0.13.0 has been mainlined. This PR is ready for final review and merging. Please note that the checkpatch failure is wrong and should be ignored. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me, with my limited knowledge of newlib's retargetable locking system.
This commit adds the tests for the newlib retargetable locking interface, as well as the tests for the internal lock functions that are supposed to internally invoke the retargetable locking interface. All of these tests must pass when the toolchain newlib is compiled with the `retargetable-locking` and `multithread` options, which are required to ensure that the newlib is thread-safe, enabled. If the toolchain newlib is compiled with either of these options disabled, this test will fail. This commit also adds the userspace testcases to ensure that the newlib is thread-safe in the user mode. Signed-off-by: Stephanos Ioannidis <root@stephanos.io>
cb4be75
to
712e605
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to update the TOOLCHAIN_ZEPHYR_MINIMUM_REQUIRED_VERSION
in the cmakefiles
In order to use the newlib retagetable locking interface (for thread safety), which requires the newlib multi-threading feature to be enabled, the Zephyr SDK 0.13.0 or above must be used. Signed-off-by: Stephanos Ioannidis <root@stephanos.io>
@galak Done |
CI says there is a whitespace issue. I'm not sure I see what the problem is though. @galak - could that be a false positive? |
@cfriedt I went ahead and merged this. |
Thanks - I don't seem to have merge rights to manually merge PR's in case of false positives, but I'm also kind of ok with that. |
Build newlib library to be thread-safe in multithreaded environment. zephyrproject-rtos/zephyr#21518 zephyrproject-rtos/zephyr#21519 zephyrproject-rtos/zephyr#36201 https://sourceware.org/legacy-ml/newlib/2016/msg01165.html https://sourceware.org/git/?p=newlib-cygwin.git;a=commit;h=bd54749095ee45d7136b6e7c8a1e5218749c87b6 Error log: newlib/libc-hooks.c:310:1: note: in expansion of macro 'BUILD_ASSERT' BUILD_ASSERT(IS_ENABLED(_RETARGETABLE_LOCKING), "Retargetable locking must be enabled"); Signed-off-by: Naveen Saini <naveen.kumar.saini@intel.com> Tested-by: Jon Mason <jon.mason@arm.com>
This series adds the newlib retargetable locking interface function implementations, as well as the tests to verify that the retargetable locking interface is functional and used by the internal lock functions.
Fixes the "Retarget Locking" portion of #21519.
NOTE1: The reentrancy-related fixes will be addressed in an upcoming (separate) PR.
NOTE2: The tests will fail because the Zephyr SDK newlib is currently compiled with
--disable-newlib-multithread
. To be merged after zephyrproject-rtos/sdk-ng#344 is mainlined in the CI.Fixes #21519.