This is an automated email from the ASF dual-hosted git repository. jerpelea pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/nuttx.git
commit ab0e1da786a07fd5dcc950223d69431c14852f72 Author: Bowen Wang <[email protected]> AuthorDate: Sun Nov 8 12:25:45 2026 +0800 openamp/libmetal: make metal_mutex a recursive spinlock by default The rpmsg transports take rdev->lock in their rx path, which may run in interrupt context. With metal_mutex as an rmutex that path has to defer to a thread, and rpmsg_port/rpmsg_virtio dump() skip the lock in interrupt and idle context. Replace libmetal patch 0003 so metal_mutex is a recursive spinlock (irqsave, nopreempt) by default, with an is_mutex flag to switch one instance back to an rmutex. The nesting depth is kept in metal_mutex, because rspinlock_t does not count without CONFIG_SPINLOCK; the irq state is saved on the outermost acquire and restored on the outermost release. try_acquire now returns non-zero on success as libmetal expects, instead of passing nxrmutex_trylock()'s 0 through. rproc->lock is held across the remoteproc ops, which may sleep, so rptun_init() sets is_mutex on it. remoteproc_init() initializes the lock before calling ops->init, so the flag is set before first use. Signed-off-by: Bowen Wang <[email protected]> Signed-off-by: Yongrong Wang <[email protected]> --- drivers/rptun/rptun.c | 1 + ...e-the-libmetal-nuttx-mutex-to-recursive-m.patch | 80 ----------- ...libmetal-nuttx-mutex-a-recursive-spinlock.patch | 159 +++++++++++++++++++++ openamp/libmetal.cmake | 2 +- openamp/libmetal.defs | 2 +- 5 files changed, 162 insertions(+), 82 deletions(-) diff --git a/drivers/rptun/rptun.c b/drivers/rptun/rptun.c index dae052a7f4f..2d26bd2921c 100644 --- a/drivers/rptun/rptun.c +++ b/drivers/rptun/rptun.c @@ -196,6 +196,7 @@ rptun_init(FAR struct remoteproc *rproc, FAR const struct remoteproc_ops *ops, FAR void *arg) { + rproc->lock.is_mutex = true; rproc->ops = ops; rproc->priv = arg; diff --git a/openamp/0003-mutex-change-the-libmetal-nuttx-mutex-to-recursive-m.patch b/openamp/0003-mutex-change-the-libmetal-nuttx-mutex-to-recursive-m.patch deleted file mode 100644 index 2b9ae675254..00000000000 --- a/openamp/0003-mutex-change-the-libmetal-nuttx-mutex-to-recursive-m.patch +++ /dev/null @@ -1,80 +0,0 @@ -From ef06cf5ac9be793200783f24996d529db715ac43 Mon Sep 17 00:00:00 2001 -From: Bowen Wang <[email protected]> -Date: Mon, 25 Nov 2024 23:07:10 +0800 -Subject: [PATCH 3/3] mutex: change the libmetal nuttx mutex to recursive mutex - -To avoid the crash when lock are acquired twice, one case is in the -destroy process: -we hold the rdev->lock to iterate the rdev->endpoints to destroy all -the endpoints, but rpmsg_destroy_ept() may send the name service message, -and need acquire the rdev->lock again to lead crash. - -Signed-off-by: Bowen Wang <[email protected]> ---- - lib/system/nuttx/mutex.h | 18 +++++++++--------- - 1 file changed, 9 insertions(+), 9 deletions(-) - -diff --git a/lib/system/nuttx/mutex.h libmetal/lib/system/nuttx/mutex.h -index 26ec4f9..d13c9c5 100644 ---- a/lib/system/nuttx/mutex.h -+++ libmetal/lib/system/nuttx/mutex.h -@@ -22,47 +22,47 @@ - extern "C" { - #endif - --typedef mutex_t metal_mutex_t; -+typedef rmutex_t metal_mutex_t; - - /* - * METAL_MUTEX_INIT - used for initializing an mutex element in a static struct - * or global - */ --#define METAL_MUTEX_INIT(m) NXMUTEX_INITIALIZER -+#define METAL_MUTEX_INIT(m) NXRMUTEX_INITIALIZER - /* - * METAL_MUTEX_DEFINE - used for defining and initializing a global or - * static singleton mutex - */ --#define METAL_MUTEX_DEFINE(m) metal_mutex_t m = NXMUTEX_INITIALIZER -+#define METAL_MUTEX_DEFINE(m) metal_mutex_t m = NXRMUTEX_INITIALIZER - - static inline void __metal_mutex_init(metal_mutex_t *mutex) - { -- nxmutex_init(mutex); -+ nxrmutex_init(mutex); - } - - static inline void __metal_mutex_deinit(metal_mutex_t *mutex) - { -- nxmutex_destroy(mutex); -+ nxrmutex_destroy(mutex); - } - - static inline int __metal_mutex_try_acquire(metal_mutex_t *mutex) - { -- return nxmutex_trylock(mutex); -+ return nxrmutex_trylock(mutex); - } - - static inline void __metal_mutex_acquire(metal_mutex_t *mutex) - { -- nxmutex_lock(mutex); -+ nxrmutex_lock(mutex); - } - - static inline void __metal_mutex_release(metal_mutex_t *mutex) - { -- nxmutex_unlock(mutex); -+ nxrmutex_unlock(mutex); - } - - static inline int __metal_mutex_is_acquired(metal_mutex_t *mutex) - { -- return nxmutex_is_locked(mutex); -+ return nxrmutex_is_locked(mutex); - } - - #ifdef __cplusplus --- -2.34.1 - diff --git a/openamp/0003-mutex-make-the-libmetal-nuttx-mutex-a-recursive-spinlock.patch b/openamp/0003-mutex-make-the-libmetal-nuttx-mutex-a-recursive-spinlock.patch new file mode 100644 index 00000000000..f939de6ecc2 --- /dev/null +++ b/openamp/0003-mutex-make-the-libmetal-nuttx-mutex-a-recursive-spinlock.patch @@ -0,0 +1,159 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Bowen Wang <[email protected]> +Date: Mon, 25 Nov 2024 23:07:10 +0800 +Subject: [PATCH 3/3] mutex: make the libmetal nuttx mutex a recursive spinlock + +metal_mutex protects rdev->lock and rproc->lock. rdev->lock is taken +from the transport rx path, which may run in interrupt context, so it +must not be a sleeping lock. + +Make metal_mutex a recursive spinlock (irqsave, nopreempt) by default. +Recursion is kept because the rpmsg paths may take rdev->lock again +from a callback. A user that sleeps while holding the lock, such as +rproc->lock across the remoteproc ops, sets is_mutex to switch that +instance to an rmutex. + +The nesting depth is kept in metal_mutex itself: without +CONFIG_SPINLOCK, rspinlock_t does not count. The irq state is saved on +the outermost acquire and restored on the outermost release. + +Also fix try_acquire to return non-zero on success, as libmetal +expects; nxrmutex_trylock() returns 0 on success. + +Squashed from: + mutex: change the libmetal nuttx mutex to recursive mutex + libmetal/mutex: use spinlock as the mutex + lib/system/nuttx/mutex.h: support both the rspinlock and rmutex + lib/system/nuttx/mutex.h: change the default mode to spinlock + libmetal/nuttx: fix spinlock irq restore for recursive mutex release + +Signed-off-by: Bowen Wang <[email protected]> +Signed-off-by: Yongrong Wang <[email protected]> +--- + lib/system/nuttx/mutex.h | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++--------- + 1 file changed, 64 insertions(+), 9 deletions(-) + +diff --git a/lib/system/nuttx/mutex.h libmetal/lib/system/nuttx/mutex.h +--- a/lib/system/nuttx/mutex.h ++++ libmetal/lib/system/nuttx/mutex.h +@@ -16,53 +16,108 @@ + #ifndef __METAL_NUTTX_MUTEX__H__ + #define __METAL_NUTTX_MUTEX__H__ + ++#include <stdbool.h> ++ + #include <nuttx/mutex.h> ++#include <nuttx/spinlock.h> + + #ifdef __cplusplus + extern "C" { + #endif + +-typedef mutex_t metal_mutex_t; ++/* ++ * The default is a recursive spinlock, so the lock can be taken from ++ * interrupt context. Set is_mutex before first use to make it an rmutex ++ * instead, for a user that sleeps while holding the lock. ++ * ++ * depth counts the nesting on the owner. It is only changed with the lock ++ * held, and the irq state is saved on the outermost acquire and restored ++ * on the outermost release. ++ */ ++typedef struct metal_mutex { ++ bool is_mutex; ++ unsigned int depth; ++ rspinlock_t lock; ++ irqstate_t flags; ++ rmutex_t mutex; ++} metal_mutex_t; + + /* + * METAL_MUTEX_INIT - used for initializing an mutex element in a static struct + * or global + */ +-#define METAL_MUTEX_INIT(m) NXMUTEX_INITIALIZER ++#define METAL_MUTEX_INIT(m) \ ++ {.is_mutex = false, .depth = 0, .lock = RSPINLOCK_INITIALIZER, \ ++ .flags = 0, .mutex = NXRMUTEX_INITIALIZER} + /* + * METAL_MUTEX_DEFINE - used for defining and initializing a global or + * static singleton mutex + */ +-#define METAL_MUTEX_DEFINE(m) metal_mutex_t m = NXMUTEX_INITIALIZER ++#define METAL_MUTEX_DEFINE(m) metal_mutex_t m = METAL_MUTEX_INIT(m) + + static inline void __metal_mutex_init(metal_mutex_t *mutex) + { +- nxmutex_init(mutex); ++ mutex->is_mutex = false; ++ mutex->depth = 0; ++ rspin_lock_init(&mutex->lock); ++ nxrmutex_init(&mutex->mutex); + } + + static inline void __metal_mutex_deinit(metal_mutex_t *mutex) + { +- nxmutex_destroy(mutex); ++ nxrmutex_destroy(&mutex->mutex); + } + + static inline int __metal_mutex_try_acquire(metal_mutex_t *mutex) + { +- return nxmutex_trylock(mutex); ++ irqstate_t flags; ++ ++ if (mutex->is_mutex) ++ return nxrmutex_trylock(&mutex->mutex) == OK; ++ ++ if (!rspin_trylock_irqsave_nopreempt(&mutex->lock, flags)) ++ return 0; ++ ++ if (mutex->depth++ == 0) ++ mutex->flags = flags; ++ ++ return 1; + } + + static inline void __metal_mutex_acquire(metal_mutex_t *mutex) + { +- nxmutex_lock(mutex); ++ irqstate_t flags; ++ ++ if (mutex->is_mutex) { ++ nxrmutex_lock(&mutex->mutex); ++ return; ++ } ++ ++ flags = rspin_lock_irqsave_nopreempt(&mutex->lock); ++ if (mutex->depth++ == 0) ++ mutex->flags = flags; + } + + static inline void __metal_mutex_release(metal_mutex_t *mutex) + { +- nxmutex_unlock(mutex); ++ irqstate_t flags; ++ ++ if (mutex->is_mutex) { ++ nxrmutex_unlock(&mutex->mutex); ++ return; ++ } ++ ++ /* An inner release must leave the irqs disabled. */ ++ flags = --mutex->depth == 0 ? mutex->flags : up_irq_save(); ++ rspin_unlock_irqrestore_nopreempt(&mutex->lock, flags); + } + + static inline int __metal_mutex_is_acquired(metal_mutex_t *mutex) + { +- return nxmutex_is_locked(mutex); ++ if (mutex->is_mutex) ++ return nxrmutex_is_locked(&mutex->mutex); ++ ++ return mutex->depth > 0; + } + + #ifdef __cplusplus +-- +2.34.1 + diff --git a/openamp/libmetal.cmake b/openamp/libmetal.cmake index cac8ac3b779..c2093280d81 100644 --- a/openamp/libmetal.cmake +++ b/openamp/libmetal.cmake @@ -41,7 +41,7 @@ if(NOT EXISTS ${CMAKE_CURRENT_LIST_DIR}/libmetal) && patch -p0 -d ${CMAKE_CURRENT_LIST_DIR} < ${CMAKE_CURRENT_LIST_DIR}/0002-libmetal-atomic-enable-64-bit-atomic-by-toolchain-bu.patch && patch -p0 -d ${CMAKE_CURRENT_LIST_DIR} < - ${CMAKE_CURRENT_LIST_DIR}/0003-mutex-change-the-libmetal-nuttx-mutex-to-recursive-m.patch + ${CMAKE_CURRENT_LIST_DIR}/0003-mutex-make-the-libmetal-nuttx-mutex-a-recursive-spinlock.patch DOWNLOAD_NO_PROGRESS true TIMEOUT 30) diff --git a/openamp/libmetal.defs b/openamp/libmetal.defs index 902caae1634..e10a7a295cf 100644 --- a/openamp/libmetal.defs +++ b/openamp/libmetal.defs @@ -84,7 +84,7 @@ libmetal.zip: mv libmetal-$(LIBMETAL_VERSION) libmetal $(Q) patch -p0 < 0001-libmetal-cmake-set-HAVE_STDATOMIC_H-default-true-in-.patch $(Q) patch -p0 < 0002-libmetal-atomic-enable-64-bit-atomic-by-toolchain-bu.patch - $(Q) patch -p0 < 0003-mutex-change-the-libmetal-nuttx-mutex-to-recursive-m.patch + $(Q) patch -p0 < 0003-mutex-make-the-libmetal-nuttx-mutex-a-recursive-spinlock.patch .libmetal_headers: libmetal.zip else
