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-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org