You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/05/16 10:07:27 UTC
[GitHub] [incubator-nuttx] anjiahao1 opened a new pull request, #6279: include:add recursive lock
anjiahao1 opened a new pull request, #6279:
URL: https://github.com/apache/incubator-nuttx/pull/6279
Signed-off-by: anjiahao <an...@xiaomi.com>
## Summary
add recurive lock
## plan
1. Replace all duplicate recursive locks with the current recursive lock implementation
2. Use mutex to replace all places in nuttx that use sem as a lock
3.Then change the default behavior of sem，it associated with [RP5070](https://github.com/apache/incubator-nuttx/pull/5070)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6279: include:add recursive lock
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6279:
URL: https://github.com/apache/incubator-nuttx/pull/6279#discussion_r876477533
##########
include/nuttx/mutex.h:
##########
@@ -205,6 +214,200 @@ static inline int nxmutex_unlock(FAR mutex_t *mutex)
return nxsem_post(mutex);
}
+/****************************************************************************
+ * Name: nxrmutex_init
+ *
+ * Description:
+ * This function initializes the UNNAMED recursive mutex. Following a
+ * successful call to nxrmutex_init(), the recursive mutex may be used in
+ * subsequent calls to nxrmutex_lock(), nxrmutex_unlock(),
+ * and nxrmutex_trylock(). The recursive mutex remains usable
+ * until it is destroyed.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be initialized
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_init(FAR rmutex_t *rmutex)
+{
+ rmutex->count = 0;
+ rmutex->holder = INVALID_PROCESS_ID;
+ return nxmutex_init(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nxrmutex_destroy
+ *
+ * Description:
+ * This function destroy the UNNAMED recursive mutex.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be destroyed
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_destroy(FAR rmutex_t *rmutex)
+{
+ return nxmutex_destroy(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nrxmutex_lock
+ *
+ * Description:
+ * This function attempts to lock the recursive mutex referenced by
+ * 'rmutex'.The recursive mutex can be locked multiple times in the same
+ * thread.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_lock(FAR rmutex_t *rmutex)
Review Comment:
I think that we need to start handling `inline` similar to other projects that are multi-compiler friendly. That means that we need to have update `compiler.h` with:
```
# define INLINE inline
# define inline_function __attribute__ ((always_inline,no_instrument_function))
```
I mean something similar to `__INLINE`, `__FORCEINLINE`, `__STATIC_INLINE` and `__STATIC_FORCEINLINE` in CMSIS.
I started shuffling things around in https://github.com/apache/incubator-nuttx/pull/5201, but didn't find enough time to finish it.
I'm not against inline usage, but just want it to be integrated in proper manner and with respect to all archs as `inline` is just a hint for compiler and most probably if you put static function into a header and it is small enough the compiler will inline it even without `inline` keyword if proper optimization level is used
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #6279: include:add recursive lock
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #6279:
URL: https://github.com/apache/incubator-nuttx/pull/6279#issuecomment-1134878227
@davids5 and @pkarashchenko could you take a look again?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] anjiahao1 commented on a diff in pull request #6279: include:add recursive lock
Posted by GitBox <gi...@apache.org>.
anjiahao1 commented on code in PR #6279:
URL: https://github.com/apache/incubator-nuttx/pull/6279#discussion_r875840077
##########
include/nuttx/mutex.h:
##########
@@ -205,6 +214,200 @@ static inline int nxmutex_unlock(FAR mutex_t *mutex)
return nxsem_post(mutex);
}
+/****************************************************************************
+ * Name: nxrmutex_init
+ *
+ * Description:
+ * This function initializes the UNNAMED recursive mutex. Following a
+ * successful call to nxrmutex_init(), the recursive mutex may be used in
+ * subsequent calls to nxrmutex_lock(), nxrmutex_unlock(),
+ * and nxrmutex_trylock(). The recursive mutex remains usable
+ * until it is destroyed.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be initialized
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_init(FAR rmutex_t *rmutex)
+{
+ rmutex->count = 0;
+ rmutex->holder = INVALID_PROCESS_ID;
+ return nxmutex_init(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nxrmutex_destroy
+ *
+ * Description:
+ * This function destroy the UNNAMED recursive mutex.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be destroyed
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_destroy(FAR rmutex_t *rmutex)
+{
+ return nxmutex_destroy(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nrxmutex_lock
+ *
+ * Description:
+ * This function attempts to lock the recursive mutex referenced by
+ * 'rmutex'.The recursive mutex can be locked multiple times in the same
+ * thread.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_lock(FAR rmutex_t *rmutex)
+{
+ int ret = OK;
+ pid_t tid = gettid();
+
+ if (rmutex->holder == tid)
+ {
+ DEBUGASSERT(rmutex->count++ < UINT16_MAX);
+ }
+ else
+ {
+ ret = nxmutex_lock(&rmutex->mutex);
+ if (ret >= OK)
+ {
+ rmutex->holder = tid;
+ rmutex->count = 1;
+ }
+ }
+
+ return ret;
+}
+
+/****************************************************************************
+ * Name: nxrmutex_trylock
+ *
+ * Description:
+ * This function locks the recursive mutex if the recursive mutex is
+ * currently not locked or the same thread call.
+ * If the recursive mutex is locked and other thread call it,
+ * the call returns without blocking.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ * -EINVAL - Invalid attempt to lock the recursive mutex
+ * -EAGAIN - The recursive mutex is not available.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_trylock(FAR rmutex_t *rmutex)
+{
+ int ret = OK;
+ pid_t tid = gettid();
+
+ if (rmutex->holder == tid)
+ {
+ DEBUGASSERT(rmutex->count++ < UINT16_MAX);
+ }
+ else
+ {
+ ret = nxmutex_trylock(&rmutex->mutex);
+ if (ret >= OK)
+ {
+ rmutex->holder = tid;
+ rmutex->count = 1;
+ }
+ }
+
+ return ret;
+}
+
+/****************************************************************************
+ * Name: nxrmutex_is_locked
+ *
+ * Description:
+ * This function get the lock state the recursive mutex
+ * referenced by 'rmutex'.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ *
+ ****************************************************************************/
+
+static inline bool nxrmutex_is_locked(FAR rmutex_t *rmutex)
+{
+ return rmutex->count ? true : false;
Review Comment:
ok
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6279: include:add recursive lock
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6279:
URL: https://github.com/apache/incubator-nuttx/pull/6279#discussion_r878096488
##########
include/nuttx/mutex.h:
##########
@@ -205,6 +214,200 @@ static inline int nxmutex_unlock(FAR mutex_t *mutex)
return nxsem_post(mutex);
}
+/****************************************************************************
+ * Name: nxrmutex_init
+ *
+ * Description:
+ * This function initializes the UNNAMED recursive mutex. Following a
+ * successful call to nxrmutex_init(), the recursive mutex may be used in
+ * subsequent calls to nxrmutex_lock(), nxrmutex_unlock(),
+ * and nxrmutex_trylock(). The recursive mutex remains usable
+ * until it is destroyed.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be initialized
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_init(FAR rmutex_t *rmutex)
+{
+ rmutex->count = 0;
+ rmutex->holder = INVALID_PROCESS_ID;
+ return nxmutex_init(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nxrmutex_destroy
+ *
+ * Description:
+ * This function destroy the UNNAMED recursive mutex.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be destroyed
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_destroy(FAR rmutex_t *rmutex)
+{
+ return nxmutex_destroy(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nrxmutex_lock
+ *
+ * Description:
+ * This function attempts to lock the recursive mutex referenced by
+ * 'rmutex'.The recursive mutex can be locked multiple times in the same
+ * thread.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_lock(FAR rmutex_t *rmutex)
Review Comment:
BWT do you know why we have `no_instrument_function` in `define inline_function`? I mean that there is a separate `# define noinstrument_function __attribute__ ((no_instrument_function))`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] anjiahao1 commented on a diff in pull request #6279: include:add recursive lock
Posted by GitBox <gi...@apache.org>.
anjiahao1 commented on code in PR #6279:
URL: https://github.com/apache/incubator-nuttx/pull/6279#discussion_r875831904
##########
include/nuttx/mutex.h:
##########
@@ -205,6 +214,200 @@ static inline int nxmutex_unlock(FAR mutex_t *mutex)
return nxsem_post(mutex);
}
+/****************************************************************************
+ * Name: nxrmutex_init
+ *
+ * Description:
+ * This function initializes the UNNAMED recursive mutex. Following a
+ * successful call to nxrmutex_init(), the recursive mutex may be used in
+ * subsequent calls to nxrmutex_lock(), nxrmutex_unlock(),
+ * and nxrmutex_trylock(). The recursive mutex remains usable
+ * until it is destroyed.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be initialized
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_init(FAR rmutex_t *rmutex)
+{
+ rmutex->count = 0;
+ rmutex->holder = INVALID_PROCESS_ID;
+ return nxmutex_init(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nxrmutex_destroy
+ *
+ * Description:
+ * This function destroy the UNNAMED recursive mutex.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be destroyed
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_destroy(FAR rmutex_t *rmutex)
+{
+ return nxmutex_destroy(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nrxmutex_lock
+ *
+ * Description:
+ * This function attempts to lock the recursive mutex referenced by
+ * 'rmutex'.The recursive mutex can be locked multiple times in the same
+ * thread.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_lock(FAR rmutex_t *rmutex)
+{
+ int ret = OK;
+ pid_t tid = gettid();
+
+ if (rmutex->holder == tid)
+ {
+ DEBUGASSERT(rmutex->count++ < UINT16_MAX);
+ }
+ else
+ {
+ ret = nxmutex_lock(&rmutex->mutex);
+ if (ret >= OK)
+ {
+ rmutex->holder = tid;
+ rmutex->count = 1;
+ }
+ }
+
+ return ret;
+}
+
+/****************************************************************************
+ * Name: nxrmutex_trylock
+ *
+ * Description:
+ * This function locks the recursive mutex if the recursive mutex is
+ * currently not locked or the same thread call.
+ * If the recursive mutex is locked and other thread call it,
+ * the call returns without blocking.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ * -EINVAL - Invalid attempt to lock the recursive mutex
+ * -EAGAIN - The recursive mutex is not available.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_trylock(FAR rmutex_t *rmutex)
+{
+ int ret = OK;
+ pid_t tid = gettid();
+
+ if (rmutex->holder == tid)
+ {
+ DEBUGASSERT(rmutex->count++ < UINT16_MAX);
+ }
+ else
+ {
+ ret = nxmutex_trylock(&rmutex->mutex);
+ if (ret >= OK)
+ {
+ rmutex->holder = tid;
+ rmutex->count = 1;
+ }
+ }
+
+ return ret;
+}
+
+/****************************************************************************
+ * Name: nxrmutex_is_locked
+ *
+ * Description:
+ * This function get the lock state the recursive mutex
+ * referenced by 'rmutex'.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ *
+ ****************************************************************************/
+
+static inline bool nxrmutex_is_locked(FAR rmutex_t *rmutex)
+{
+ return rmutex->count ? true : false;
+}
+
+/****************************************************************************
+ * Name: nxrmutex_unlock
+ *
+ * Description:
+ * This function attempts to unlock the recursive mutex
+ * referenced by 'rmutex'.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ * Assumptions:
+ * This function may be called from an interrupt handler.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_unlock(FAR rmutex_t *rmutex)
+{
+ int ret = OK;
+ pid_t tid = gettid();
+
+ if (rmutex->holder != tid)
+ {
+ return -EPERM;
+ }
+
+ DEBUGASSERT(rmutex->count > 0);
+ if (--rmutex->count == 0)
+ {
+ rmutex->holder = INVALID_PROCESS_ID;
+ ret = nxmutex_unlock(&rmutex->mutex);
Review Comment:
i think
1. if `rmutex->count > 0` just `rmutex->count--`
2. if `rmutex->count == 0` then call`nxmutex_unlock`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6279: include:add recursive lock
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6279:
URL: https://github.com/apache/incubator-nuttx/pull/6279#discussion_r876477533
##########
include/nuttx/mutex.h:
##########
@@ -205,6 +214,200 @@ static inline int nxmutex_unlock(FAR mutex_t *mutex)
return nxsem_post(mutex);
}
+/****************************************************************************
+ * Name: nxrmutex_init
+ *
+ * Description:
+ * This function initializes the UNNAMED recursive mutex. Following a
+ * successful call to nxrmutex_init(), the recursive mutex may be used in
+ * subsequent calls to nxrmutex_lock(), nxrmutex_unlock(),
+ * and nxrmutex_trylock(). The recursive mutex remains usable
+ * until it is destroyed.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be initialized
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_init(FAR rmutex_t *rmutex)
+{
+ rmutex->count = 0;
+ rmutex->holder = INVALID_PROCESS_ID;
+ return nxmutex_init(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nxrmutex_destroy
+ *
+ * Description:
+ * This function destroy the UNNAMED recursive mutex.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be destroyed
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_destroy(FAR rmutex_t *rmutex)
+{
+ return nxmutex_destroy(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nrxmutex_lock
+ *
+ * Description:
+ * This function attempts to lock the recursive mutex referenced by
+ * 'rmutex'.The recursive mutex can be locked multiple times in the same
+ * thread.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_lock(FAR rmutex_t *rmutex)
Review Comment:
I think that we need to start handling `inline` similar to other projects that are multi-compiler friendly. That means that we need to have update `compiler.h` with:
```
# define INLINE inline
# define inline_function __attribute__ ((always_inline,no_instrument_function))
```
I mean something similar to `__INLINE`, `__FORCEINLINE`, `__STATIC_INLINE` and `__STATIC_FORCEINLINE` in CMSIS.
I started shuffling things around in https://github.com/apache/incubator-nuttx/pull/5201, but didn't find enough time to finish it.
I'm not against inline usage, but just want it to be integrated in proper manner and with respect to all archs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] davids5 commented on a diff in pull request #6279: include:add recursive lock
Posted by GitBox <gi...@apache.org>.
davids5 commented on code in PR #6279:
URL: https://github.com/apache/incubator-nuttx/pull/6279#discussion_r877035967
##########
include/nuttx/mutex.h:
##########
@@ -205,6 +216,208 @@ static inline int nxmutex_unlock(FAR mutex_t *mutex)
return nxsem_post(mutex);
}
+/****************************************************************************
+ * Name: nxrmutex_init
+ *
+ * Description:
+ * This function initializes the UNNAMED recursive mutex. Following a
+ * successful call to nxrmutex_init(), the recursive mutex may be used in
+ * subsequent calls to nxrmutex_lock(), nxrmutex_unlock(),
+ * and nxrmutex_trylock(). The recursive mutex remains usable
+ * until it is destroyed.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be initialized
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_init(FAR rmutex_t *rmutex)
+{
+ rmutex->count = 0;
+ rmutex->holder = INVALID_PROCESS_ID;
+ return nxmutex_init(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nxrmutex_destroy
+ *
+ * Description:
+ * This function destroy the UNNAMED recursive mutex.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be destroyed
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_destroy(FAR rmutex_t *rmutex)
+{
+ return nxmutex_destroy(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nrxmutex_lock
+ *
+ * Description:
+ * This function attempts to lock the recursive mutex referenced by
+ * 'rmutex'.The recursive mutex can be locked multiple times in the same
+ * thread.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_lock(FAR rmutex_t *rmutex)
+{
+ int ret = OK;
+ pid_t tid = gettid();
+
+ if (rmutex->holder == tid)
+ {
+ DEBUGASSERT(rmutex->count++ < UINT16_MAX);
Review Comment:
```suggestion
rmutex->count++;
DEBUGASSERT(rmutex->count < UINT16_MAX);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6279: include:add recursive lock
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6279:
URL: https://github.com/apache/incubator-nuttx/pull/6279#discussion_r876477533
##########
include/nuttx/mutex.h:
##########
@@ -205,6 +214,200 @@ static inline int nxmutex_unlock(FAR mutex_t *mutex)
return nxsem_post(mutex);
}
+/****************************************************************************
+ * Name: nxrmutex_init
+ *
+ * Description:
+ * This function initializes the UNNAMED recursive mutex. Following a
+ * successful call to nxrmutex_init(), the recursive mutex may be used in
+ * subsequent calls to nxrmutex_lock(), nxrmutex_unlock(),
+ * and nxrmutex_trylock(). The recursive mutex remains usable
+ * until it is destroyed.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be initialized
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_init(FAR rmutex_t *rmutex)
+{
+ rmutex->count = 0;
+ rmutex->holder = INVALID_PROCESS_ID;
+ return nxmutex_init(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nxrmutex_destroy
+ *
+ * Description:
+ * This function destroy the UNNAMED recursive mutex.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be destroyed
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_destroy(FAR rmutex_t *rmutex)
+{
+ return nxmutex_destroy(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nrxmutex_lock
+ *
+ * Description:
+ * This function attempts to lock the recursive mutex referenced by
+ * 'rmutex'.The recursive mutex can be locked multiple times in the same
+ * thread.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_lock(FAR rmutex_t *rmutex)
Review Comment:
I think that we need to start handling `inline` similar to other projects that are multi-compiler friendly. That means that we need to have update `compiler.h` with:
```
# define INLINE inline
# define inline_function __attribute__ ((always_inline,no_instrument_function))
```
I mean something similar to `__INLINE`, `__FORCEINLINE`, `__STATIC_INLINE` and `__STATIC_FORCEINLINE` in CMSIS.
I started shuffling things around in https://github.com/apache/incubator-nuttx/pull/5201, but didn't find enough time to finish it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6279: include:add recursive lock
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6279:
URL: https://github.com/apache/incubator-nuttx/pull/6279#discussion_r875839220
##########
include/nuttx/mutex.h:
##########
@@ -205,6 +214,200 @@ static inline int nxmutex_unlock(FAR mutex_t *mutex)
return nxsem_post(mutex);
}
+/****************************************************************************
+ * Name: nxrmutex_init
+ *
+ * Description:
+ * This function initializes the UNNAMED recursive mutex. Following a
+ * successful call to nxrmutex_init(), the recursive mutex may be used in
+ * subsequent calls to nxrmutex_lock(), nxrmutex_unlock(),
+ * and nxrmutex_trylock(). The recursive mutex remains usable
+ * until it is destroyed.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be initialized
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_init(FAR rmutex_t *rmutex)
+{
+ rmutex->count = 0;
+ rmutex->holder = INVALID_PROCESS_ID;
+ return nxmutex_init(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nxrmutex_destroy
+ *
+ * Description:
+ * This function destroy the UNNAMED recursive mutex.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be destroyed
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_destroy(FAR rmutex_t *rmutex)
+{
+ return nxmutex_destroy(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nrxmutex_lock
+ *
+ * Description:
+ * This function attempts to lock the recursive mutex referenced by
+ * 'rmutex'.The recursive mutex can be locked multiple times in the same
+ * thread.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_lock(FAR rmutex_t *rmutex)
+{
+ int ret = OK;
+ pid_t tid = gettid();
+
+ if (rmutex->holder == tid)
+ {
+ DEBUGASSERT(rmutex->count++ < UINT16_MAX);
+ }
+ else
+ {
+ ret = nxmutex_lock(&rmutex->mutex);
+ if (ret >= OK)
+ {
+ rmutex->holder = tid;
+ rmutex->count = 1;
+ }
+ }
+
+ return ret;
+}
+
+/****************************************************************************
+ * Name: nxrmutex_trylock
+ *
+ * Description:
+ * This function locks the recursive mutex if the recursive mutex is
+ * currently not locked or the same thread call.
+ * If the recursive mutex is locked and other thread call it,
+ * the call returns without blocking.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ * -EINVAL - Invalid attempt to lock the recursive mutex
+ * -EAGAIN - The recursive mutex is not available.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_trylock(FAR rmutex_t *rmutex)
+{
+ int ret = OK;
+ pid_t tid = gettid();
+
+ if (rmutex->holder == tid)
+ {
+ DEBUGASSERT(rmutex->count++ < UINT16_MAX);
+ }
+ else
+ {
+ ret = nxmutex_trylock(&rmutex->mutex);
+ if (ret >= OK)
+ {
+ rmutex->holder = tid;
+ rmutex->count = 1;
+ }
+ }
+
+ return ret;
+}
+
+/****************************************************************************
+ * Name: nxrmutex_is_locked
+ *
+ * Description:
+ * This function get the lock state the recursive mutex
+ * referenced by 'rmutex'.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ *
+ ****************************************************************************/
+
+static inline bool nxrmutex_is_locked(FAR rmutex_t *rmutex)
+{
+ return rmutex->count ? true : false;
Review Comment:
`rmutex->count > 0` is bool
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] anchao commented on a diff in pull request #6279: include:add recursive lock
Posted by GitBox <gi...@apache.org>.
anchao commented on code in PR #6279:
URL: https://github.com/apache/incubator-nuttx/pull/6279#discussion_r879932826
##########
include/nuttx/mutex.h:
##########
@@ -205,6 +214,200 @@ static inline int nxmutex_unlock(FAR mutex_t *mutex)
return nxsem_post(mutex);
}
+/****************************************************************************
+ * Name: nxrmutex_init
+ *
+ * Description:
+ * This function initializes the UNNAMED recursive mutex. Following a
+ * successful call to nxrmutex_init(), the recursive mutex may be used in
+ * subsequent calls to nxrmutex_lock(), nxrmutex_unlock(),
+ * and nxrmutex_trylock(). The recursive mutex remains usable
+ * until it is destroyed.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be initialized
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_init(FAR rmutex_t *rmutex)
+{
+ rmutex->count = 0;
+ rmutex->holder = INVALID_PROCESS_ID;
+ return nxmutex_init(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nxrmutex_destroy
+ *
+ * Description:
+ * This function destroy the UNNAMED recursive mutex.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be destroyed
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_destroy(FAR rmutex_t *rmutex)
+{
+ return nxmutex_destroy(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nrxmutex_lock
+ *
+ * Description:
+ * This function attempts to lock the recursive mutex referenced by
+ * 'rmutex'.The recursive mutex can be locked multiple times in the same
+ * thread.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_lock(FAR rmutex_t *rmutex)
Review Comment:
@xiaoxiang781216 @pkarashchenko
Some simple functions seem to get better performance if implemented in inline.
The following is an optimization of up_interrupt_context(). The inline version saves more instruction cycles:
Test on cortex-R:
Origin:
```
0000bb88 <up_interrupt_context>:
bb88: e59f300c ldr r3, [pc, #12] ; bb9c <up_interrupt_context+0x14>
bb8c: e5930000 ldr r0, [r3]
bb90: e2500000 subs r0, r0, #0
bb94: 13a00001 movne r0, #1
bb98: e12fff1e bx lr
bb9c: 0001a978 .word 0x0001a978
int file_mq_send(FAR struct file *mq, FAR const char *msg, size_t msglen,
unsigned int prio)
{
1c38: e92d47f3 push {r0, r1, r4, r5, r6, r7, r8, r9, sl, lr}
1c3c: e1a09003 mov r9, r3
...
1c80: eb000556 bl 31e0 <sched_lock>
1c84: e10fa000 mrs sl, CPSR
1c88: f10c0080 cpsid i
1c8c: eb0027bd bl bb88 <up_interrupt_context>
1c90: e3500000 cmp r0, #0
1c94: 0a000005 beq 1cb0 <file_mq_send+0x78>
...
```
inline:
```
int file_mq_send(FAR struct file *mq, FAR const char *msg, size_t msglen,
unsigned int prio)
{
1c38: e92d47f3 push {r0, r1, r4, r5, r6, r7, r8, r9, sl, lr}
1c3c: e1a09003 mov r9, r3
...
1c8c: e59f3078 ldr r3, [pc, #120] ; 1d0c <file_mq_send+0xd4>
1c90: e5933000 ldr r3, [r3]
1c94: e3530000 cmp r3, #0
1c98: 0a000005 beq 1cb4 <file_mq_send+0x7c>
```
you can see that the optimization version save about 6 instructions in an inline process,
in addition, some functions(file_mq_send) will call up_interrupt_context() repeatedly, eg:
```
file_mq_send
|
->up_interrupt_context
->nxmq_alloc_msg
->up_interrupt_context
```
In this way, we can save about 12 instructions in a single send,
it is worth noting that this optimization is more obvious in some scenarios:
mq_send perf count test:
mq send (low prio -> hi prio)
thread 1 (prio 100):
```
begin = up_perf_gettime();
ret = mq_send(mqdes, msg, 1, 1);
```
thread 2 (prio 101):
```
ret = mq_receive(mqdes, msg, sizeof(msg), NULL);
end = up_perf_gettime();
```
Before optimization:
`end - begin = 718`
Optimized:
`end - begin = 688`
optimized version needs 688 cycle counts, but this is far from other RTOS....
From the data below, you can see that the performance gap between nuttx and threadx is very large,
```
nuttx threadx (cycle)
mq_send(hi-low) 833 282
mq_send(low-hi) 688 172
mq_send(multi) 1978 467
sem_post(low-hi) 225 147
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6279: include:add recursive lock
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6279:
URL: https://github.com/apache/incubator-nuttx/pull/6279#discussion_r875216267
##########
include/nuttx/mutex.h:
##########
@@ -205,6 +214,200 @@ static inline int nxmutex_unlock(FAR mutex_t *mutex)
return nxsem_post(mutex);
}
+/****************************************************************************
+ * Name: nxrmutex_init
+ *
+ * Description:
+ * This function initializes the UNNAMED recursive mutex. Following a
+ * successful call to nxrmutex_init(), the recursive mutex may be used in
+ * subsequent calls to nxrmutex_lock(), nxrmutex_unlock(),
+ * and nxrmutex_trylock(). The recursive mutex remains usable
+ * until it is destroyed.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be initialized
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_init(FAR rmutex_t *rmutex)
+{
+ rmutex->count = 0;
+ rmutex->holder = INVALID_PROCESS_ID;
+ return nxmutex_init(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nxrmutex_destroy
+ *
+ * Description:
+ * This function destroy the UNNAMED recursive mutex.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be destroyed
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_destroy(FAR rmutex_t *rmutex)
+{
+ return nxmutex_destroy(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nrxmutex_lock
+ *
+ * Description:
+ * This function attempts to lock the recursive mutex referenced by
+ * 'rmutex'.The recursive mutex can be locked multiple times in the same
+ * thread.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_lock(FAR rmutex_t *rmutex)
Review Comment:
I'm not sure if using `inline` in common code is a good idea. I know that we have fair amount of that already, but the question is should we continue to do that. @xiaoxiang781216 what do you think?
##########
include/nuttx/mutex.h:
##########
@@ -205,6 +214,200 @@ static inline int nxmutex_unlock(FAR mutex_t *mutex)
return nxsem_post(mutex);
}
+/****************************************************************************
+ * Name: nxrmutex_init
+ *
+ * Description:
+ * This function initializes the UNNAMED recursive mutex. Following a
+ * successful call to nxrmutex_init(), the recursive mutex may be used in
+ * subsequent calls to nxrmutex_lock(), nxrmutex_unlock(),
+ * and nxrmutex_trylock(). The recursive mutex remains usable
+ * until it is destroyed.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be initialized
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_init(FAR rmutex_t *rmutex)
+{
+ rmutex->count = 0;
+ rmutex->holder = INVALID_PROCESS_ID;
+ return nxmutex_init(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nxrmutex_destroy
+ *
+ * Description:
+ * This function destroy the UNNAMED recursive mutex.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be destroyed
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_destroy(FAR rmutex_t *rmutex)
+{
+ return nxmutex_destroy(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nrxmutex_lock
+ *
+ * Description:
+ * This function attempts to lock the recursive mutex referenced by
+ * 'rmutex'.The recursive mutex can be locked multiple times in the same
+ * thread.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_lock(FAR rmutex_t *rmutex)
+{
+ int ret = OK;
+ pid_t tid = gettid();
+
+ if (rmutex->holder == tid)
+ {
+ DEBUGASSERT(rmutex->count++ < UINT16_MAX);
+ }
+ else
+ {
+ ret = nxmutex_lock(&rmutex->mutex);
+ if (ret >= OK)
+ {
+ rmutex->holder = tid;
+ rmutex->count = 1;
+ }
+ }
+
+ return ret;
+}
+
+/****************************************************************************
+ * Name: nxrmutex_trylock
+ *
+ * Description:
+ * This function locks the recursive mutex if the recursive mutex is
+ * currently not locked or the same thread call.
+ * If the recursive mutex is locked and other thread call it,
+ * the call returns without blocking.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ * -EINVAL - Invalid attempt to lock the recursive mutex
+ * -EAGAIN - The recursive mutex is not available.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_trylock(FAR rmutex_t *rmutex)
+{
+ int ret = OK;
+ pid_t tid = gettid();
+
+ if (rmutex->holder == tid)
+ {
+ DEBUGASSERT(rmutex->count++ < UINT16_MAX);
+ }
+ else
+ {
+ ret = nxmutex_trylock(&rmutex->mutex);
+ if (ret >= OK)
+ {
+ rmutex->holder = tid;
+ rmutex->count = 1;
+ }
+ }
+
+ return ret;
+}
+
+/****************************************************************************
+ * Name: nxrmutex_is_locked
+ *
+ * Description:
+ * This function get the lock state the recursive mutex
+ * referenced by 'rmutex'.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ *
+ ****************************************************************************/
+
+static inline bool nxrmutex_is_locked(FAR rmutex_t *rmutex)
+{
+ return rmutex->count ? true : false;
+}
+
+/****************************************************************************
+ * Name: nxrmutex_unlock
+ *
+ * Description:
+ * This function attempts to unlock the recursive mutex
+ * referenced by 'rmutex'.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ * Assumptions:
+ * This function may be called from an interrupt handler.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_unlock(FAR rmutex_t *rmutex)
+{
+ int ret = OK;
+ pid_t tid = gettid();
+
+ if (rmutex->holder != tid)
+ {
+ return -EPERM;
+ }
+
+ DEBUGASSERT(rmutex->count > 0);
+ if (--rmutex->count == 0)
+ {
+ rmutex->holder = INVALID_PROCESS_ID;
+ ret = nxmutex_unlock(&rmutex->mutex);
Review Comment:
I'm not sure is that matter, but maybe code can be reorganized in the way:
1. check `rmutex->count > 0`
2. `ret = nxmutex_unlock(&rmutex->mutex);`
3. if `ret == OK` -> `--rmutex->count` -> `rmutex->holder = INVALID_PROCESS_ID;`
Do you see it reasonable?
##########
include/nuttx/mutex.h:
##########
@@ -205,6 +214,200 @@ static inline int nxmutex_unlock(FAR mutex_t *mutex)
return nxsem_post(mutex);
}
+/****************************************************************************
+ * Name: nxrmutex_init
+ *
+ * Description:
+ * This function initializes the UNNAMED recursive mutex. Following a
+ * successful call to nxrmutex_init(), the recursive mutex may be used in
+ * subsequent calls to nxrmutex_lock(), nxrmutex_unlock(),
+ * and nxrmutex_trylock(). The recursive mutex remains usable
+ * until it is destroyed.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be initialized
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_init(FAR rmutex_t *rmutex)
+{
+ rmutex->count = 0;
+ rmutex->holder = INVALID_PROCESS_ID;
+ return nxmutex_init(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nxrmutex_destroy
+ *
+ * Description:
+ * This function destroy the UNNAMED recursive mutex.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be destroyed
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_destroy(FAR rmutex_t *rmutex)
+{
+ return nxmutex_destroy(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nrxmutex_lock
+ *
+ * Description:
+ * This function attempts to lock the recursive mutex referenced by
+ * 'rmutex'.The recursive mutex can be locked multiple times in the same
+ * thread.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_lock(FAR rmutex_t *rmutex)
+{
+ int ret = OK;
+ pid_t tid = gettid();
+
+ if (rmutex->holder == tid)
+ {
+ DEBUGASSERT(rmutex->count++ < UINT16_MAX);
+ }
+ else
+ {
+ ret = nxmutex_lock(&rmutex->mutex);
+ if (ret >= OK)
+ {
+ rmutex->holder = tid;
+ rmutex->count = 1;
+ }
+ }
+
+ return ret;
+}
+
+/****************************************************************************
+ * Name: nxrmutex_trylock
+ *
+ * Description:
+ * This function locks the recursive mutex if the recursive mutex is
+ * currently not locked or the same thread call.
+ * If the recursive mutex is locked and other thread call it,
+ * the call returns without blocking.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ * -EINVAL - Invalid attempt to lock the recursive mutex
+ * -EAGAIN - The recursive mutex is not available.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_trylock(FAR rmutex_t *rmutex)
+{
+ int ret = OK;
+ pid_t tid = gettid();
+
+ if (rmutex->holder == tid)
+ {
+ DEBUGASSERT(rmutex->count++ < UINT16_MAX);
+ }
+ else
+ {
+ ret = nxmutex_trylock(&rmutex->mutex);
+ if (ret >= OK)
+ {
+ rmutex->holder = tid;
+ rmutex->count = 1;
+ }
+ }
+
+ return ret;
+}
+
+/****************************************************************************
+ * Name: nxrmutex_is_locked
+ *
+ * Description:
+ * This function get the lock state the recursive mutex
+ * referenced by 'rmutex'.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ *
+ ****************************************************************************/
+
+static inline bool nxrmutex_is_locked(FAR rmutex_t *rmutex)
+{
+ return rmutex->count ? true : false;
Review Comment:
```suggestion
return rmutex->count > 0;
```
##########
include/nuttx/mutex.h:
##########
@@ -205,6 +214,200 @@ static inline int nxmutex_unlock(FAR mutex_t *mutex)
return nxsem_post(mutex);
}
+/****************************************************************************
+ * Name: nxrmutex_init
+ *
+ * Description:
+ * This function initializes the UNNAMED recursive mutex. Following a
+ * successful call to nxrmutex_init(), the recursive mutex may be used in
+ * subsequent calls to nxrmutex_lock(), nxrmutex_unlock(),
+ * and nxrmutex_trylock(). The recursive mutex remains usable
+ * until it is destroyed.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be initialized
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_init(FAR rmutex_t *rmutex)
Review Comment:
Probably it is good to add a `rmutex_t` static initialiser as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6279: include:add recursive lock
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6279:
URL: https://github.com/apache/incubator-nuttx/pull/6279#discussion_r876096009
##########
include/nuttx/mutex.h:
##########
@@ -205,6 +214,200 @@ static inline int nxmutex_unlock(FAR mutex_t *mutex)
return nxsem_post(mutex);
}
+/****************************************************************************
+ * Name: nxrmutex_init
+ *
+ * Description:
+ * This function initializes the UNNAMED recursive mutex. Following a
+ * successful call to nxrmutex_init(), the recursive mutex may be used in
+ * subsequent calls to nxrmutex_lock(), nxrmutex_unlock(),
+ * and nxrmutex_trylock(). The recursive mutex remains usable
+ * until it is destroyed.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be initialized
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_init(FAR rmutex_t *rmutex)
+{
+ rmutex->count = 0;
+ rmutex->holder = INVALID_PROCESS_ID;
+ return nxmutex_init(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nxrmutex_destroy
+ *
+ * Description:
+ * This function destroy the UNNAMED recursive mutex.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be destroyed
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_destroy(FAR rmutex_t *rmutex)
+{
+ return nxmutex_destroy(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nrxmutex_lock
+ *
+ * Description:
+ * This function attempts to lock the recursive mutex referenced by
+ * 'rmutex'.The recursive mutex can be locked multiple times in the same
+ * thread.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_lock(FAR rmutex_t *rmutex)
Review Comment:
BTW, since not all functions can be converted to macro equally. If we still forbid to use the inline in common used code, it's very hard to approximate or even exceed other RTOS.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6279: include:add recursive lock
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6279:
URL: https://github.com/apache/incubator-nuttx/pull/6279#discussion_r877309712
##########
include/nuttx/mutex.h:
##########
@@ -205,6 +214,200 @@ static inline int nxmutex_unlock(FAR mutex_t *mutex)
return nxsem_post(mutex);
}
+/****************************************************************************
+ * Name: nxrmutex_init
+ *
+ * Description:
+ * This function initializes the UNNAMED recursive mutex. Following a
+ * successful call to nxrmutex_init(), the recursive mutex may be used in
+ * subsequent calls to nxrmutex_lock(), nxrmutex_unlock(),
+ * and nxrmutex_trylock(). The recursive mutex remains usable
+ * until it is destroyed.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be initialized
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_init(FAR rmutex_t *rmutex)
+{
+ rmutex->count = 0;
+ rmutex->holder = INVALID_PROCESS_ID;
+ return nxmutex_init(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nxrmutex_destroy
+ *
+ * Description:
+ * This function destroy the UNNAMED recursive mutex.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be destroyed
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_destroy(FAR rmutex_t *rmutex)
+{
+ return nxmutex_destroy(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nrxmutex_lock
+ *
+ * Description:
+ * This function attempts to lock the recursive mutex referenced by
+ * 'rmutex'.The recursive mutex can be locked multiple times in the same
+ * thread.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_lock(FAR rmutex_t *rmutex)
Review Comment:
> I think that we need to start handling `inline` similar to other projects that are multi-compiler friendly. That means that we need to have update `compiler.h` with:
>
> ```
> # define INLINE inline
> # define inline_function __attribute__ ((always_inline,no_instrument_function))
> ```
>
> I mean something similar to `__INLINE`, `__FORCEINLINE`, `__STATIC_INLINE` and `__STATIC_FORCEINLINE` in CMSIS. I started shuffling things around in #5201, but didn't find enough time to finish it.
>
> I'm not against inline usage, but just want it to be integrated in proper manner and with respect to all archs as `inline` is just a hint for compiler and most probably if you put static function into a header and it is small enough the compiler will inline it even without `inline` keyword if proper optimization level is used
Sure, I will create a new patch to convert all inline to the new macro after this patch get merged.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6279: include:add recursive lock
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6279:
URL: https://github.com/apache/incubator-nuttx/pull/6279#discussion_r876478088
##########
include/nuttx/mutex.h:
##########
@@ -205,6 +214,200 @@ static inline int nxmutex_unlock(FAR mutex_t *mutex)
return nxsem_post(mutex);
}
+/****************************************************************************
+ * Name: nxrmutex_init
+ *
+ * Description:
+ * This function initializes the UNNAMED recursive mutex. Following a
+ * successful call to nxrmutex_init(), the recursive mutex may be used in
+ * subsequent calls to nxrmutex_lock(), nxrmutex_unlock(),
+ * and nxrmutex_trylock(). The recursive mutex remains usable
+ * until it is destroyed.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be initialized
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_init(FAR rmutex_t *rmutex)
+{
+ rmutex->count = 0;
+ rmutex->holder = INVALID_PROCESS_ID;
+ return nxmutex_init(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nxrmutex_destroy
+ *
+ * Description:
+ * This function destroy the UNNAMED recursive mutex.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be destroyed
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_destroy(FAR rmutex_t *rmutex)
+{
+ return nxmutex_destroy(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nrxmutex_lock
+ *
+ * Description:
+ * This function attempts to lock the recursive mutex referenced by
+ * 'rmutex'.The recursive mutex can be locked multiple times in the same
+ * thread.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_lock(FAR rmutex_t *rmutex)
Review Comment:
I'm also fine with macro.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] anchao commented on a diff in pull request #6279: include:add recursive lock
Posted by GitBox <gi...@apache.org>.
anchao commented on code in PR #6279:
URL: https://github.com/apache/incubator-nuttx/pull/6279#discussion_r879932826
##########
include/nuttx/mutex.h:
##########
@@ -205,6 +214,200 @@ static inline int nxmutex_unlock(FAR mutex_t *mutex)
return nxsem_post(mutex);
}
+/****************************************************************************
+ * Name: nxrmutex_init
+ *
+ * Description:
+ * This function initializes the UNNAMED recursive mutex. Following a
+ * successful call to nxrmutex_init(), the recursive mutex may be used in
+ * subsequent calls to nxrmutex_lock(), nxrmutex_unlock(),
+ * and nxrmutex_trylock(). The recursive mutex remains usable
+ * until it is destroyed.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be initialized
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_init(FAR rmutex_t *rmutex)
+{
+ rmutex->count = 0;
+ rmutex->holder = INVALID_PROCESS_ID;
+ return nxmutex_init(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nxrmutex_destroy
+ *
+ * Description:
+ * This function destroy the UNNAMED recursive mutex.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be destroyed
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_destroy(FAR rmutex_t *rmutex)
+{
+ return nxmutex_destroy(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nrxmutex_lock
+ *
+ * Description:
+ * This function attempts to lock the recursive mutex referenced by
+ * 'rmutex'.The recursive mutex can be locked multiple times in the same
+ * thread.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_lock(FAR rmutex_t *rmutex)
Review Comment:
@xiaoxiang781216 @pkarashchenko
Some simple functions seem to get better performance if implemented in inline.
The following is an optimization of up_interrupt_context(). The inline version saves more instruction cycles:
Test on cortex-R:
Origin:
```
0000bb88 <up_interrupt_context>:
bb88: e59f300c ldr r3, [pc, #12] ; bb9c <up_interrupt_context+0x14>
bb8c: e5930000 ldr r0, [r3]
bb90: e2500000 subs r0, r0, #0
bb94: 13a00001 movne r0, #1
bb98: e12fff1e bx lr
bb9c: 0001a978 .word 0x0001a978
int file_mq_send(FAR struct file *mq, FAR const char *msg, size_t msglen,
unsigned int prio)
{
1c38: e92d47f3 push {r0, r1, r4, r5, r6, r7, r8, r9, sl, lr}
1c3c: e1a09003 mov r9, r3
...
1c80: eb000556 bl 31e0 <sched_lock>
1c84: e10fa000 mrs sl, CPSR
1c88: f10c0080 cpsid i
1c8c: eb0027bd bl bb88 <up_interrupt_context>
1c90: e3500000 cmp r0, #0
1c94: 0a000005 beq 1cb0 <file_mq_send+0x78>
...
```
inline:
```
int file_mq_send(FAR struct file *mq, FAR const char *msg, size_t msglen,
unsigned int prio)
{
1c38: e92d47f3 push {r0, r1, r4, r5, r6, r7, r8, r9, sl, lr}
1c3c: e1a09003 mov r9, r3
...
1c8c: e59f3078 ldr r3, [pc, #120] ; 1d0c <file_mq_send+0xd4>
1c90: e5933000 ldr r3, [r3]
1c94: e3530000 cmp r3, #0
1c98: 0a000005 beq 1cb4 <file_mq_send+0x7c>
```
you can see that the optimization version save about 6 instructions in an inline process,
in addition, some functions(file_mq_send) will call up_interrupt_context() repeatedly, eg:
```
file_mq_send
|
->up_interrupt_context
->nxmq_alloc_msg
->up_interrupt_context
```
In this way, we can save about 12 instructions in a single send,
it is worth noting that this optimization is more obvious in some scenarios:
mq_send perf count test:
mq send (low prio -> hi prio)
thread 1 (prio 100):
```
begin = up_perf_gettime();
ret = mq_send(mqdes, msg, 1, 1);
```
thread 2 (prio 101):
```
ret = mq_receive(mqdes, msg, sizeof(msg), NULL);
end = up_perf_gettime();
```
Before optimization:
`end - begin = 718`
Optimized:
`end - begin = 688`
optimized version needs 688 cycle counts, but this is far from other RTOS....
From the data below, you can be seen that the performance gap between nuttx and threadx is very large,
```
nuttx threadx (cycle)
mq_send(hi-low) 833 282
mq_send(low-hi) 688 172
mq_send(multi) 1978 467
sem_post(low-hi) 225 147
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a diff in pull request #6279: include:add recursive lock
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on code in PR #6279:
URL: https://github.com/apache/incubator-nuttx/pull/6279#discussion_r876240198
##########
include/nuttx/mutex.h:
##########
@@ -42,6 +44,13 @@
typedef sem_t mutex_t;
+typedef struct
+{
+ mutex_t mutex;
+ pid_t holder;
+ uint16_t count;
+} rmutex_t;
Review Comment:
```suggestion
struct rmutex_s
{
mutex_t mutex;
pid_t holder;
uint16_t count;
};
typedef struct rmutex_s rmutex_t;
```
NuttX coding standard forbids unnamed structs:
https://nuttx.apache.org/docs/latest/contributing/coding_style.html#structures
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6279: include:add recursive lock
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6279:
URL: https://github.com/apache/incubator-nuttx/pull/6279#discussion_r876092239
##########
include/nuttx/mutex.h:
##########
@@ -205,6 +214,200 @@ static inline int nxmutex_unlock(FAR mutex_t *mutex)
return nxsem_post(mutex);
}
+/****************************************************************************
+ * Name: nxrmutex_init
+ *
+ * Description:
+ * This function initializes the UNNAMED recursive mutex. Following a
+ * successful call to nxrmutex_init(), the recursive mutex may be used in
+ * subsequent calls to nxrmutex_lock(), nxrmutex_unlock(),
+ * and nxrmutex_trylock(). The recursive mutex remains usable
+ * until it is destroyed.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be initialized
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_init(FAR rmutex_t *rmutex)
+{
+ rmutex->count = 0;
+ rmutex->holder = INVALID_PROCESS_ID;
+ return nxmutex_init(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nxrmutex_destroy
+ *
+ * Description:
+ * This function destroy the UNNAMED recursive mutex.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be destroyed
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_destroy(FAR rmutex_t *rmutex)
+{
+ return nxmutex_destroy(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nrxmutex_lock
+ *
+ * Description:
+ * This function attempts to lock the recursive mutex referenced by
+ * 'rmutex'.The recursive mutex can be locked multiple times in the same
+ * thread.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_lock(FAR rmutex_t *rmutex)
Review Comment:
How about we change inline to macro? Our internal test show that NuttX is much slow than other competitor(e.g. ThreadX). @anchao is busy to improve the performance in the recent two or three months, and found that the small but common wrapper function is one of performance block. That's why he created https://github.com/apache/incubator-nuttx/pull/6286.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #6279: include:add recursive lock
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #6279:
URL: https://github.com/apache/incubator-nuttx/pull/6279#issuecomment-1129139968
@anjiahao1 please rebase to the last mainline. CI error is fixed here: https://github.com/apache/incubator-nuttx/pull/6281
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6279: include:add recursive lock
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6279:
URL: https://github.com/apache/incubator-nuttx/pull/6279#discussion_r875835351
##########
include/nuttx/mutex.h:
##########
@@ -205,6 +214,200 @@ static inline int nxmutex_unlock(FAR mutex_t *mutex)
return nxsem_post(mutex);
}
+/****************************************************************************
+ * Name: nxrmutex_init
+ *
+ * Description:
+ * This function initializes the UNNAMED recursive mutex. Following a
+ * successful call to nxrmutex_init(), the recursive mutex may be used in
+ * subsequent calls to nxrmutex_lock(), nxrmutex_unlock(),
+ * and nxrmutex_trylock(). The recursive mutex remains usable
+ * until it is destroyed.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be initialized
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_init(FAR rmutex_t *rmutex)
+{
+ rmutex->count = 0;
+ rmutex->holder = INVALID_PROCESS_ID;
+ return nxmutex_init(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nxrmutex_destroy
+ *
+ * Description:
+ * This function destroy the UNNAMED recursive mutex.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be destroyed
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_destroy(FAR rmutex_t *rmutex)
+{
+ return nxmutex_destroy(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nrxmutex_lock
+ *
+ * Description:
+ * This function attempts to lock the recursive mutex referenced by
+ * 'rmutex'.The recursive mutex can be locked multiple times in the same
+ * thread.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_lock(FAR rmutex_t *rmutex)
+{
+ int ret = OK;
+ pid_t tid = gettid();
+
+ if (rmutex->holder == tid)
+ {
+ DEBUGASSERT(rmutex->count++ < UINT16_MAX);
+ }
+ else
+ {
+ ret = nxmutex_lock(&rmutex->mutex);
+ if (ret >= OK)
+ {
+ rmutex->holder = tid;
+ rmutex->count = 1;
+ }
+ }
+
+ return ret;
+}
+
+/****************************************************************************
+ * Name: nxrmutex_trylock
+ *
+ * Description:
+ * This function locks the recursive mutex if the recursive mutex is
+ * currently not locked or the same thread call.
+ * If the recursive mutex is locked and other thread call it,
+ * the call returns without blocking.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ * -EINVAL - Invalid attempt to lock the recursive mutex
+ * -EAGAIN - The recursive mutex is not available.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_trylock(FAR rmutex_t *rmutex)
+{
+ int ret = OK;
+ pid_t tid = gettid();
+
+ if (rmutex->holder == tid)
+ {
+ DEBUGASSERT(rmutex->count++ < UINT16_MAX);
+ }
+ else
+ {
+ ret = nxmutex_trylock(&rmutex->mutex);
+ if (ret >= OK)
+ {
+ rmutex->holder = tid;
+ rmutex->count = 1;
+ }
+ }
+
+ return ret;
+}
+
+/****************************************************************************
+ * Name: nxrmutex_is_locked
+ *
+ * Description:
+ * This function get the lock state the recursive mutex
+ * referenced by 'rmutex'.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ *
+ ****************************************************************************/
+
+static inline bool nxrmutex_is_locked(FAR rmutex_t *rmutex)
+{
+ return rmutex->count ? true : false;
+}
+
+/****************************************************************************
+ * Name: nxrmutex_unlock
+ *
+ * Description:
+ * This function attempts to unlock the recursive mutex
+ * referenced by 'rmutex'.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ * Assumptions:
+ * This function may be called from an interrupt handler.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_unlock(FAR rmutex_t *rmutex)
+{
+ int ret = OK;
+ pid_t tid = gettid();
+
+ if (rmutex->holder != tid)
+ {
+ return -EPERM;
+ }
+
+ DEBUGASSERT(rmutex->count > 0);
+ if (--rmutex->count == 0)
+ {
+ rmutex->holder = INVALID_PROCESS_ID;
+ ret = nxmutex_unlock(&rmutex->mutex);
Review Comment:
yes. you are right. I was writing comment late in the evening, so made some mistakes. My comment is more related to separation of `--rmutex->count` and `ret = nxmutex_unlock(&rmutex->mutex);`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] anjiahao1 commented on a diff in pull request #6279: include:add recursive lock
Posted by GitBox <gi...@apache.org>.
anjiahao1 commented on code in PR #6279:
URL: https://github.com/apache/incubator-nuttx/pull/6279#discussion_r875834682
##########
include/nuttx/mutex.h:
##########
@@ -205,6 +214,200 @@ static inline int nxmutex_unlock(FAR mutex_t *mutex)
return nxsem_post(mutex);
}
+/****************************************************************************
+ * Name: nxrmutex_init
+ *
+ * Description:
+ * This function initializes the UNNAMED recursive mutex. Following a
+ * successful call to nxrmutex_init(), the recursive mutex may be used in
+ * subsequent calls to nxrmutex_lock(), nxrmutex_unlock(),
+ * and nxrmutex_trylock(). The recursive mutex remains usable
+ * until it is destroyed.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be initialized
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_init(FAR rmutex_t *rmutex)
+{
+ rmutex->count = 0;
+ rmutex->holder = INVALID_PROCESS_ID;
+ return nxmutex_init(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nxrmutex_destroy
+ *
+ * Description:
+ * This function destroy the UNNAMED recursive mutex.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be destroyed
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_destroy(FAR rmutex_t *rmutex)
+{
+ return nxmutex_destroy(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nrxmutex_lock
+ *
+ * Description:
+ * This function attempts to lock the recursive mutex referenced by
+ * 'rmutex'.The recursive mutex can be locked multiple times in the same
+ * thread.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_lock(FAR rmutex_t *rmutex)
+{
+ int ret = OK;
+ pid_t tid = gettid();
+
+ if (rmutex->holder == tid)
+ {
+ DEBUGASSERT(rmutex->count++ < UINT16_MAX);
+ }
+ else
+ {
+ ret = nxmutex_lock(&rmutex->mutex);
+ if (ret >= OK)
+ {
+ rmutex->holder = tid;
+ rmutex->count = 1;
+ }
+ }
+
+ return ret;
+}
+
+/****************************************************************************
+ * Name: nxrmutex_trylock
+ *
+ * Description:
+ * This function locks the recursive mutex if the recursive mutex is
+ * currently not locked or the same thread call.
+ * If the recursive mutex is locked and other thread call it,
+ * the call returns without blocking.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ * -EINVAL - Invalid attempt to lock the recursive mutex
+ * -EAGAIN - The recursive mutex is not available.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_trylock(FAR rmutex_t *rmutex)
+{
+ int ret = OK;
+ pid_t tid = gettid();
+
+ if (rmutex->holder == tid)
+ {
+ DEBUGASSERT(rmutex->count++ < UINT16_MAX);
+ }
+ else
+ {
+ ret = nxmutex_trylock(&rmutex->mutex);
+ if (ret >= OK)
+ {
+ rmutex->holder = tid;
+ rmutex->count = 1;
+ }
+ }
+
+ return ret;
+}
+
+/****************************************************************************
+ * Name: nxrmutex_is_locked
+ *
+ * Description:
+ * This function get the lock state the recursive mutex
+ * referenced by 'rmutex'.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ *
+ ****************************************************************************/
+
+static inline bool nxrmutex_is_locked(FAR rmutex_t *rmutex)
+{
+ return rmutex->count ? true : false;
Review Comment:
return tpye is `bool` use `ture` and `false` better?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] anchao commented on a diff in pull request #6279: include:add recursive lock
Posted by GitBox <gi...@apache.org>.
anchao commented on code in PR #6279:
URL: https://github.com/apache/incubator-nuttx/pull/6279#discussion_r879932826
##########
include/nuttx/mutex.h:
##########
@@ -205,6 +214,200 @@ static inline int nxmutex_unlock(FAR mutex_t *mutex)
return nxsem_post(mutex);
}
+/****************************************************************************
+ * Name: nxrmutex_init
+ *
+ * Description:
+ * This function initializes the UNNAMED recursive mutex. Following a
+ * successful call to nxrmutex_init(), the recursive mutex may be used in
+ * subsequent calls to nxrmutex_lock(), nxrmutex_unlock(),
+ * and nxrmutex_trylock(). The recursive mutex remains usable
+ * until it is destroyed.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be initialized
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_init(FAR rmutex_t *rmutex)
+{
+ rmutex->count = 0;
+ rmutex->holder = INVALID_PROCESS_ID;
+ return nxmutex_init(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nxrmutex_destroy
+ *
+ * Description:
+ * This function destroy the UNNAMED recursive mutex.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be destroyed
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_destroy(FAR rmutex_t *rmutex)
+{
+ return nxmutex_destroy(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nrxmutex_lock
+ *
+ * Description:
+ * This function attempts to lock the recursive mutex referenced by
+ * 'rmutex'.The recursive mutex can be locked multiple times in the same
+ * thread.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_lock(FAR rmutex_t *rmutex)
Review Comment:
@xiaoxiang781216 @pkarashchenko
Some simple functions seem to get better performance if implemented in inline.
The following is an optimization of up_interrupt_context(). The inline version saves more instruction cycles:
Test on cortex-R:
Origin:
```
0000bb88 <up_interrupt_context>:
bb88: e59f300c ldr r3, [pc, #12] ; bb9c <up_interrupt_context+0x14>
bb8c: e5930000 ldr r0, [r3]
bb90: e2500000 subs r0, r0, #0
bb94: 13a00001 movne r0, #1
bb98: e12fff1e bx lr
bb9c: 0001a978 .word 0x0001a978
int file_mq_send(FAR struct file *mq, FAR const char *msg, size_t msglen,
unsigned int prio)
{
1c38: e92d47f3 push {r0, r1, r4, r5, r6, r7, r8, r9, sl, lr}
1c3c: e1a09003 mov r9, r3
...
1c80: eb000556 bl 31e0 <sched_lock>
1c84: e10fa000 mrs sl, CPSR
1c88: f10c0080 cpsid i
1c8c: eb0027bd bl bb88 <up_interrupt_context>
1c90: e3500000 cmp r0, #0
1c94: 0a000005 beq 1cb0 <file_mq_send+0x78>
...
```
inline:
```
int file_mq_send(FAR struct file *mq, FAR const char *msg, size_t msglen,
unsigned int prio)
{
1c38: e92d47f3 push {r0, r1, r4, r5, r6, r7, r8, r9, sl, lr}
1c3c: e1a09003 mov r9, r3
...
1c8c: e59f3078 ldr r3, [pc, #120] ; 1d0c <file_mq_send+0xd4>
1c90: e5933000 ldr r3, [r3]
1c94: e3530000 cmp r3, #0
1c98: 0a000005 beq 1cb4 <file_mq_send+0x7c>
```
you can see that the optimization version save about 6 instructions in an inline process,
in addition, some functions(file_mq_send) will call up_interrupt_context() repeatedly, eg:
```
file_mq_send
|
->up_interrupt_context
->nxmq_alloc_msg
->up_interrupt_context
```
In this way, we can save about 12 instructions in a single send,
it is worth noting that this optimization is more obvious in some scenarios:
mq_send perf count test:
mq send (low prio -> hi prio)
thread 1 (prio 100):
```
begin = up_perf_gettime();
ret = mq_send(mqdes, msg, 1, 1);
```
thread 2 (prio 101):
```
ret = mq_receive(mqdes, msg, sizeof(msg), NULL);
end = up_perf_gettime();
```
Before optimization:
`end - begin = 718`
Optimized:
`end - begin = 688`
optimized version needs 688 cycle counts, but this is far from other RTOS....
From the data below, you can be seen that the performance gap between nuttx and threadx is very large,
```
nuttx threadx (cycle)
mq_send(hi-low) 833 282
mq_send(low-hi) 688 172
mq_send(multi) 1978 467
sem_post(low-hi) 225 147
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6279: include:add recursive lock
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6279:
URL: https://github.com/apache/incubator-nuttx/pull/6279#discussion_r880290508
##########
include/nuttx/mutex.h:
##########
@@ -205,6 +217,210 @@ static inline int nxmutex_unlock(FAR mutex_t *mutex)
return nxsem_post(mutex);
}
+/****************************************************************************
+ * Name: nxrmutex_init
+ *
+ * Description:
+ * This function initializes the UNNAMED recursive mutex. Following a
+ * successful call to nxrmutex_init(), the recursive mutex may be used in
+ * subsequent calls to nxrmutex_lock(), nxrmutex_unlock(),
+ * and nxrmutex_trylock(). The recursive mutex remains usable
+ * until it is destroyed.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be initialized
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_init(FAR rmutex_t *rmutex)
+{
+ rmutex->count = 0;
+ rmutex->holder = INVALID_PROCESS_ID;
+ return nxmutex_init(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nxrmutex_destroy
+ *
+ * Description:
+ * This function destroy the UNNAMED recursive mutex.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be destroyed
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_destroy(FAR rmutex_t *rmutex)
+{
+ return nxmutex_destroy(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nrxmutex_lock
+ *
+ * Description:
+ * This function attempts to lock the recursive mutex referenced by
+ * 'rmutex'.The recursive mutex can be locked multiple times in the same
+ * thread.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_lock(FAR rmutex_t *rmutex)
+{
+ int ret = OK;
+ pid_t tid = gettid();
+
+ if (rmutex->holder == tid)
+ {
+ DEBUGASSERT(rmutex->count < UINT16_MAX);
+ rmutex->count++;
+ }
+ else
+ {
+ ret = nxmutex_lock(&rmutex->mutex);
+ if (ret >= OK)
+ {
+ rmutex->holder = tid;
+ rmutex->count = 1;
+ }
+ }
+
+ return ret;
+}
+
+/****************************************************************************
+ * Name: nxrmutex_trylock
+ *
+ * Description:
+ * This function locks the recursive mutex if the recursive mutex is
+ * currently not locked or the same thread call.
+ * If the recursive mutex is locked and other thread call it,
+ * the call returns without blocking.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ * -EINVAL - Invalid attempt to lock the recursive mutex
+ * -EAGAIN - The recursive mutex is not available.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_trylock(FAR rmutex_t *rmutex)
+{
+ int ret = OK;
+ pid_t tid = gettid();
+
+ if (rmutex->holder == tid)
+ {
+ DEBUGASSERT(rmutex->count < UINT16_MAX);
+ rmutex->count++;
+ }
+ else
+ {
+ ret = nxmutex_trylock(&rmutex->mutex);
+ if (ret >= OK)
+ {
+ rmutex->holder = tid;
+ rmutex->count = 1;
+ }
+ }
+
+ return ret;
+}
+
+/****************************************************************************
+ * Name: nxrmutex_is_locked
+ *
+ * Description:
+ * This function get the lock state the recursive mutex
+ * referenced by 'rmutex'.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ *
+ ****************************************************************************/
+
+static inline bool nxrmutex_is_locked(FAR rmutex_t *rmutex)
+{
+ return rmutex->count > 0;
+}
+
+/****************************************************************************
+ * Name: nxrmutex_unlock
+ *
+ * Description:
+ * This function attempts to unlock the recursive mutex
+ * referenced by 'rmutex'.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ * Assumptions:
+ * This function may be called from an interrupt handler.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_unlock(FAR rmutex_t *rmutex)
+{
+ int ret = OK;
+ pid_t tid = gettid();
+
+ if (rmutex->holder != tid)
+ {
+ return -EPERM;
+ }
+
+ DEBUGASSERT(rmutex->count > 0);
+ if (rmutex->count == 1)
+ {
+ ret = nxmutex_unlock(&rmutex->mutex);
+ if (ret >= 0)
Review Comment:
We need to add `sched_lock();`/`sched_unlock();` at least
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6279: include:add recursive lock
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6279:
URL: https://github.com/apache/incubator-nuttx/pull/6279#discussion_r880265838
##########
include/nuttx/mutex.h:
##########
@@ -205,6 +214,200 @@ static inline int nxmutex_unlock(FAR mutex_t *mutex)
return nxsem_post(mutex);
}
+/****************************************************************************
+ * Name: nxrmutex_init
+ *
+ * Description:
+ * This function initializes the UNNAMED recursive mutex. Following a
+ * successful call to nxrmutex_init(), the recursive mutex may be used in
+ * subsequent calls to nxrmutex_lock(), nxrmutex_unlock(),
+ * and nxrmutex_trylock(). The recursive mutex remains usable
+ * until it is destroyed.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be initialized
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_init(FAR rmutex_t *rmutex)
+{
+ rmutex->count = 0;
+ rmutex->holder = INVALID_PROCESS_ID;
+ return nxmutex_init(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nxrmutex_destroy
+ *
+ * Description:
+ * This function destroy the UNNAMED recursive mutex.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be destroyed
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_destroy(FAR rmutex_t *rmutex)
+{
+ return nxmutex_destroy(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nrxmutex_lock
+ *
+ * Description:
+ * This function attempts to lock the recursive mutex referenced by
+ * 'rmutex'.The recursive mutex can be locked multiple times in the same
+ * thread.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_lock(FAR rmutex_t *rmutex)
Review Comment:
I see. The mq is not the best for performance bench-marking but most probably can highlight the biggest gap. I mean that I know how queue send is implemented in ThreadX and comparing that to NuttX implementation the code gap (I mean even in the number of lines of the code) is huge. But from other hand the functionality is different. I mean that we can try to build a user space queue based on a list + semaphore and I think that would be better to compare with ThreadX queue. I think we should target to get performance of the base blocks like semaphore/mutex/eventfd as close as possible at the first step and then try to see what can be done for the mq.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6279: include:add recursive lock
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6279:
URL: https://github.com/apache/incubator-nuttx/pull/6279#discussion_r876809567
##########
include/nuttx/mutex.h:
##########
@@ -205,6 +214,200 @@ static inline int nxmutex_unlock(FAR mutex_t *mutex)
return nxsem_post(mutex);
}
+/****************************************************************************
+ * Name: nxrmutex_init
+ *
+ * Description:
+ * This function initializes the UNNAMED recursive mutex. Following a
+ * successful call to nxrmutex_init(), the recursive mutex may be used in
+ * subsequent calls to nxrmutex_lock(), nxrmutex_unlock(),
+ * and nxrmutex_trylock(). The recursive mutex remains usable
+ * until it is destroyed.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be initialized
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_init(FAR rmutex_t *rmutex)
+{
+ rmutex->count = 0;
+ rmutex->holder = INVALID_PROCESS_ID;
+ return nxmutex_init(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nxrmutex_destroy
+ *
+ * Description:
+ * This function destroy the UNNAMED recursive mutex.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be destroyed
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_destroy(FAR rmutex_t *rmutex)
+{
+ return nxmutex_destroy(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nrxmutex_lock
+ *
+ * Description:
+ * This function attempts to lock the recursive mutex referenced by
+ * 'rmutex'.The recursive mutex can be locked multiple times in the same
+ * thread.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_lock(FAR rmutex_t *rmutex)
Review Comment:
@anchao could you share the original and current result?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6279: include:add recursive lock
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6279:
URL: https://github.com/apache/incubator-nuttx/pull/6279#discussion_r875835351
##########
include/nuttx/mutex.h:
##########
@@ -205,6 +214,200 @@ static inline int nxmutex_unlock(FAR mutex_t *mutex)
return nxsem_post(mutex);
}
+/****************************************************************************
+ * Name: nxrmutex_init
+ *
+ * Description:
+ * This function initializes the UNNAMED recursive mutex. Following a
+ * successful call to nxrmutex_init(), the recursive mutex may be used in
+ * subsequent calls to nxrmutex_lock(), nxrmutex_unlock(),
+ * and nxrmutex_trylock(). The recursive mutex remains usable
+ * until it is destroyed.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be initialized
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_init(FAR rmutex_t *rmutex)
+{
+ rmutex->count = 0;
+ rmutex->holder = INVALID_PROCESS_ID;
+ return nxmutex_init(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nxrmutex_destroy
+ *
+ * Description:
+ * This function destroy the UNNAMED recursive mutex.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be destroyed
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_destroy(FAR rmutex_t *rmutex)
+{
+ return nxmutex_destroy(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nrxmutex_lock
+ *
+ * Description:
+ * This function attempts to lock the recursive mutex referenced by
+ * 'rmutex'.The recursive mutex can be locked multiple times in the same
+ * thread.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_lock(FAR rmutex_t *rmutex)
+{
+ int ret = OK;
+ pid_t tid = gettid();
+
+ if (rmutex->holder == tid)
+ {
+ DEBUGASSERT(rmutex->count++ < UINT16_MAX);
+ }
+ else
+ {
+ ret = nxmutex_lock(&rmutex->mutex);
+ if (ret >= OK)
+ {
+ rmutex->holder = tid;
+ rmutex->count = 1;
+ }
+ }
+
+ return ret;
+}
+
+/****************************************************************************
+ * Name: nxrmutex_trylock
+ *
+ * Description:
+ * This function locks the recursive mutex if the recursive mutex is
+ * currently not locked or the same thread call.
+ * If the recursive mutex is locked and other thread call it,
+ * the call returns without blocking.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ * -EINVAL - Invalid attempt to lock the recursive mutex
+ * -EAGAIN - The recursive mutex is not available.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_trylock(FAR rmutex_t *rmutex)
+{
+ int ret = OK;
+ pid_t tid = gettid();
+
+ if (rmutex->holder == tid)
+ {
+ DEBUGASSERT(rmutex->count++ < UINT16_MAX);
+ }
+ else
+ {
+ ret = nxmutex_trylock(&rmutex->mutex);
+ if (ret >= OK)
+ {
+ rmutex->holder = tid;
+ rmutex->count = 1;
+ }
+ }
+
+ return ret;
+}
+
+/****************************************************************************
+ * Name: nxrmutex_is_locked
+ *
+ * Description:
+ * This function get the lock state the recursive mutex
+ * referenced by 'rmutex'.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ *
+ ****************************************************************************/
+
+static inline bool nxrmutex_is_locked(FAR rmutex_t *rmutex)
+{
+ return rmutex->count ? true : false;
+}
+
+/****************************************************************************
+ * Name: nxrmutex_unlock
+ *
+ * Description:
+ * This function attempts to unlock the recursive mutex
+ * referenced by 'rmutex'.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ * Assumptions:
+ * This function may be called from an interrupt handler.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_unlock(FAR rmutex_t *rmutex)
+{
+ int ret = OK;
+ pid_t tid = gettid();
+
+ if (rmutex->holder != tid)
+ {
+ return -EPERM;
+ }
+
+ DEBUGASSERT(rmutex->count > 0);
+ if (--rmutex->count == 0)
+ {
+ rmutex->holder = INVALID_PROCESS_ID;
+ ret = nxmutex_unlock(&rmutex->mutex);
Review Comment:
yes. you are right. I was writing comment late in the evening, so made some mistakes. My comment is more related to separation of `--rmutex->count` and `ret = nxmutex_unlock(&rmutex->mutex);`, so if `rmutex->count == 1` and `nxmutex_unlock(&rmutex->mutex);` fails then `rmutex->count` will not be set to `0` as well as `rmutex->holder = INVALID_PROCESS_ID;`. This is more theoretical case, so anyway it is minor and should never happen
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] davids5 commented on a diff in pull request #6279: include:add recursive lock
Posted by GitBox <gi...@apache.org>.
davids5 commented on code in PR #6279:
URL: https://github.com/apache/incubator-nuttx/pull/6279#discussion_r878007864
##########
include/nuttx/mutex.h:
##########
@@ -205,6 +216,210 @@ static inline int nxmutex_unlock(FAR mutex_t *mutex)
return nxsem_post(mutex);
}
+/****************************************************************************
+ * Name: nxrmutex_init
+ *
+ * Description:
+ * This function initializes the UNNAMED recursive mutex. Following a
+ * successful call to nxrmutex_init(), the recursive mutex may be used in
+ * subsequent calls to nxrmutex_lock(), nxrmutex_unlock(),
+ * and nxrmutex_trylock(). The recursive mutex remains usable
+ * until it is destroyed.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be initialized
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_init(FAR rmutex_t *rmutex)
+{
+ rmutex->count = 0;
+ rmutex->holder = INVALID_PROCESS_ID;
+ return nxmutex_init(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nxrmutex_destroy
+ *
+ * Description:
+ * This function destroy the UNNAMED recursive mutex.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be destroyed
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_destroy(FAR rmutex_t *rmutex)
+{
+ return nxmutex_destroy(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nrxmutex_lock
+ *
+ * Description:
+ * This function attempts to lock the recursive mutex referenced by
+ * 'rmutex'.The recursive mutex can be locked multiple times in the same
+ * thread.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_lock(FAR rmutex_t *rmutex)
+{
+ int ret = OK;
+ pid_t tid = gettid();
+
+ if (rmutex->holder == tid)
+ {
+ rmutex->count;
Review Comment:
++ is now missing
##########
include/nuttx/mutex.h:
##########
@@ -205,6 +216,210 @@ static inline int nxmutex_unlock(FAR mutex_t *mutex)
return nxsem_post(mutex);
}
+/****************************************************************************
+ * Name: nxrmutex_init
+ *
+ * Description:
+ * This function initializes the UNNAMED recursive mutex. Following a
+ * successful call to nxrmutex_init(), the recursive mutex may be used in
+ * subsequent calls to nxrmutex_lock(), nxrmutex_unlock(),
+ * and nxrmutex_trylock(). The recursive mutex remains usable
+ * until it is destroyed.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be initialized
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_init(FAR rmutex_t *rmutex)
+{
+ rmutex->count = 0;
+ rmutex->holder = INVALID_PROCESS_ID;
+ return nxmutex_init(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nxrmutex_destroy
+ *
+ * Description:
+ * This function destroy the UNNAMED recursive mutex.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be destroyed
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_destroy(FAR rmutex_t *rmutex)
+{
+ return nxmutex_destroy(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nrxmutex_lock
+ *
+ * Description:
+ * This function attempts to lock the recursive mutex referenced by
+ * 'rmutex'.The recursive mutex can be locked multiple times in the same
+ * thread.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_lock(FAR rmutex_t *rmutex)
+{
+ int ret = OK;
+ pid_t tid = gettid();
+
+ if (rmutex->holder == tid)
+ {
+ rmutex->count;
+ DEBUGASSERT(rmutex->count < UINT16_MAX);
+ }
+ else
+ {
+ ret = nxmutex_lock(&rmutex->mutex);
+ if (ret >= OK)
+ {
+ rmutex->holder = tid;
+ rmutex->count = 1;
+ }
+ }
+
+ return ret;
+}
+
+/****************************************************************************
+ * Name: nxrmutex_trylock
+ *
+ * Description:
+ * This function locks the recursive mutex if the recursive mutex is
+ * currently not locked or the same thread call.
+ * If the recursive mutex is locked and other thread call it,
+ * the call returns without blocking.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ * -EINVAL - Invalid attempt to lock the recursive mutex
+ * -EAGAIN - The recursive mutex is not available.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_trylock(FAR rmutex_t *rmutex)
+{
+ int ret = OK;
+ pid_t tid = gettid();
+
+ if (rmutex->holder == tid)
+ {
+ rmutex->count;
Review Comment:
dito
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6279: include:add recursive lock
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6279:
URL: https://github.com/apache/incubator-nuttx/pull/6279#discussion_r880373020
##########
include/nuttx/mutex.h:
##########
@@ -205,6 +214,200 @@ static inline int nxmutex_unlock(FAR mutex_t *mutex)
return nxsem_post(mutex);
}
+/****************************************************************************
+ * Name: nxrmutex_init
+ *
+ * Description:
+ * This function initializes the UNNAMED recursive mutex. Following a
+ * successful call to nxrmutex_init(), the recursive mutex may be used in
+ * subsequent calls to nxrmutex_lock(), nxrmutex_unlock(),
+ * and nxrmutex_trylock(). The recursive mutex remains usable
+ * until it is destroyed.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be initialized
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_init(FAR rmutex_t *rmutex)
+{
+ rmutex->count = 0;
+ rmutex->holder = INVALID_PROCESS_ID;
+ return nxmutex_init(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nxrmutex_destroy
+ *
+ * Description:
+ * This function destroy the UNNAMED recursive mutex.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be destroyed
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_destroy(FAR rmutex_t *rmutex)
+{
+ return nxmutex_destroy(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nrxmutex_lock
+ *
+ * Description:
+ * This function attempts to lock the recursive mutex referenced by
+ * 'rmutex'.The recursive mutex can be locked multiple times in the same
+ * thread.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_lock(FAR rmutex_t *rmutex)
Review Comment:
> I see. The mq is not the best for performance bench-marking but most probably can highlight the biggest gap. I mean that I know how queue send is implemented in ThreadX and comparing that to NuttX implementation the code gap (I mean even in the number of lines of the code) is huge. But from other hand the functionality is different. I mean that we can try to build a user space queue based on a list + semaphore and I think that would be better to compare with ThreadX queue.
Yes, @anchao is preparing SystemV queue API which is more simple and can catch the gap a bit:
https://man7.org/linux/man-pages/man7/sysvipc.7.html
> I think we should target to get performance of the base blocks like semaphore/mutex/eventfd as close as possible at the first step and then try to see what can be done for the mq.
Yes, here is the sem result:
```
before optimization 225
after optimization 184
threadx 147
```
You can see we still have 37 cycle gap.
##########
include/nuttx/mutex.h:
##########
@@ -205,6 +217,210 @@ static inline int nxmutex_unlock(FAR mutex_t *mutex)
return nxsem_post(mutex);
}
+/****************************************************************************
+ * Name: nxrmutex_init
+ *
+ * Description:
+ * This function initializes the UNNAMED recursive mutex. Following a
+ * successful call to nxrmutex_init(), the recursive mutex may be used in
+ * subsequent calls to nxrmutex_lock(), nxrmutex_unlock(),
+ * and nxrmutex_trylock(). The recursive mutex remains usable
+ * until it is destroyed.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be initialized
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_init(FAR rmutex_t *rmutex)
+{
+ rmutex->count = 0;
+ rmutex->holder = INVALID_PROCESS_ID;
+ return nxmutex_init(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nxrmutex_destroy
+ *
+ * Description:
+ * This function destroy the UNNAMED recursive mutex.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be destroyed
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_destroy(FAR rmutex_t *rmutex)
+{
+ return nxmutex_destroy(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nrxmutex_lock
+ *
+ * Description:
+ * This function attempts to lock the recursive mutex referenced by
+ * 'rmutex'.The recursive mutex can be locked multiple times in the same
+ * thread.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_lock(FAR rmutex_t *rmutex)
+{
+ int ret = OK;
+ pid_t tid = gettid();
+
+ if (rmutex->holder == tid)
+ {
+ DEBUGASSERT(rmutex->count < UINT16_MAX);
+ rmutex->count++;
+ }
+ else
+ {
+ ret = nxmutex_lock(&rmutex->mutex);
+ if (ret >= OK)
+ {
+ rmutex->holder = tid;
+ rmutex->count = 1;
+ }
+ }
+
+ return ret;
+}
+
+/****************************************************************************
+ * Name: nxrmutex_trylock
+ *
+ * Description:
+ * This function locks the recursive mutex if the recursive mutex is
+ * currently not locked or the same thread call.
+ * If the recursive mutex is locked and other thread call it,
+ * the call returns without blocking.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ * -EINVAL - Invalid attempt to lock the recursive mutex
+ * -EAGAIN - The recursive mutex is not available.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_trylock(FAR rmutex_t *rmutex)
+{
+ int ret = OK;
+ pid_t tid = gettid();
+
+ if (rmutex->holder == tid)
+ {
+ DEBUGASSERT(rmutex->count < UINT16_MAX);
+ rmutex->count++;
+ }
+ else
+ {
+ ret = nxmutex_trylock(&rmutex->mutex);
+ if (ret >= OK)
+ {
+ rmutex->holder = tid;
+ rmutex->count = 1;
+ }
+ }
+
+ return ret;
+}
+
+/****************************************************************************
+ * Name: nxrmutex_is_locked
+ *
+ * Description:
+ * This function get the lock state the recursive mutex
+ * referenced by 'rmutex'.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ *
+ ****************************************************************************/
+
+static inline bool nxrmutex_is_locked(FAR rmutex_t *rmutex)
+{
+ return rmutex->count > 0;
+}
+
+/****************************************************************************
+ * Name: nxrmutex_unlock
+ *
+ * Description:
+ * This function attempts to unlock the recursive mutex
+ * referenced by 'rmutex'.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ * Assumptions:
+ * This function may be called from an interrupt handler.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_unlock(FAR rmutex_t *rmutex)
+{
+ int ret = OK;
+ pid_t tid = gettid();
+
+ if (rmutex->holder != tid)
+ {
+ return -EPERM;
+ }
+
+ DEBUGASSERT(rmutex->count > 0);
+ if (rmutex->count == 1)
+ {
+ ret = nxmutex_unlock(&rmutex->mutex);
+ if (ret >= 0)
Review Comment:
Yes, the order should reverse
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #6279: include:add recursive lock
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged PR #6279:
URL: https://github.com/apache/incubator-nuttx/pull/6279
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6279: include:add recursive lock
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6279:
URL: https://github.com/apache/incubator-nuttx/pull/6279#discussion_r879651029
##########
include/nuttx/mutex.h:
##########
@@ -205,6 +214,200 @@ static inline int nxmutex_unlock(FAR mutex_t *mutex)
return nxsem_post(mutex);
}
+/****************************************************************************
+ * Name: nxrmutex_init
+ *
+ * Description:
+ * This function initializes the UNNAMED recursive mutex. Following a
+ * successful call to nxrmutex_init(), the recursive mutex may be used in
+ * subsequent calls to nxrmutex_lock(), nxrmutex_unlock(),
+ * and nxrmutex_trylock(). The recursive mutex remains usable
+ * until it is destroyed.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be initialized
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_init(FAR rmutex_t *rmutex)
+{
+ rmutex->count = 0;
+ rmutex->holder = INVALID_PROCESS_ID;
+ return nxmutex_init(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nxrmutex_destroy
+ *
+ * Description:
+ * This function destroy the UNNAMED recursive mutex.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be destroyed
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_destroy(FAR rmutex_t *rmutex)
+{
+ return nxmutex_destroy(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nrxmutex_lock
+ *
+ * Description:
+ * This function attempts to lock the recursive mutex referenced by
+ * 'rmutex'.The recursive mutex can be locked multiple times in the same
+ * thread.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_lock(FAR rmutex_t *rmutex)
Review Comment:
I don't know, but I think we should remove no_instrument_function from inline_function
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] anjiahao1 commented on a diff in pull request #6279: include:add recursive lock
Posted by GitBox <gi...@apache.org>.
anjiahao1 commented on code in PR #6279:
URL: https://github.com/apache/incubator-nuttx/pull/6279#discussion_r875834682
##########
include/nuttx/mutex.h:
##########
@@ -205,6 +214,200 @@ static inline int nxmutex_unlock(FAR mutex_t *mutex)
return nxsem_post(mutex);
}
+/****************************************************************************
+ * Name: nxrmutex_init
+ *
+ * Description:
+ * This function initializes the UNNAMED recursive mutex. Following a
+ * successful call to nxrmutex_init(), the recursive mutex may be used in
+ * subsequent calls to nxrmutex_lock(), nxrmutex_unlock(),
+ * and nxrmutex_trylock(). The recursive mutex remains usable
+ * until it is destroyed.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be initialized
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_init(FAR rmutex_t *rmutex)
+{
+ rmutex->count = 0;
+ rmutex->holder = INVALID_PROCESS_ID;
+ return nxmutex_init(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nxrmutex_destroy
+ *
+ * Description:
+ * This function destroy the UNNAMED recursive mutex.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be destroyed
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_destroy(FAR rmutex_t *rmutex)
+{
+ return nxmutex_destroy(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nrxmutex_lock
+ *
+ * Description:
+ * This function attempts to lock the recursive mutex referenced by
+ * 'rmutex'.The recursive mutex can be locked multiple times in the same
+ * thread.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_lock(FAR rmutex_t *rmutex)
+{
+ int ret = OK;
+ pid_t tid = gettid();
+
+ if (rmutex->holder == tid)
+ {
+ DEBUGASSERT(rmutex->count++ < UINT16_MAX);
+ }
+ else
+ {
+ ret = nxmutex_lock(&rmutex->mutex);
+ if (ret >= OK)
+ {
+ rmutex->holder = tid;
+ rmutex->count = 1;
+ }
+ }
+
+ return ret;
+}
+
+/****************************************************************************
+ * Name: nxrmutex_trylock
+ *
+ * Description:
+ * This function locks the recursive mutex if the recursive mutex is
+ * currently not locked or the same thread call.
+ * If the recursive mutex is locked and other thread call it,
+ * the call returns without blocking.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ * -EINVAL - Invalid attempt to lock the recursive mutex
+ * -EAGAIN - The recursive mutex is not available.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_trylock(FAR rmutex_t *rmutex)
+{
+ int ret = OK;
+ pid_t tid = gettid();
+
+ if (rmutex->holder == tid)
+ {
+ DEBUGASSERT(rmutex->count++ < UINT16_MAX);
+ }
+ else
+ {
+ ret = nxmutex_trylock(&rmutex->mutex);
+ if (ret >= OK)
+ {
+ rmutex->holder = tid;
+ rmutex->count = 1;
+ }
+ }
+
+ return ret;
+}
+
+/****************************************************************************
+ * Name: nxrmutex_is_locked
+ *
+ * Description:
+ * This function get the lock state the recursive mutex
+ * referenced by 'rmutex'.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ *
+ ****************************************************************************/
+
+static inline bool nxrmutex_is_locked(FAR rmutex_t *rmutex)
+{
+ return rmutex->count ? true : false;
Review Comment:
return tpye is `bool` use `ture` and `false` batter?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6279: include:add recursive lock
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6279:
URL: https://github.com/apache/incubator-nuttx/pull/6279#discussion_r878274715
##########
include/nuttx/mutex.h:
##########
@@ -205,6 +217,210 @@ static inline int nxmutex_unlock(FAR mutex_t *mutex)
return nxsem_post(mutex);
}
+/****************************************************************************
+ * Name: nxrmutex_init
+ *
+ * Description:
+ * This function initializes the UNNAMED recursive mutex. Following a
+ * successful call to nxrmutex_init(), the recursive mutex may be used in
+ * subsequent calls to nxrmutex_lock(), nxrmutex_unlock(),
+ * and nxrmutex_trylock(). The recursive mutex remains usable
+ * until it is destroyed.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be initialized
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_init(FAR rmutex_t *rmutex)
+{
+ rmutex->count = 0;
+ rmutex->holder = INVALID_PROCESS_ID;
+ return nxmutex_init(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nxrmutex_destroy
+ *
+ * Description:
+ * This function destroy the UNNAMED recursive mutex.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be destroyed
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_destroy(FAR rmutex_t *rmutex)
+{
+ return nxmutex_destroy(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nrxmutex_lock
+ *
+ * Description:
+ * This function attempts to lock the recursive mutex referenced by
+ * 'rmutex'.The recursive mutex can be locked multiple times in the same
+ * thread.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_lock(FAR rmutex_t *rmutex)
+{
+ int ret = OK;
+ pid_t tid = gettid();
+
+ if (rmutex->holder == tid)
+ {
+ rmutex->count++;
+ DEBUGASSERT(rmutex->count < UINT16_MAX);
Review Comment:
move before line 294
##########
include/nuttx/mutex.h:
##########
@@ -205,6 +217,210 @@ static inline int nxmutex_unlock(FAR mutex_t *mutex)
return nxsem_post(mutex);
}
+/****************************************************************************
+ * Name: nxrmutex_init
+ *
+ * Description:
+ * This function initializes the UNNAMED recursive mutex. Following a
+ * successful call to nxrmutex_init(), the recursive mutex may be used in
+ * subsequent calls to nxrmutex_lock(), nxrmutex_unlock(),
+ * and nxrmutex_trylock(). The recursive mutex remains usable
+ * until it is destroyed.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be initialized
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_init(FAR rmutex_t *rmutex)
+{
+ rmutex->count = 0;
+ rmutex->holder = INVALID_PROCESS_ID;
+ return nxmutex_init(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nxrmutex_destroy
+ *
+ * Description:
+ * This function destroy the UNNAMED recursive mutex.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be destroyed
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_destroy(FAR rmutex_t *rmutex)
+{
+ return nxmutex_destroy(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nrxmutex_lock
+ *
+ * Description:
+ * This function attempts to lock the recursive mutex referenced by
+ * 'rmutex'.The recursive mutex can be locked multiple times in the same
+ * thread.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_lock(FAR rmutex_t *rmutex)
+{
+ int ret = OK;
+ pid_t tid = gettid();
+
+ if (rmutex->holder == tid)
+ {
+ rmutex->count++;
+ DEBUGASSERT(rmutex->count < UINT16_MAX);
+ }
+ else
+ {
+ ret = nxmutex_lock(&rmutex->mutex);
+ if (ret >= OK)
+ {
+ rmutex->holder = tid;
+ rmutex->count = 1;
+ }
+ }
+
+ return ret;
+}
+
+/****************************************************************************
+ * Name: nxrmutex_trylock
+ *
+ * Description:
+ * This function locks the recursive mutex if the recursive mutex is
+ * currently not locked or the same thread call.
+ * If the recursive mutex is locked and other thread call it,
+ * the call returns without blocking.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ * -EINVAL - Invalid attempt to lock the recursive mutex
+ * -EAGAIN - The recursive mutex is not available.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_trylock(FAR rmutex_t *rmutex)
+{
+ int ret = OK;
+ pid_t tid = gettid();
+
+ if (rmutex->holder == tid)
+ {
+ rmutex->count++;
+ DEBUGASSERT(rmutex->count < UINT16_MAX);
Review Comment:
move before line 340
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6279: include:add recursive lock
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6279:
URL: https://github.com/apache/incubator-nuttx/pull/6279#discussion_r876781286
##########
include/nuttx/mutex.h:
##########
@@ -205,6 +214,200 @@ static inline int nxmutex_unlock(FAR mutex_t *mutex)
return nxsem_post(mutex);
}
+/****************************************************************************
+ * Name: nxrmutex_init
+ *
+ * Description:
+ * This function initializes the UNNAMED recursive mutex. Following a
+ * successful call to nxrmutex_init(), the recursive mutex may be used in
+ * subsequent calls to nxrmutex_lock(), nxrmutex_unlock(),
+ * and nxrmutex_trylock(). The recursive mutex remains usable
+ * until it is destroyed.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be initialized
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_init(FAR rmutex_t *rmutex)
+{
+ rmutex->count = 0;
+ rmutex->holder = INVALID_PROCESS_ID;
+ return nxmutex_init(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nxrmutex_destroy
+ *
+ * Description:
+ * This function destroy the UNNAMED recursive mutex.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be destroyed
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_destroy(FAR rmutex_t *rmutex)
+{
+ return nxmutex_destroy(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nrxmutex_lock
+ *
+ * Description:
+ * This function attempts to lock the recursive mutex referenced by
+ * 'rmutex'.The recursive mutex can be locked multiple times in the same
+ * thread.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_lock(FAR rmutex_t *rmutex)
Review Comment:
@xiaoxiang781216 BTW do you have any charts with comparison vs other RTOSes that can be shared? I just want to understand the gap
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] davids5 commented on a diff in pull request #6279: include:add recursive lock
Posted by GitBox <gi...@apache.org>.
davids5 commented on code in PR #6279:
URL: https://github.com/apache/incubator-nuttx/pull/6279#discussion_r877036238
##########
include/nuttx/mutex.h:
##########
@@ -205,6 +216,208 @@ static inline int nxmutex_unlock(FAR mutex_t *mutex)
return nxsem_post(mutex);
}
+/****************************************************************************
+ * Name: nxrmutex_init
+ *
+ * Description:
+ * This function initializes the UNNAMED recursive mutex. Following a
+ * successful call to nxrmutex_init(), the recursive mutex may be used in
+ * subsequent calls to nxrmutex_lock(), nxrmutex_unlock(),
+ * and nxrmutex_trylock(). The recursive mutex remains usable
+ * until it is destroyed.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be initialized
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_init(FAR rmutex_t *rmutex)
+{
+ rmutex->count = 0;
+ rmutex->holder = INVALID_PROCESS_ID;
+ return nxmutex_init(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nxrmutex_destroy
+ *
+ * Description:
+ * This function destroy the UNNAMED recursive mutex.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be destroyed
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_destroy(FAR rmutex_t *rmutex)
+{
+ return nxmutex_destroy(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nrxmutex_lock
+ *
+ * Description:
+ * This function attempts to lock the recursive mutex referenced by
+ * 'rmutex'.The recursive mutex can be locked multiple times in the same
+ * thread.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_lock(FAR rmutex_t *rmutex)
+{
+ int ret = OK;
+ pid_t tid = gettid();
+
+ if (rmutex->holder == tid)
+ {
+ DEBUGASSERT(rmutex->count++ < UINT16_MAX);
+ }
+ else
+ {
+ ret = nxmutex_lock(&rmutex->mutex);
+ if (ret >= OK)
+ {
+ rmutex->holder = tid;
+ rmutex->count = 1;
+ }
+ }
+
+ return ret;
+}
+
+/****************************************************************************
+ * Name: nxrmutex_trylock
+ *
+ * Description:
+ * This function locks the recursive mutex if the recursive mutex is
+ * currently not locked or the same thread call.
+ * If the recursive mutex is locked and other thread call it,
+ * the call returns without blocking.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ * -EINVAL - Invalid attempt to lock the recursive mutex
+ * -EAGAIN - The recursive mutex is not available.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_trylock(FAR rmutex_t *rmutex)
+{
+ int ret = OK;
+ pid_t tid = gettid();
+
+ if (rmutex->holder == tid)
+ {
+ DEBUGASSERT(rmutex->count++ < UINT16_MAX);
Review Comment:
ditto
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6279: include:add recursive lock
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6279:
URL: https://github.com/apache/incubator-nuttx/pull/6279#discussion_r880268809
##########
include/nuttx/mutex.h:
##########
@@ -205,6 +217,210 @@ static inline int nxmutex_unlock(FAR mutex_t *mutex)
return nxsem_post(mutex);
}
+/****************************************************************************
+ * Name: nxrmutex_init
+ *
+ * Description:
+ * This function initializes the UNNAMED recursive mutex. Following a
+ * successful call to nxrmutex_init(), the recursive mutex may be used in
+ * subsequent calls to nxrmutex_lock(), nxrmutex_unlock(),
+ * and nxrmutex_trylock(). The recursive mutex remains usable
+ * until it is destroyed.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be initialized
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_init(FAR rmutex_t *rmutex)
+{
+ rmutex->count = 0;
+ rmutex->holder = INVALID_PROCESS_ID;
+ return nxmutex_init(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nxrmutex_destroy
+ *
+ * Description:
+ * This function destroy the UNNAMED recursive mutex.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be destroyed
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_destroy(FAR rmutex_t *rmutex)
+{
+ return nxmutex_destroy(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nrxmutex_lock
+ *
+ * Description:
+ * This function attempts to lock the recursive mutex referenced by
+ * 'rmutex'.The recursive mutex can be locked multiple times in the same
+ * thread.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_lock(FAR rmutex_t *rmutex)
+{
+ int ret = OK;
+ pid_t tid = gettid();
+
+ if (rmutex->holder == tid)
+ {
+ DEBUGASSERT(rmutex->count < UINT16_MAX);
+ rmutex->count++;
+ }
+ else
+ {
+ ret = nxmutex_lock(&rmutex->mutex);
+ if (ret >= OK)
+ {
+ rmutex->holder = tid;
+ rmutex->count = 1;
+ }
+ }
+
+ return ret;
+}
+
+/****************************************************************************
+ * Name: nxrmutex_trylock
+ *
+ * Description:
+ * This function locks the recursive mutex if the recursive mutex is
+ * currently not locked or the same thread call.
+ * If the recursive mutex is locked and other thread call it,
+ * the call returns without blocking.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ * -EINVAL - Invalid attempt to lock the recursive mutex
+ * -EAGAIN - The recursive mutex is not available.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_trylock(FAR rmutex_t *rmutex)
+{
+ int ret = OK;
+ pid_t tid = gettid();
+
+ if (rmutex->holder == tid)
+ {
+ DEBUGASSERT(rmutex->count < UINT16_MAX);
+ rmutex->count++;
+ }
+ else
+ {
+ ret = nxmutex_trylock(&rmutex->mutex);
+ if (ret >= OK)
Review Comment:
ditto
##########
include/nuttx/mutex.h:
##########
@@ -205,6 +217,210 @@ static inline int nxmutex_unlock(FAR mutex_t *mutex)
return nxsem_post(mutex);
}
+/****************************************************************************
+ * Name: nxrmutex_init
+ *
+ * Description:
+ * This function initializes the UNNAMED recursive mutex. Following a
+ * successful call to nxrmutex_init(), the recursive mutex may be used in
+ * subsequent calls to nxrmutex_lock(), nxrmutex_unlock(),
+ * and nxrmutex_trylock(). The recursive mutex remains usable
+ * until it is destroyed.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be initialized
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_init(FAR rmutex_t *rmutex)
+{
+ rmutex->count = 0;
+ rmutex->holder = INVALID_PROCESS_ID;
+ return nxmutex_init(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nxrmutex_destroy
+ *
+ * Description:
+ * This function destroy the UNNAMED recursive mutex.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be destroyed
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_destroy(FAR rmutex_t *rmutex)
+{
+ return nxmutex_destroy(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nrxmutex_lock
+ *
+ * Description:
+ * This function attempts to lock the recursive mutex referenced by
+ * 'rmutex'.The recursive mutex can be locked multiple times in the same
+ * thread.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_lock(FAR rmutex_t *rmutex)
+{
+ int ret = OK;
+ pid_t tid = gettid();
+
+ if (rmutex->holder == tid)
+ {
+ DEBUGASSERT(rmutex->count < UINT16_MAX);
+ rmutex->count++;
+ }
+ else
+ {
+ ret = nxmutex_lock(&rmutex->mutex);
+ if (ret >= OK)
+ {
+ rmutex->holder = tid;
+ rmutex->count = 1;
+ }
+ }
+
+ return ret;
+}
+
+/****************************************************************************
+ * Name: nxrmutex_trylock
+ *
+ * Description:
+ * This function locks the recursive mutex if the recursive mutex is
+ * currently not locked or the same thread call.
+ * If the recursive mutex is locked and other thread call it,
+ * the call returns without blocking.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ * -EINVAL - Invalid attempt to lock the recursive mutex
+ * -EAGAIN - The recursive mutex is not available.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_trylock(FAR rmutex_t *rmutex)
+{
+ int ret = OK;
+ pid_t tid = gettid();
+
+ if (rmutex->holder == tid)
+ {
+ DEBUGASSERT(rmutex->count < UINT16_MAX);
+ rmutex->count++;
+ }
+ else
+ {
+ ret = nxmutex_trylock(&rmutex->mutex);
+ if (ret >= OK)
+ {
+ rmutex->holder = tid;
+ rmutex->count = 1;
+ }
+ }
+
+ return ret;
+}
+
+/****************************************************************************
+ * Name: nxrmutex_is_locked
+ *
+ * Description:
+ * This function get the lock state the recursive mutex
+ * referenced by 'rmutex'.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ *
+ ****************************************************************************/
+
+static inline bool nxrmutex_is_locked(FAR rmutex_t *rmutex)
+{
+ return rmutex->count > 0;
+}
+
+/****************************************************************************
+ * Name: nxrmutex_unlock
+ *
+ * Description:
+ * This function attempts to unlock the recursive mutex
+ * referenced by 'rmutex'.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ * Assumptions:
+ * This function may be called from an interrupt handler.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_unlock(FAR rmutex_t *rmutex)
+{
+ int ret = OK;
+ pid_t tid = gettid();
+
+ if (rmutex->holder != tid)
+ {
+ return -EPERM;
+ }
+
+ DEBUGASSERT(rmutex->count > 0);
+ if (rmutex->count == 1)
+ {
+ ret = nxmutex_unlock(&rmutex->mutex);
+ if (ret >= 0)
Review Comment:
The question about MT safety. If we `nxmutex_unlock(&rmutex->mutex);` successfully and then the task is preempted and another task gets running from line 299 of `nxrmutex_lock()`. The new task (mutex holder) sets `rmutex->holder = tid;` + `rmutex->count = 1;` and sleeps for a tick. So the task that is executing `nxrmutex_unlock()` will resume execution at this line and will `rmutex->count = 0;` + `rmutex->holder = INVALID_PROCESS_ID;` (overwrite info about current holder). Seems to be unsafe to do it without a critical section.
##########
include/nuttx/mutex.h:
##########
@@ -205,6 +217,210 @@ static inline int nxmutex_unlock(FAR mutex_t *mutex)
return nxsem_post(mutex);
}
+/****************************************************************************
+ * Name: nxrmutex_init
+ *
+ * Description:
+ * This function initializes the UNNAMED recursive mutex. Following a
+ * successful call to nxrmutex_init(), the recursive mutex may be used in
+ * subsequent calls to nxrmutex_lock(), nxrmutex_unlock(),
+ * and nxrmutex_trylock(). The recursive mutex remains usable
+ * until it is destroyed.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be initialized
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_init(FAR rmutex_t *rmutex)
+{
+ rmutex->count = 0;
+ rmutex->holder = INVALID_PROCESS_ID;
+ return nxmutex_init(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nxrmutex_destroy
+ *
+ * Description:
+ * This function destroy the UNNAMED recursive mutex.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex to be destroyed
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_destroy(FAR rmutex_t *rmutex)
+{
+ return nxmutex_destroy(&rmutex->mutex);
+}
+
+/****************************************************************************
+ * Name: nrxmutex_lock
+ *
+ * Description:
+ * This function attempts to lock the recursive mutex referenced by
+ * 'rmutex'.The recursive mutex can be locked multiple times in the same
+ * thread.
+ *
+ * Parameters:
+ * rmutex - Recursive mutex descriptor.
+ *
+ * Return Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
+ *
+ ****************************************************************************/
+
+static inline int nxrmutex_lock(FAR rmutex_t *rmutex)
+{
+ int ret = OK;
+ pid_t tid = gettid();
+
+ if (rmutex->holder == tid)
+ {
+ DEBUGASSERT(rmutex->count < UINT16_MAX);
+ rmutex->count++;
+ }
+ else
+ {
+ ret = nxmutex_lock(&rmutex->mutex);
+ if (ret >= OK)
Review Comment:
I do not see a case when it can be `> OK`
```suggestion
if (ret == OK)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org