xiaoxiang781216 commented on code in PR #16194:
URL: https://github.com/apache/nuttx/pull/16194#discussion_r2073187368
##########
libs/libc/semaphore/sem_getvalue.c:
##########
@@ -65,7 +69,23 @@ int nxsem_get_value(FAR sem_t *sem, FAR int *sval)
{
if (sem != NULL && sval != NULL)
{
- *sval = atomic_read(NXSEM_COUNT(sem));
+ if (NXSEM_IS_MUTEX(sem))
+ {
+ uint32_t mholder = atomic_read(NXSEM_MHOLDER(sem));
+ if (NXSEM_MACQUIRED(mholder))
+ {
+ *sval = NXSEM_MBLOCKS(mholder) ? -1 : 0;
Review Comment:
NXSEM_MBLOCKS->NXSEM_MBLOCKING
##########
sched/semaphore/sem_post.c:
##########
@@ -156,7 +195,17 @@ int nxsem_post_slow(FAR sem_t *sem)
* it is awakened.
*/
- nxsem_add_holder_tcb(stcb, sem);
+ if (mutex)
+ {
+ uint32_t blocking = dq_empty(SEM_WAITLIST(sem)) ?
Review Comment:
```
uint32_t blocking_bit = dq_empty(SEM_WAITLIST(sem)) ?
0 : NXSEM_MBLOCKING_BIT;
```
##########
sched/semaphore/sem_recover.c:
##########
@@ -104,7 +105,23 @@ void nxsem_recover(FAR struct tcb_s *tcb)
* place.
*/
- atomic_fetch_add(NXSEM_COUNT(sem), 1);
+ if (NXSEM_IS_MUTEX(sem))
+ {
+ /* Clear the blocking bit, if not blocked any more */
+
+ if (dq_empty(SEM_WAITLIST(sem)))
+ {
+ uint32_t mholder =
+ atomic_fetch_and(NXSEM_MHOLDER(sem), ~NXSEM_MBLOCKING_BIT);
+ DEBUGASSERT(NXSEM_MBLOCKING(mholder));
+ }
+ }
+ else
+ {
+ DEBUGASSERT(atomic_read(NXSEM_COUNT(sem)) < 0);
+
Review Comment:
remove the blank line
##########
sched/semaphore/sem_waitirq.c:
##########
@@ -72,31 +72,55 @@ void nxsem_wait_irq(FAR struct tcb_s *wtcb, int errcode)
{
FAR struct tcb_s *rtcb = this_task();
FAR sem_t *sem = wtcb->waitobj;
+ bool mutex = NXSEM_IS_MUTEX(sem);
/* It is possible that an interrupt/context switch beat us to the punch
* and already changed the task's state.
*/
- DEBUGASSERT(sem != NULL && atomic_read(NXSEM_COUNT(sem)) < 0);
+ DEBUGASSERT(sem != NULL);
+ DEBUGASSERT(
Review Comment:
```
DEBUGASSERT(NXSEM_IS_MUTEX(sem) || atomic_read(NXSEM_COUNT(sem)) <= 0);
DEBUGASSERT(!NXSEM_IS_MUTEX(sem) ||
NXSEM_MBLOCKING(atomic_read(NXSEM_MHOLDER(sem))));
```
##########
libs/libc/semaphore/sem_wait.c:
##########
@@ -151,9 +151,9 @@ int nxsem_wait(FAR sem_t *sem)
#ifndef CONFIG_LIBC_ARCH_ATOMIC
- if ((sem->flags & SEM_TYPE_MUTEX)
-# if defined(CONFIG_PRIORITY_PROTECT) || defined(CONFIG_PRIORITY_INHERITANCE)
- && (sem->flags & SEM_PRIO_MASK) == SEM_PRIO_NONE
+ if (NXSEM_IS_MUTEX(sem)
+# if defined(CONFIG_PRIORITY_PROTECT)
Review Comment:
ditto
##########
include/nuttx/mutex.h:
##########
@@ -354,407 +384,497 @@ void nxmutex_reset(FAR mutex_t *mutex);
*
****************************************************************************/
-int nxmutex_breaklock(FAR mutex_t *mutex, FAR unsigned int *locked);
+int nxrmutex_breaklock(FAR rmutex_t *rmutex, FAR unsigned int *count);
/****************************************************************************
- * Name: nxmutex_restorelock
+ * Name: nxrmutex_restorelock
*
* Description:
- * This function attempts to restore the mutex.
+ * This function attempts to restore the recursive mutex.
*
* Parameters:
- * mutex - mutex descriptor.
- * locked - true: it's mean that the mutex is broke success
+ * 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
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
*
****************************************************************************/
-int nxmutex_restorelock(FAR mutex_t *mutex, unsigned int locked);
+int nxrmutex_restorelock(FAR rmutex_t *rmutex, unsigned int count);
+
+#define nxrmutex_set_protocol(rmutex, protocol) \
+ nxmutex_set_protocol(&(rmutex)->mutex, protocol)
+#define nxrmutex_getprioceiling(rmutex, prioceiling) \
+ nxmutex_getprioceiling(&(rmutex)->mutex, prioceiling)
+#define nxrmutex_setprioceiling(rmutex, prioceiling, old_ceiling) \
+ nxmutex_setprioceiling(&(rmutex)->mutex, prioceiling, old_ceiling)
+
+/****************************************************************************
+ * Inline functions
+ ****************************************************************************/
/****************************************************************************
- * Name: nxmutex_set_protocol
+ * Name: nxmutex_get_holder
*
* Description:
- * This function attempts to set the priority protocol of a mutex.
+ * This function get the holder of the mutex referenced by 'mutex'.
+ * Note that this is inherently racy unless the calling thread is
+ * holding the mutex.
*
* Parameters:
- * mutex - mutex descriptor.
- * protocol - mutex protocol value to set.
+ * mutex - 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
*
****************************************************************************/
-int nxmutex_set_protocol(FAR mutex_t *mutex, int protocol);
+static inline pid_t nxmutex_get_holder(FAR mutex_t *mutex)
+{
+ uint32_t mholder = mutex->sem.val.mholder & ~NXSEM_MBLOCKING_BIT;
+ return NXSEM_MACQUIRED(mholder) ? (pid_t)mholder : -1;
+}
/****************************************************************************
- * Name: nxmutex_getprioceiling
+ * Name: nxmutex_is_locked
*
* Description:
- * This function attempts to get the priority ceiling of a mutex.
+ * This function get the lock state the mutex referenced by 'mutex'.
+ * Note that this is inherently racy unless the calling thread is
+ * holding the mutex.
*
* Parameters:
- * mutex - mutex descriptor.
- * prioceiling - location to return the mutex priority ceiling.
+ * mutex - 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
*
****************************************************************************/
-int nxmutex_getprioceiling(FAR const mutex_t *mutex, FAR int *prioceiling);
+static inline bool nxmutex_is_locked(FAR mutex_t *mutex)
+{
+ return NXSEM_MACQUIRED(mutex->sem.val.mholder);
+}
/****************************************************************************
- * Name: nxmutex_setprioceiling
+ * Name: nxmutex_destroy
*
* Description:
- * This function attempts to set the priority ceiling of a mutex.
+ * This function initializes the UNNAMED mutex. Following a
+ * successful call to nxmutex_init(), the mutex may be used in subsequent
+ * calls to nxmutex_lock(), nxmutex_unlock(), and nxmutex_trylock(). The
+ * mutex remains usable until it is destroyed.
*
* Parameters:
- * mutex - mutex descriptor.
- * prioceiling - mutex priority ceiling value to set.
- * old_ceiling - location to return the mutex ceiling priority set before.
+ * mutex - Semaphore 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
+ * returned on success. A negated errno value is returned on failure.
*
****************************************************************************/
-int nxmutex_setprioceiling(FAR mutex_t *mutex, int prioceiling,
- FAR int *old_ceiling);
+static inline int nxmutex_destroy(FAR mutex_t *mutex)
+{
+ return nxsem_destroy(&mutex->sem);
+}
/****************************************************************************
- * Name: nxrmutex_init
+ * Name: nxmutex_lock
*
* 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.
+ * This function attempts to lock the mutex referenced by 'mutex'. The
+ * mutex is implemented with a semaphore, so if the semaphore value is
+ * (<=) zero, then the calling task will not return until it successfully
+ * acquires the lock.
*
* Parameters:
- * rmutex - Recursive mutex to be initialized
+ * mutex - 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:
*
****************************************************************************/
-int nxrmutex_init(FAR rmutex_t *rmutex);
+static inline int nxmutex_lock(FAR mutex_t *mutex)
+{
+ int ret;
+
+ ret = nxsem_wait(&mutex->sem);
+ if (ret >= 0)
+ {
+ nxmutex_add_backtrace(mutex);
+ }
+
+ return ret;
+}
/****************************************************************************
- * Name: nxrmutex_destroy
+ * Name: nxmutex_trylock
*
* Description:
- * This function destroy the UNNAMED recursive mutex.
+ * This function locks the mutex only if the mutex is currently not locked.
+ * If the mutex has been locked already, the call returns without blocking.
*
* Parameters:
- * rmutex - Recursive mutex to be destroyed
+ * mutex - 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 mutex
+ * -EAGAIN - The mutex is not available.
*
****************************************************************************/
-int nxrmutex_destroy(FAR rmutex_t *rmutex);
+static inline int nxmutex_trylock(FAR mutex_t *mutex)
+{
+ int ret;
+
+ ret = nxsem_trywait(&mutex->sem);
+ if (ret >= 0)
+ {
+ nxmutex_add_backtrace(mutex);
+ }
+
+ return ret;
+}
/****************************************************************************
- * Name: nxrmutex_is_hold
+ * Name: nxmutex_unlock
*
* Description:
- * This function check whether the calling thread hold the recursive mutex
- * referenced by 'rmutex'.
+ * This function attempts to unlock the mutex referenced by 'mutex'.
*
* Parameters:
- * rmutex - Recursive mutex descriptor.
+ * mutex - 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.
*
****************************************************************************/
-bool nxrmutex_is_hold(FAR rmutex_t *rmutex);
+static inline int nxmutex_unlock(FAR mutex_t *mutex)
+{
+ return nxsem_post(&mutex->sem);
+}
/****************************************************************************
- * Name: nxrmutex_is_recursive
+ * Name: nxmutex_reset
*
* Description:
- * This function check whether the recursive mutex is currently held
- * recursively. That is, whether it's locked more than once by the
- * current holder.
- * Note that this is inherently racy unless the calling thread is
- * holding the mutex.
+ * This function reset lock state.
*
* Parameters:
- * rmutex - Recursive mutex descriptor.
- *
- * Return Value:
- * If rmutex has returned to True recursively, otherwise returns false.
+ * mutex - mutex descriptor.
*
****************************************************************************/
-bool nxrmutex_is_recursive(FAR rmutex_t *rmutex);
+#if defined(CONFIG_BUILD_FLAT) || defined(__KERNEL__)
+static inline void nxmutex_reset(FAR mutex_t *mutex)
+{
+ nxsem_reset(&mutex->sem, 1);
+}
+#endif
/****************************************************************************
- * Name: nxrmutex_get_holder
+ * Name: nxmutex_breaklock
*
* Description:
- * This function get the holder of the mutex referenced by 'mutex'.
- * Note that this is inherently racy unless the calling thread is
- * holding the mutex.
+ * This function attempts to break the mutex
*
* Parameters:
- * rmutex - Rmutex descriptor.
+ * mutex - Mutex descriptor.
+ * locked - Is the mutex break success
*
* 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:
*
****************************************************************************/
-int nxrmutex_get_holder(FAR rmutex_t *rmutex);
+static inline int nxmutex_breaklock(FAR mutex_t *mutex,
+ FAR unsigned int *locked)
+{
+ int ret = OK;
+
+ *locked = false;
+ ret = nxmutex_unlock(mutex);
+ if (ret >= 0)
+ {
+ *locked = true;
+ }
+
+ return ret;
+}
/****************************************************************************
- * Name: nxrmutex_is_locked
+ * Name: nxmutex_restorelock
*
* Description:
- * This function get the lock state the recursive mutex
- * referenced by 'rmutex'.
- * Note that this is inherently racy unless the calling thread is
- * holding the mutex.
+ * This function attempts to restore the mutex.
*
* Parameters:
- * rmutex - Recursive mutex descriptor.
+ * mutex - mutex descriptor.
+ * locked - true: it's mean that the mutex is broke success
*
* 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
*
****************************************************************************/
-bool nxrmutex_is_locked(FAR rmutex_t *rmutex);
+static inline int nxmutex_restorelock(FAR mutex_t *mutex,
+ unsigned int locked)
+{
+ return locked ? nxmutex_lock(mutex) : OK;
+}
/****************************************************************************
- * Name: nrxmutex_lock
+ * Name: nxmutex_set_protocol
*
* Description:
- * This function attempts to lock the recursive mutex referenced by
- * 'rmutex'.The recursive mutex can be locked multiple times in the same
- * thread.
+ * This function attempts to set the priority protocol of a mutex.
*
* Parameters:
- * rmutex - Recursive mutex descriptor.
+ * mutex - mutex descriptor.
+ * protocol - mutex protocol value to set.
*
* 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:
+ * returned on success. A negated errno value is returned on failure
*
****************************************************************************/
-int nxrmutex_lock(FAR rmutex_t *rmutex);
+static inline int nxmutex_set_protocol(FAR mutex_t *mutex, int protocol)
+{
+ return nxsem_set_protocol(&mutex->sem, protocol);
+}
/****************************************************************************
- * Name: nxrmutex_trylock
+ * Name: nxmutex_getprioceiling
*
* 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.
+ * This function attempts to get the priority ceiling of a mutex.
*
* Parameters:
- * rmutex - Recursive mutex descriptor.
+ * mutex - mutex descriptor.
+ * prioceiling - location to return the mutex priority ceiling.
*
* 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.
+ * returned on success. A negated errno value is returned on failure
*
****************************************************************************/
-int nxrmutex_trylock(FAR rmutex_t *rmutex);
+#ifdef CONFIG_PRIORITY_PROTECT
+static inline int nxmutex_getprioceiling(FAR const mutex_t *mutex,
+ FAR int *prioceiling)
+{
+ return nxsem_getprioceiling(&mutex->sem, prioceiling);
+}
+#endif
/****************************************************************************
- * Name: nxrmutex_ticklock
+ * Name: nxmutex_setprioceiling
*
* Description:
- * This function attempts to lock the mutex referenced by 'mutex'. If the
- * mutex value is (<=) zero, then the calling task will not return until it
- * successfully acquires the lock or timed out
+ * This function attempts to set the priority ceiling of a mutex.
*
- * Input Parameters:
- * rmutex - Rmutex object
- * delay - Ticks to wait from the start time until the semaphore is
- * posted. If ticks is zero, then this function is equivalent
- * to nxrmutex_trylock().
+ * Parameters:
+ * mutex - mutex descriptor.
+ * prioceiling - mutex priority ceiling value to set.
+ * old_ceiling - location to return the mutex ceiling priority set before.
*
- * Returned Value:
- * OK The mutex successfully acquires
- * EINVAL The mutex argument does not refer to a valid mutex. Or the
- * thread would have blocked, and the abstime parameter specified
- * a nanoseconds field value less than zero or greater than or
- * equal to 1000 million.
- * ETIMEDOUT The mutex could not be locked before the specified timeout
- * expired.
- * EDEADLK A deadlock condition was detected.
+ * 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
*
****************************************************************************/
-int nxrmutex_ticklock(FAR rmutex_t *rmutex, uint32_t delay);
+#ifdef CONFIG_PRIORITY_PROTECT
+static inline int nxmutex_setprioceiling(FAR mutex_t *mutex, int prioceiling,
+ FAR int *old_ceiling)
+{
+ return nxsem_setprioceiling(&mutex->sem, prioceiling, old_ceiling);
+}
+#endif
/****************************************************************************
- * Name: nxrmutex_clocklock
+ * Name: nxrmutex_init
*
* Description:
- * This function attempts to lock the mutex referenced by 'mutex'. If the
- * mutex value is (<=) zero, then the calling task will not return until it
- * successfully acquires the lock or timed out
+ * 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.
*
- * Input Parameters:
- * rmutex - Rmutex object
- * clockid - The clock to be used as the time base
- * abstime - The absolute time when the mutex lock timed out
+ * Parameters:
+ * rmutex - Recursive mutex to be initialized
*
- * Returned Value:
- * OK The mutex successfully acquires
- * EINVAL The mutex argument does not refer to a valid mutex. Or the
- * thread would have blocked, and the abstime parameter specified
- * a nanoseconds field value less than zero or greater than or
- * equal to 1000 million.
- * ETIMEDOUT The mutex could not be locked before the specified timeout
- * expired.
- * EDEADLK A deadlock condition was detected.
+ * 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.
*
****************************************************************************/
-int nxrmutex_clocklock(FAR rmutex_t *rmutex, clockid_t clockid,
- FAR const struct timespec *abstime);
+static inline int nxrmutex_init(FAR rmutex_t *rmutex)
+{
+ rmutex->count = 0;
+ return nxmutex_init(&rmutex->mutex);
+}
/****************************************************************************
- * Name: nxrmutex_timedlock
+ * Name: nxrmutex_destroy
*
* Description:
- * This function attempts to lock the mutex . If the mutex value
- * is (<=) zero,then the calling task will not return until it
- * successfully acquires the lock or timed out
+ * This function destroy the UNNAMED recursive mutex.
*
- * Input Parameters:
- * rmutex - Rmutex object
- * timeout - The time when mutex lock timed out
+ * Parameters:
+ * rmutex - Recursive mutex to be destroyed
*
- * Returned Value:
- * OK The mutex successfully acquires
- * EINVAL The mutex argument does not refer to a valid mutex. Or the
- * thread would have blocked, and the abstime parameter specified
- * a nanoseconds field value less than zero or greater than or
- * equal to 1000 million.
- * ETIMEDOUT The mutex could not be locked before the specified timeout
- * expired.
- * EDEADLK A deadlock condition was detected.
- * ECANCELED May be returned if the thread is canceled while waiting.
+ * 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.
*
****************************************************************************/
-int nxrmutex_timedlock(FAR rmutex_t *rmutex, unsigned int timeout);
+static inline int nxrmutex_destroy(FAR rmutex_t *rmutex)
+{
+ int ret = nxmutex_destroy(&rmutex->mutex);
+
+ if (ret >= 0)
+ {
+ rmutex->count = 0;
+ }
+
+ return ret;
+}
/****************************************************************************
- * Name: nxrmutex_unlock
+ * Name: nxrmutex_is_hold
*
* Description:
- * This function attempts to unlock the recursive mutex
+ * This function check whether the calling thread hold 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.
*
****************************************************************************/
-int nxrmutex_unlock(FAR rmutex_t *rmutex);
+static inline bool nxrmutex_is_hold(FAR rmutex_t *rmutex)
Review Comment:
change ALL inline to inline_function
##########
sched/semaphore/sem_trywait.c:
##########
@@ -63,8 +63,11 @@
int nxsem_trywait_slow(FAR sem_t *sem)
{
irqstate_t flags;
- int32_t semcount;
- int ret;
+ int ret = -EAGAIN;
+ bool mutex = NXSEM_IS_MUTEX(sem);
+ FAR atomic_t *const val = mutex ? NXSEM_MHOLDER(sem) : NXSEM_COUNT(sem);
Review Comment:
`FAR const atomic_t *val `
##########
sched/semaphore/sem_post.c:
##########
@@ -73,10 +73,12 @@ int nxsem_post_slow(FAR sem_t *sem)
{
FAR struct tcb_s *stcb = NULL;
irqstate_t flags;
- int32_t sem_count;
#if defined(CONFIG_PRIORITY_INHERITANCE) || defined(CONFIG_PRIORITY_PROTECT)
uint8_t proto;
#endif
+ bool sem_is_blocking = false;
Review Comment:
blocking
##########
sched/semaphore/sem_destroy.c:
##########
@@ -61,6 +61,9 @@
int nxsem_destroy(FAR sem_t *sem)
{
int32_t old;
+ bool mutex = NXSEM_IS_MUTEX(sem);
+ FAR atomic_t *const val = mutex ? NXSEM_MHOLDER(sem) : NXSEM_COUNT(sem);
Review Comment:
FAR const atomic_t *val
##########
sched/semaphore/sem_wait.c:
##########
@@ -218,6 +270,16 @@ int nxsem_wait_slow(FAR sem_t *sem)
#endif
}
+ /* If this now holds the mutex, set the holder TID and the lock bit */
+
+ if (mutex && ret == OK)
+ {
+ uint32_t blocking =
Review Comment:
blocking_bit
##########
sched/semaphore/sem_trywait.c:
##########
@@ -74,29 +77,57 @@ int nxsem_trywait_slow(FAR sem_t *sem)
/* If the semaphore is available, give it to the requesting task */
- semcount = atomic_read(NXSEM_COUNT(sem));
+ if (mutex)
+ {
+ new = nxsched_gettid();
+ }
+
+ old = atomic_read(NXSEM_COUNT(sem));
do
{
- if (semcount <= 0)
+ if (mutex)
{
- leave_critical_section(flags);
- return -EAGAIN;
+ if (NXSEM_MACQUIRED(old))
+ {
+ goto out;
+ }
+ }
+ else
+ {
+ if (old <= 0)
+ {
+ goto out;
+ }
+
+ new = old - 1;
}
}
- while (!atomic_try_cmpxchg_acquire(NXSEM_COUNT(sem),
- &semcount, semcount - 1));
+ while (!atomic_try_cmpxchg_acquire(val, &old, new));
/* It is, let the task take the semaphore */
ret = nxsem_protect_wait(sem);
if (ret < 0)
{
- atomic_fetch_add(NXSEM_COUNT(sem), 1);
+ if (mutex)
+ {
+ atomic_set(NXSEM_MHOLDER(sem), NXSEM_NO_MHOLDER);
+ }
+ else
+ {
+ atomic_fetch_add(NXSEM_COUNT(sem), 1);
+ }
+
leave_critical_section(flags);
Review Comment:
change to `goto out;`
##########
sched/semaphore/sem_trywait.c:
##########
@@ -74,29 +77,57 @@ int nxsem_trywait_slow(FAR sem_t *sem)
/* If the semaphore is available, give it to the requesting task */
- semcount = atomic_read(NXSEM_COUNT(sem));
+ if (mutex)
+ {
+ new = nxsched_gettid();
+ }
+
+ old = atomic_read(NXSEM_COUNT(sem));
Review Comment:
`old = atomic_read(val);`
##########
libs/libc/semaphore/sem_getvalue.c:
##########
@@ -50,10 +50,6 @@
* be zero or a negative number whose absolute value represents the number
* of tasks waiting for the semaphore.
*
- * For a mutex, this sets svalue to 1 for a non-acquired mutex, 0 for an
Review Comment:
let's remove the change from the previous patch directly
##########
libs/libc/semaphore/sem_post.c:
##########
@@ -128,9 +128,9 @@ int nxsem_post(FAR sem_t *sem)
#ifndef CONFIG_LIBC_ARCH_ATOMIC
- if ((sem->flags & SEM_TYPE_MUTEX)
-# if defined(CONFIG_PRIORITY_PROTECT) || defined(CONFIG_PRIORITY_INHERITANCE)
- && (sem->flags & SEM_PRIO_MASK) == SEM_PRIO_NONE
+ if (NXSEM_IS_MUTEX(sem)
+# if defined(CONFIG_PRIORITY_PROTECT)
Review Comment:
```
# ifdef CONFIG_PRIORITY_PROTECT
```
##########
libs/libc/semaphore/sem_trywait.c:
##########
@@ -123,9 +123,9 @@ int nxsem_trywait(FAR sem_t *sem)
#ifndef CONFIG_LIBC_ARCH_ATOMIC
- if ((sem->flags & SEM_TYPE_MUTEX)
-#if defined(CONFIG_PRIORITY_PROTECT) || defined(CONFIG_PRIORITY_INHERITANCE)
- && (sem->flags & SEM_PRIO_MASK) == SEM_PRIO_NONE
+ if (NXSEM_IS_MUTEX(sem)
+#if defined(CONFIG_PRIORITY_PROTECT)
Review Comment:
ditto
##########
libs/libc/semaphore/sem_getvalue.c:
##########
@@ -67,25 +63,9 @@
int nxsem_get_value(FAR sem_t *sem, FAR int *sval)
{
- if (sem != NULL && sval != NULL)
+ if (sem != NULL && sval != NULL && !NXSEM_IS_MUTEX(sem))
{
- if (NXSEM_IS_MUTEX(sem))
Review Comment:
ditto
##########
sched/semaphore/sem_holder.c:
##########
@@ -876,7 +876,9 @@ void nxsem_canceled(FAR struct tcb_s *stcb, FAR sem_t *sem)
{
/* Check our assumptions */
- DEBUGASSERT(atomic_read(NXSEM_COUNT(sem)) <= 0);
+ DEBUGASSERT((NXSEM_IS_MUTEX(sem) || atomic_read(NXSEM_COUNT(sem)) <= 0));
+ DEBUGASSERT((!NXSEM_IS_MUTEX(sem) ||
Review Comment:
yes, you are right, but let's remove the redundant ().
##########
include/nuttx/mutex.h:
##########
@@ -354,407 +384,497 @@ void nxmutex_reset(FAR mutex_t *mutex);
*
****************************************************************************/
-int nxmutex_breaklock(FAR mutex_t *mutex, FAR unsigned int *locked);
+int nxrmutex_breaklock(FAR rmutex_t *rmutex, FAR unsigned int *count);
/****************************************************************************
- * Name: nxmutex_restorelock
+ * Name: nxrmutex_restorelock
*
* Description:
- * This function attempts to restore the mutex.
+ * This function attempts to restore the recursive mutex.
*
* Parameters:
- * mutex - mutex descriptor.
- * locked - true: it's mean that the mutex is broke success
+ * 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
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
*
****************************************************************************/
-int nxmutex_restorelock(FAR mutex_t *mutex, unsigned int locked);
+int nxrmutex_restorelock(FAR rmutex_t *rmutex, unsigned int count);
+
+#define nxrmutex_set_protocol(rmutex, protocol) \
+ nxmutex_set_protocol(&(rmutex)->mutex, protocol)
+#define nxrmutex_getprioceiling(rmutex, prioceiling) \
+ nxmutex_getprioceiling(&(rmutex)->mutex, prioceiling)
+#define nxrmutex_setprioceiling(rmutex, prioceiling, old_ceiling) \
+ nxmutex_setprioceiling(&(rmutex)->mutex, prioceiling, old_ceiling)
+
+/****************************************************************************
+ * Inline functions
+ ****************************************************************************/
/****************************************************************************
- * Name: nxmutex_set_protocol
+ * Name: nxmutex_get_holder
*
* Description:
- * This function attempts to set the priority protocol of a mutex.
+ * This function get the holder of the mutex referenced by 'mutex'.
+ * Note that this is inherently racy unless the calling thread is
+ * holding the mutex.
*
* Parameters:
- * mutex - mutex descriptor.
- * protocol - mutex protocol value to set.
+ * mutex - 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
*
****************************************************************************/
-int nxmutex_set_protocol(FAR mutex_t *mutex, int protocol);
+static inline pid_t nxmutex_get_holder(FAR mutex_t *mutex)
+{
+ uint32_t mholder = mutex->sem.val.mholder & ~NXSEM_MBLOCKING_BIT;
Review Comment:
let's use atomic operation: `atomic_read(NXSEM_MHOLDER(&mutex->sem))`
##########
sched/semaphore/sem_holder.c:
##########
@@ -876,7 +876,9 @@ void nxsem_canceled(FAR struct tcb_s *stcb, FAR sem_t *sem)
{
/* Check our assumptions */
- DEBUGASSERT(atomic_read(NXSEM_COUNT(sem)) <= 0);
+ DEBUGASSERT((NXSEM_IS_MUTEX(sem) || atomic_read(NXSEM_COUNT(sem)) <= 0));
Review Comment:
yes, you are right, but let's remove the redundant ()
##########
include/nuttx/mutex.h:
##########
@@ -354,407 +384,497 @@ void nxmutex_reset(FAR mutex_t *mutex);
*
****************************************************************************/
-int nxmutex_breaklock(FAR mutex_t *mutex, FAR unsigned int *locked);
+int nxrmutex_breaklock(FAR rmutex_t *rmutex, FAR unsigned int *count);
/****************************************************************************
- * Name: nxmutex_restorelock
+ * Name: nxrmutex_restorelock
*
* Description:
- * This function attempts to restore the mutex.
+ * This function attempts to restore the recursive mutex.
*
* Parameters:
- * mutex - mutex descriptor.
- * locked - true: it's mean that the mutex is broke success
+ * 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
+ * returned on success. A negated errno value is returned on failure.
+ * Possible returned errors:
*
****************************************************************************/
-int nxmutex_restorelock(FAR mutex_t *mutex, unsigned int locked);
+int nxrmutex_restorelock(FAR rmutex_t *rmutex, unsigned int count);
+
+#define nxrmutex_set_protocol(rmutex, protocol) \
+ nxmutex_set_protocol(&(rmutex)->mutex, protocol)
+#define nxrmutex_getprioceiling(rmutex, prioceiling) \
+ nxmutex_getprioceiling(&(rmutex)->mutex, prioceiling)
+#define nxrmutex_setprioceiling(rmutex, prioceiling, old_ceiling) \
+ nxmutex_setprioceiling(&(rmutex)->mutex, prioceiling, old_ceiling)
+
+/****************************************************************************
+ * Inline functions
+ ****************************************************************************/
/****************************************************************************
- * Name: nxmutex_set_protocol
+ * Name: nxmutex_get_holder
*
* Description:
- * This function attempts to set the priority protocol of a mutex.
+ * This function get the holder of the mutex referenced by 'mutex'.
+ * Note that this is inherently racy unless the calling thread is
+ * holding the mutex.
*
* Parameters:
- * mutex - mutex descriptor.
- * protocol - mutex protocol value to set.
+ * mutex - 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
*
****************************************************************************/
-int nxmutex_set_protocol(FAR mutex_t *mutex, int protocol);
+static inline pid_t nxmutex_get_holder(FAR mutex_t *mutex)
+{
+ uint32_t mholder = mutex->sem.val.mholder & ~NXSEM_MBLOCKING_BIT;
+ return NXSEM_MACQUIRED(mholder) ? (pid_t)mholder : -1;
+}
/****************************************************************************
- * Name: nxmutex_getprioceiling
+ * Name: nxmutex_is_locked
*
* Description:
- * This function attempts to get the priority ceiling of a mutex.
+ * This function get the lock state the mutex referenced by 'mutex'.
+ * Note that this is inherently racy unless the calling thread is
+ * holding the mutex.
*
* Parameters:
- * mutex - mutex descriptor.
- * prioceiling - location to return the mutex priority ceiling.
+ * mutex - 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
*
****************************************************************************/
-int nxmutex_getprioceiling(FAR const mutex_t *mutex, FAR int *prioceiling);
+static inline bool nxmutex_is_locked(FAR mutex_t *mutex)
+{
+ return NXSEM_MACQUIRED(mutex->sem.val.mholder);
Review Comment:
ditto
##########
libs/libc/misc/lib_mutex.c:
##########
@@ -35,30 +35,14 @@
* Pre-processor Definitions
****************************************************************************/
-#define NXMUTEX_RESET ((pid_t)-2)
-
/****************************************************************************
* Private Functions
Review Comment:
remove the private function and preprocess banner
--
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
