From b74a50775f6209c2a5a21ff5a8ce4eb2ca0212cf Mon Sep 17 00:00:00 2001 From: zhangyuan29 Date: Thu, 22 Aug 2024 10:25:32 +0800 Subject: [PATCH] sem: change sem wait to atomic operation Add sem_wait fast operations, use atomic to ensure atomicity of semcount operations, and do not depend on critical section. Test with robot: before modify: nxmutex_lock cost: 78 ns nxmutex_unlock cost: 82 ns after modify: nxmutex_lock cost: 28 ns nxmutex_unlock cost: 14 ns Signed-off-by: zhangyuan29 --- include/limits.h | 2 +- include/semaphore.h | 4 +- libs/libc/semaphore/sem_init.c | 2 +- sched/semaphore/sem_destroy.c | 11 +++- sched/semaphore/sem_holder.c | 4 +- sched/semaphore/sem_post.c | 83 ++++++++++++++++++++----- sched/semaphore/sem_recover.c | 4 +- sched/semaphore/sem_reset.c | 12 +++- sched/semaphore/sem_trywait.c | 110 +++++++++++++++++++++++---------- sched/semaphore/sem_wait.c | 82 ++++++++++++++++++------ sched/semaphore/sem_waitirq.c | 4 +- sched/semaphore/semaphore.h | 7 +++ 12 files changed, 247 insertions(+), 78 deletions(-) diff --git a/include/limits.h b/include/limits.h index e46f30898d..45a4aa7d41 100644 --- a/include/limits.h +++ b/include/limits.h @@ -194,7 +194,7 @@ /* Required for POSIX semaphores */ #define _POSIX_SEM_NSEMS_MAX INT_MAX -#define _POSIX_SEM_VALUE_MAX 0x7fff +#define _POSIX_SEM_VALUE_MAX INT_MAX /* Numerical limits. These values may be increased from the POSIX minimum * values above or made indeterminate diff --git a/include/semaphore.h b/include/semaphore.h index bfd96cab57..0710b403df 100644 --- a/include/semaphore.h +++ b/include/semaphore.h @@ -73,7 +73,7 @@ struct semholder_s FAR struct semholder_s *tlink; /* List of task held semaphores */ FAR struct sem_s *sem; /* Ths corresponding semaphore */ FAR struct tcb_s *htcb; /* Ths corresponding TCB */ - int16_t counts; /* Number of counts owned by this holder */ + int32_t counts; /* Number of counts owned by this holder */ }; #if CONFIG_SEM_PREALLOCHOLDERS > 0 @@ -104,7 +104,7 @@ struct semholder_s struct sem_s { - volatile int16_t semcount; /* >0 -> Num counts available */ + volatile int32_t semcount; /* >0 -> Num counts available */ /* <0 -> Num tasks waiting for semaphore */ /* If priority inheritance is enabled, then we have to keep track of which diff --git a/libs/libc/semaphore/sem_init.c b/libs/libc/semaphore/sem_init.c index 37facb0b1f..c1b9bd7d7e 100644 --- a/libs/libc/semaphore/sem_init.c +++ b/libs/libc/semaphore/sem_init.c @@ -70,7 +70,7 @@ int nxsem_init(FAR sem_t *sem, int pshared, unsigned int value) /* Initialize the semaphore count */ - sem->semcount = (int16_t)value; + sem->semcount = (int32_t)value; /* Initialize semaphore wait list */ diff --git a/sched/semaphore/sem_destroy.c b/sched/semaphore/sem_destroy.c index f207b339bb..b77e58ccb3 100644 --- a/sched/semaphore/sem_destroy.c +++ b/sched/semaphore/sem_destroy.c @@ -60,6 +60,8 @@ int nxsem_destroy(FAR sem_t *sem) { + int32_t old; + DEBUGASSERT(sem != NULL); /* There is really no particular action that we need @@ -72,10 +74,15 @@ int nxsem_destroy(FAR sem_t *sem) * leave the count unchanged but still return OK. */ - if (sem->semcount >= 0) + old = atomic_read(NXSEM_COUNT(sem)); + do { - sem->semcount = 1; + if (old < 0) + { + break; + } } + while (!atomic_try_cmpxchg_release(NXSEM_COUNT(sem), &old, 1)); /* Release holders of the semaphore */ diff --git a/sched/semaphore/sem_holder.c b/sched/semaphore/sem_holder.c index a51ea9a30a..ebd6578d9f 100644 --- a/sched/semaphore/sem_holder.c +++ b/sched/semaphore/sem_holder.c @@ -880,7 +880,7 @@ void nxsem_canceled(FAR struct tcb_s *stcb, FAR sem_t *sem) { /* Check our assumptions */ - DEBUGASSERT(sem->semcount <= 0); + DEBUGASSERT(atomic_read(NXSEM_COUNT(sem)) <= 0); /* Adjust the priority of every holder as necessary */ @@ -978,7 +978,7 @@ void nxsem_release_all(FAR struct tcb_s *htcb) * that was taken by sem_wait() or sem_post(). */ - sem->semcount++; + atomic_fetch_add(NXSEM_COUNT(sem), 1); } } diff --git a/sched/semaphore/sem_post.c b/sched/semaphore/sem_post.c index 33293ad037..d63d3fb2aa 100644 --- a/sched/semaphore/sem_post.c +++ b/sched/semaphore/sem_post.c @@ -37,11 +37,11 @@ #include "semaphore/semaphore.h" /**************************************************************************** - * Public Functions + * Private Functions ****************************************************************************/ /**************************************************************************** - * Name: nxsem_post + * Name: nxsem_post_slow * * Description: * When a kernel thread has finished with a semaphore, it will call @@ -69,17 +69,15 @@ * ****************************************************************************/ -int nxsem_post(FAR sem_t *sem) +static int nxsem_post_slow(FAR sem_t *sem) { FAR struct tcb_s *stcb = NULL; irqstate_t flags; - int16_t sem_count; + int32_t sem_count; #if defined(CONFIG_PRIORITY_INHERITANCE) || defined(CONFIG_PRIORITY_PROTECT) uint8_t proto; #endif - DEBUGASSERT(sem != NULL); - /* The following operations must be performed with interrupts * disabled because sem_post() may be called from an interrupt * handler. @@ -87,15 +85,19 @@ int nxsem_post(FAR sem_t *sem) flags = enter_critical_section(); - sem_count = sem->semcount; - /* Check the maximum allowable value */ - if (sem_count >= SEM_VALUE_MAX) + sem_count = atomic_read(NXSEM_COUNT(sem)); + do { - leave_critical_section(flags); - return -EOVERFLOW; + if (sem_count >= SEM_VALUE_MAX) + { + leave_critical_section(flags); + return -EOVERFLOW; + } } + while (!atomic_try_cmpxchg_release(NXSEM_COUNT(sem), &sem_count, + sem_count + 1)); /* Perform the semaphore unlock operation, releasing this task as a * holder then also incrementing the count on the semaphore. @@ -115,8 +117,6 @@ int nxsem_post(FAR sem_t *sem) */ nxsem_release_holder(sem); - sem_count++; - sem->semcount = sem_count; #if defined(CONFIG_PRIORITY_INHERITANCE) || defined(CONFIG_PRIORITY_PROTECT) /* Don't let any unblocked tasks run until we complete any priority @@ -138,7 +138,7 @@ int nxsem_post(FAR sem_t *sem) * there must be some task waiting for the semaphore. */ - if (sem_count <= 0) + if (sem_count < 0) { /* Check if there are any tasks in the waiting for semaphore * task list that are waiting for this semaphore. This is a @@ -217,3 +217,58 @@ int nxsem_post(FAR sem_t *sem) return OK; } + +/**************************************************************************** + * Public Functions + ****************************************************************************/ + +/**************************************************************************** + * Name: nxsem_post + * + * Description: + * When a kernel thread has finished with a semaphore, it will call + * nxsem_post(). This function unlocks the semaphore referenced by sem + * by performing the semaphore unlock operation on that semaphore. + * + * If the semaphore value resulting from this operation is positive, then + * no tasks were blocked waiting for the semaphore to become unlocked; the + * semaphore is simply incremented. + * + * If the value of the semaphore resulting from this operation is zero, + * then one of the tasks blocked waiting for the semaphore shall be + * allowed to return successfully from its call to nxsem_wait(). + * + * Input Parameters: + * sem - Semaphore descriptor + * + * Returned 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. + * + * Assumptions: + * This function may be called from an interrupt handler. + * + ****************************************************************************/ + +int nxsem_post(FAR sem_t *sem) +{ + DEBUGASSERT(sem != NULL); + + /* If this is a mutex, we can try to unlock the mutex in fast mode, + * else try to get it in slow mode. + */ + +#if !defined(CONFIG_PRIORITY_INHERITANCE) && !defined(CONFIG_PRIORITY_PROTECT) + if (sem->flags & SEM_TYPE_MUTEX) + { + int32_t old = 0; + if (atomic_try_cmpxchg_release(NXSEM_COUNT(sem), &old, 1)) + { + return OK; + } + } +#endif + + return nxsem_post_slow(sem); +} diff --git a/sched/semaphore/sem_recover.c b/sched/semaphore/sem_recover.c index 1a6e7ea2bf..ad543a2a1e 100644 --- a/sched/semaphore/sem_recover.c +++ b/sched/semaphore/sem_recover.c @@ -85,7 +85,7 @@ void nxsem_recover(FAR struct tcb_s *tcb) if (tcb->task_state == TSTATE_WAIT_SEM) { FAR sem_t *sem = tcb->waitobj; - DEBUGASSERT(sem != NULL && sem->semcount < 0); + DEBUGASSERT(sem != NULL && atomic_read(NXSEM_COUNT(sem)) < 0); /* Restore the correct priority of all threads that hold references * to this semaphore. @@ -99,7 +99,7 @@ void nxsem_recover(FAR struct tcb_s *tcb) * place. */ - sem->semcount++; + atomic_fetch_add(NXSEM_COUNT(sem), 1); } /* Release all semphore holders for the task */ diff --git a/sched/semaphore/sem_reset.c b/sched/semaphore/sem_reset.c index 14418fe704..f038879442 100644 --- a/sched/semaphore/sem_reset.c +++ b/sched/semaphore/sem_reset.c @@ -60,6 +60,7 @@ int nxsem_reset(FAR sem_t *sem, int16_t count) { irqstate_t flags; + int32_t semcount; DEBUGASSERT(sem != NULL && count >= 0); @@ -80,7 +81,7 @@ int nxsem_reset(FAR sem_t *sem, int16_t count) * out counts to any waiting threads. */ - while (sem->semcount < 0 && count > 0) + while (atomic_read(NXSEM_COUNT(sem)) < 0 && count > 0) { /* Give out one counting, waking up one of the waiting threads * and, perhaps, kicking off a lot of priority inheritance @@ -98,10 +99,15 @@ int nxsem_reset(FAR sem_t *sem, int16_t count) * value of sem->semcount is already correct in this case. */ - if (sem->semcount >= 0) + semcount = atomic_read(NXSEM_COUNT(sem)); + do { - sem->semcount = count; + if (semcount < 0) + { + break; + } } + while (!atomic_try_cmpxchg_release(NXSEM_COUNT(sem), &semcount, count)); /* Allow any pending context switches to occur now */ diff --git a/sched/semaphore/sem_trywait.c b/sched/semaphore/sem_trywait.c index 8b0239ec9e..7789a63a84 100644 --- a/sched/semaphore/sem_trywait.c +++ b/sched/semaphore/sem_trywait.c @@ -38,6 +38,75 @@ #include "sched/sched.h" #include "semaphore/semaphore.h" +/**************************************************************************** + * Private Functions + ****************************************************************************/ + +/**************************************************************************** + * Name: nxsem_trywait_slow + * + * Description: + * This function locks the specified semaphore in slow mode. + * + * Input Parameters: + * sem - the semaphore descriptor + * + * Returned Value: + * + * EINVAL - Invalid attempt to get the semaphore + * EAGAIN - The semaphore is not available. + * + * Assumptions: + * + ****************************************************************************/ + +static int nxsem_trywait_slow(FAR sem_t *sem) +{ + FAR struct tcb_s *rtcb; + irqstate_t flags; + int32_t semcount; + int ret; + + /* The following operations must be performed with interrupts disabled + * because sem_post() may be called from an interrupt handler. + */ + + flags = enter_critical_section(); + rtcb = this_task(); + + /* If the semaphore is available, give it to the requesting task */ + + semcount = atomic_read(NXSEM_COUNT(sem)); + do + { + if (semcount <= 0) + { + leave_critical_section(flags); + return -EAGAIN; + } + } + while (!atomic_try_cmpxchg_acquire(NXSEM_COUNT(sem), + &semcount, semcount - 1)); + + /* It is, let the task take the semaphore */ + + ret = nxsem_protect_wait(sem); + if (ret < 0) + { + atomic_fetch_add(NXSEM_COUNT(sem), 1); + leave_critical_section(flags); + return ret; + } + + nxsem_add_holder(sem); + rtcb->waitobj = NULL; + + /* Interrupts may now be enabled. */ + + leave_critical_section(flags); + return ret; +} + /**************************************************************************** * Public Functions ****************************************************************************/ @@ -68,49 +137,28 @@ int nxsem_trywait(FAR sem_t *sem) { - FAR struct tcb_s *rtcb = this_task(); - irqstate_t flags; - int ret; - /* This API should not be called from the idleloop */ DEBUGASSERT(sem != NULL); DEBUGASSERT(!OSINIT_IDLELOOP() || !sched_idletask() || up_interrupt_context()); - /* The following operations must be performed with interrupts disabled - * because sem_post() may be called from an interrupt handler. + /* If this is a mutex, we can try to get the mutex in fast mode, + * else try to get it in slow mode. */ - flags = enter_critical_section(); - - /* If the semaphore is available, give it to the requesting task */ - - if (sem->semcount > 0) +#if !defined(CONFIG_PRIORITY_INHERITANCE) && !defined(CONFIG_PRIORITY_PROTECT) + if (sem->flags & SEM_TYPE_MUTEX) { - /* It is, let the task take the semaphore */ - - ret = nxsem_protect_wait(sem); - if (ret < 0) + int32_t old = 1; + if (atomic_try_cmpxchg_acquire(NXSEM_COUNT(sem), &old, 0)) { - leave_critical_section(flags); - return ret; + return OK; } - sem->semcount--; - nxsem_add_holder(sem); - rtcb->waitobj = NULL; - ret = OK; + return -EAGAIN; } - else - { - /* Semaphore is not available */ +#endif - ret = -EAGAIN; - } - - /* Interrupts may now be enabled. */ - - leave_critical_section(flags); - return ret; + return nxsem_trywait_slow(sem); } diff --git a/sched/semaphore/sem_wait.c b/sched/semaphore/sem_wait.c index 7ea4d36480..136369d5c3 100644 --- a/sched/semaphore/sem_wait.c +++ b/sched/semaphore/sem_wait.c @@ -38,16 +38,15 @@ #include "semaphore/semaphore.h" /**************************************************************************** - * Public Functions + * Private Functions ****************************************************************************/ /**************************************************************************** - * Name: nxsem_wait + * Name: nxsem_wait_slow * * Description: - * This function attempts to lock the semaphore referenced by 'sem'. If - * the semaphore value is (<=) zero, then the calling task will not return - * until it successfully acquires the lock. + * This function attempts to lock the semaphore referenced by 'sem' in + * slow mode. * * This is an internal OS interface. It is functionally equivalent to * sem_wait except that: @@ -69,17 +68,12 @@ * ****************************************************************************/ -int nxsem_wait(FAR sem_t *sem) +static int nxsem_wait_slow(FAR sem_t *sem) { FAR struct tcb_s *rtcb = this_task(); irqstate_t flags; int ret; - /* This API should not be called from interrupt handlers & idleloop */ - - DEBUGASSERT(sem != NULL && up_interrupt_context() == false); - DEBUGASSERT(!OSINIT_IDLELOOP() || !sched_idletask()); - /* The following operations must be performed with interrupts * disabled because nxsem_post() may be called from an interrupt * handler. @@ -91,21 +85,20 @@ int nxsem_wait(FAR sem_t *sem) /* Check if the lock is available */ - if (sem->semcount > 0) + if (atomic_fetch_sub(NXSEM_COUNT(sem), 1) > 0) { /* It is, let the task take the semaphore. */ ret = nxsem_protect_wait(sem); if (ret < 0) { + atomic_fetch_add(NXSEM_COUNT(sem), 1); leave_critical_section(flags); return ret; } - sem->semcount--; nxsem_add_holder(sem); rtcb->waitobj = NULL; - ret = OK; } /* The semaphore is NOT available, We will have to block the @@ -124,10 +117,6 @@ int nxsem_wait(FAR sem_t *sem) DEBUGASSERT(rtcb->waitobj == NULL); - /* Handle the POSIX semaphore (but don't set the owner yet) */ - - sem->semcount--; - /* Save the waited on semaphore in the TCB */ rtcb->waitobj = sem; @@ -224,6 +213,63 @@ int nxsem_wait(FAR sem_t *sem) return ret; } +/**************************************************************************** + * Public Functions + ****************************************************************************/ + +/**************************************************************************** + * Name: nxsem_wait + * + * Description: + * This function attempts to lock the semaphore referenced by 'sem'. If + * the semaphore value is (<=) zero, then the calling task will not return + * until it successfully acquires the lock. + * + * This is an internal OS interface. It is functionally equivalent to + * sem_wait except that: + * + * - It is not a cancellation point, and + * - It does not modify the errno value. + * + * Input Parameters: + * sem - Semaphore descriptor. + * + * Returned 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 get the semaphore + * - EINTR: The wait was interrupted by the receipt of a signal. + * + ****************************************************************************/ + +int nxsem_wait(FAR sem_t *sem) +{ + /* This API should not be called from interrupt handlers & idleloop */ + + DEBUGASSERT(sem != NULL && up_interrupt_context() == false); + DEBUGASSERT(!OSINIT_IDLELOOP() || !sched_idletask()); + + /* If this is a mutex, we can try to get the mutex in fast mode, + * else try to get it in slow mode. + */ + +#if !defined(CONFIG_PRIORITY_INHERITANCE) && !defined(CONFIG_PRIORITY_PROTECT) + if (sem->flags & SEM_TYPE_MUTEX) + { + int32_t old = 1; + if (atomic_try_cmpxchg_acquire(NXSEM_COUNT(sem), &old, 0)) + { + return OK; + } + } +#endif + + return nxsem_wait_slow(sem); +} + /**************************************************************************** * Name: nxsem_wait_uninterruptible * diff --git a/sched/semaphore/sem_waitirq.c b/sched/semaphore/sem_waitirq.c index 746c46e05f..9995f3529e 100644 --- a/sched/semaphore/sem_waitirq.c +++ b/sched/semaphore/sem_waitirq.c @@ -87,7 +87,7 @@ void nxsem_wait_irq(FAR struct tcb_s *wtcb, int errcode) * and already changed the task's state. */ - DEBUGASSERT(sem != NULL && sem->semcount < 0); + DEBUGASSERT(sem != NULL && atomic_read(NXSEM_COUNT(sem)) < 0); /* Restore the correct priority of all threads that hold references * to this semaphore. @@ -101,7 +101,7 @@ void nxsem_wait_irq(FAR struct tcb_s *wtcb, int errcode) * place. */ - sem->semcount++; + atomic_fetch_add(NXSEM_COUNT(sem), 1); /* Remove task from waiting list */ diff --git a/sched/semaphore/semaphore.h b/sched/semaphore/semaphore.h index b61085ea3c..fde697358a 100644 --- a/sched/semaphore/semaphore.h +++ b/sched/semaphore/semaphore.h @@ -31,10 +31,17 @@ #include #include #include +#include #include #include +/**************************************************************************** + * Pre-processor Definitions + ****************************************************************************/ + +#define NXSEM_COUNT(s) ((FAR atomic_t *)&(s)->semcount) + /**************************************************************************** * Public Function Prototypes ****************************************************************************/