semaphore: move param check to sem_xx level

for the speed improve

Signed-off-by: ligd <liguiding1@xiaomi.com>
This commit is contained in:
ligd 2022-10-21 00:00:08 +08:00 committed by Xiang Xiao
parent a4b378a583
commit a9c647d418
6 changed files with 255 additions and 246 deletions

View File

@ -25,6 +25,7 @@
#include <nuttx/config.h> #include <nuttx/config.h>
#include <sys/types.h> #include <sys/types.h>
#include <assert.h>
#include <limits.h> #include <limits.h>
#include <errno.h> #include <errno.h>
@ -63,35 +64,28 @@ int nxsem_init(FAR sem_t *sem, int pshared, unsigned int value)
{ {
UNUSED(pshared); UNUSED(pshared);
/* Verify that a semaphore was provided and the count is within the valid DEBUGASSERT(sem != NULL && value <= SEM_VALUE_MAX);
* range.
*/
if (sem != NULL && value <= SEM_VALUE_MAX) /* Initialize the semaphore count */
{
/* Initialize the semaphore count */
sem->semcount = (int16_t)value; sem->semcount = (int16_t)value;
/* Initialize semaphore wait list */ /* Initialize semaphore wait list */
dq_init(&sem->waitlist); dq_init(&sem->waitlist);
/* Initialize to support priority inheritance */ /* Initialize to support priority inheritance */
#ifdef CONFIG_PRIORITY_INHERITANCE #ifdef CONFIG_PRIORITY_INHERITANCE
sem->flags = 0; sem->flags = 0;
# if CONFIG_SEM_PREALLOCHOLDERS > 0 # if CONFIG_SEM_PREALLOCHOLDERS > 0
sem->hhead = NULL; sem->hhead = NULL;
# else # else
INITIALIZE_SEMHOLDER(&sem->holder[0]); INITIALIZE_SEMHOLDER(&sem->holder[0]);
INITIALIZE_SEMHOLDER(&sem->holder[1]); INITIALIZE_SEMHOLDER(&sem->holder[1]);
# endif # endif
#endif #endif
return OK; return OK;
}
return -EINVAL;
} }
/**************************************************************************** /****************************************************************************
@ -122,6 +116,16 @@ int sem_init(FAR sem_t *sem, int pshared, unsigned int value)
{ {
int ret; int ret;
/* Verify that a semaphore was provided and the count is within the valid
* range.
*/
if (sem == NULL || value > SEM_VALUE_MAX)
{
set_errno(EINVAL);
return ERROR;
}
ret = nxsem_init(sem, pshared, value); ret = nxsem_init(sem, pshared, value);
if (ret < 0) if (ret < 0)
{ {

View File

@ -97,19 +97,9 @@ int nxsem_clockwait(FAR sem_t *sem, clockid_t clockid,
int status; int status;
int ret = ERROR; int ret = ERROR;
DEBUGASSERT(sem != NULL && abstime != NULL);
DEBUGASSERT(up_interrupt_context() == false); DEBUGASSERT(up_interrupt_context() == false);
/* Verify the input parameters and, in case of an error, set
* errno appropriately.
*/
#ifdef CONFIG_DEBUG_FEATURES
if (!abstime || !sem)
{
return -EINVAL;
}
#endif
/* We will disable interrupts until we have completed the semaphore /* We will disable interrupts until we have completed the semaphore
* wait. We need to do this (as opposed to just disabling pre-emption) * wait. We need to do this (as opposed to just disabling pre-emption)
* because there could be interrupt handlers that are asynchronously * because there could be interrupt handlers that are asynchronously
@ -134,11 +124,13 @@ int nxsem_clockwait(FAR sem_t *sem, clockid_t clockid,
* with a valid timeout. * with a valid timeout.
*/ */
#ifdef CONFIG_DEBUG_FEATURES
if (abstime->tv_nsec < 0 || abstime->tv_nsec >= 1000000000) if (abstime->tv_nsec < 0 || abstime->tv_nsec >= 1000000000)
{ {
ret = -EINVAL; ret = -EINVAL;
goto out; goto out;
} }
#endif
/* Convert the timespec to clock ticks. We must have interrupts /* Convert the timespec to clock ticks. We must have interrupts
* disabled here so that this time stays valid until the wait begins. * disabled here so that this time stays valid until the wait begins.
@ -268,6 +260,16 @@ int sem_clockwait(FAR sem_t *sem, clockid_t clockid,
{ {
int ret; int ret;
/* Verify the input parameters and, in case of an error, set
* errno appropriately.
*/
if (sem == NULL || abstime == NULL)
{
set_errno(EINVAL);
return ERROR;
}
/* sem_timedwait() is a cancellation point */ /* sem_timedwait() is a cancellation point */
enter_cancellation_point(); enter_cancellation_point();

View File

@ -56,34 +56,29 @@
* *
****************************************************************************/ ****************************************************************************/
int nxsem_destroy (FAR sem_t *sem) int nxsem_destroy(FAR sem_t *sem)
{ {
/* Assure a valid semaphore is specified */ DEBUGASSERT(sem != NULL);
if (sem != NULL) /* There is really no particular action that we need
* take to destroy a semaphore. We will just reset
* the count to some reasonable value (0) and release
* ownership.
*
* Check if other threads are waiting on the semaphore.
* In this case, the behavior is undefined. We will:
* leave the count unchanged but still return OK.
*/
if (sem->semcount >= 0)
{ {
/* There is really no particular action that we need sem->semcount = 1;
* take to destroy a semaphore. We will just reset
* the count to some reasonable value (1) and release
* ownership.
*
* Check if other threads are waiting on the semaphore.
* In this case, the behavior is undefined. We will:
* leave the count unchanged but still return OK.
*/
if (sem->semcount >= 0)
{
sem->semcount = 1;
}
/* Release holders of the semaphore */
nxsem_destroyholder(sem);
return OK;
} }
return -EINVAL; /* Release holders of the semaphore */
nxsem_destroyholder(sem);
return OK;
} }
/**************************************************************************** /****************************************************************************
@ -114,6 +109,14 @@ int sem_destroy (FAR sem_t *sem)
{ {
int ret; int ret;
/* Assure a valid semaphore is specified */
if (sem == NULL)
{
set_errno(EINVAL);
return ERROR;
}
ret = nxsem_destroy(sem); ret = nxsem_destroy(sem);
if (ret < 0) if (ret < 0)
{ {

View File

@ -72,121 +72,112 @@ int nxsem_post(FAR sem_t *sem)
FAR struct tcb_s *stcb = NULL; FAR struct tcb_s *stcb = NULL;
irqstate_t flags; irqstate_t flags;
int16_t sem_count; int16_t sem_count;
int ret = -EINVAL;
/* Make sure we were supplied with a valid semaphore. */ DEBUGASSERT(sem != NULL);
if (sem != NULL) /* The following operations must be performed with interrupts
{ * disabled because sem_post() may be called from an interrupt
/* The following operations must be performed with interrupts * handler.
* disabled because sem_post() may be called from an interrupt */
* handler.
*/
flags = enter_critical_section(); flags = enter_critical_section();
sem_count = sem->semcount; sem_count = sem->semcount;
/* Check the maximum allowable value */ /* Check the maximum allowable value */
if (sem_count >= SEM_VALUE_MAX) DEBUGASSERT(sem_count < SEM_VALUE_MAX);
{
leave_critical_section(flags);
return -EOVERFLOW;
}
/* Perform the semaphore unlock operation, releasing this task as a /* Perform the semaphore unlock operation, releasing this task as a
* holder then also incrementing the count on the semaphore. * holder then also incrementing the count on the semaphore.
* *
* NOTE: When semaphores are used for signaling purposes, the holder * NOTE: When semaphores are used for signaling purposes, the holder
* of the semaphore may not be this thread! In this case, * of the semaphore may not be this thread! In this case,
* nxsem_release_holder() will do nothing. * nxsem_release_holder() will do nothing.
* *
* In the case of a mutex this could be simply resolved since there is * In the case of a mutex this could be simply resolved since there is
* only one holder but for the case of counting semaphores, there may * only one holder but for the case of counting semaphores, there may
* be many holders and if the holder is not this thread, then it is * be many holders and if the holder is not this thread, then it is
* not possible to know which thread/holder should be released. * not possible to know which thread/holder should be released.
* *
* For this reason, it is recommended that priority inheritance be * For this reason, it is recommended that priority inheritance be
* disabled via nxsem_set_protocol(SEM_PRIO_NONE) when the semaphore is * disabled via nxsem_set_protocol(SEM_PRIO_NONE) when the semaphore is
* initialized if the semaphore is to used for signaling purposes. * initialized if the semaphore is to used for signaling purposes.
*/ */
nxsem_release_holder(sem); nxsem_release_holder(sem);
sem_count++; sem_count++;
sem->semcount = sem_count; sem->semcount = sem_count;
#ifdef CONFIG_PRIORITY_INHERITANCE #ifdef CONFIG_PRIORITY_INHERITANCE
/* Don't let any unblocked tasks run until we complete any priority /* Don't let any unblocked tasks run until we complete any priority
* restoration steps. Interrupts are disabled, but we do not want * restoration steps. Interrupts are disabled, but we do not want
* the head of the ready-to-run list to be modified yet. * the head of the ready-to-run list to be modified yet.
* *
* NOTE: If this sched_lock is called from an interrupt handler, it * NOTE: If this sched_lock is called from an interrupt handler, it
* will do nothing. * will do nothing.
*/ */
sched_lock(); sched_lock();
#endif #endif
/* If the result of semaphore unlock is non-positive, then /* If the result of semaphore unlock is non-positive, then
* there must be some task waiting for the semaphore. * there must be some task waiting for the semaphore.
*/
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
* prioritized list so the first one we encounter is the one
* that we want.
*/ */
if (sem_count <= 0) stcb = (FAR struct tcb_s *)dq_peek(SEM_WAITLIST(sem));
if (stcb != NULL)
{ {
/* Check if there are any tasks in the waiting for semaphore /* The task will be the new holder of the semaphore when
* task list that are waiting for this semaphore. This is a * it is awakened.
* prioritized list so the first one we encounter is the one
* that we want.
*/ */
stcb = (FAR struct tcb_s *)dq_peek(SEM_WAITLIST(sem)); nxsem_add_holder_tcb(stcb, sem);
if (stcb != NULL) /* Stop the watchdog timer */
if (WDOG_ISACTIVE(&stcb->waitdog))
{ {
/* The task will be the new holder of the semaphore when wd_cancel(&stcb->waitdog);
* it is awakened.
*/
nxsem_add_holder_tcb(stcb, sem);
/* Stop the watchdog timer */
if (WDOG_ISACTIVE(&stcb->waitdog))
{
wd_cancel(&stcb->waitdog);
}
/* Restart the waiting task. */
up_unblock_task(stcb);
} }
#if 0 /* REVISIT: This can fire on IOB throttle semaphore */
else
{
/* This should not happen. */
DEBUGPANIC(); /* Restart the waiting task. */
}
#endif up_unblock_task(stcb);
} }
#if 0 /* REVISIT: This can fire on IOB throttle semaphore */
else
{
/* This should not happen. */
/* Check if we need to drop the priority of any threads holding DEBUGPANIC();
* this semaphore. The priority could have been boosted while they }
* held the semaphore.
*/
#ifdef CONFIG_PRIORITY_INHERITANCE
nxsem_restore_baseprio(stcb, sem);
sched_unlock();
#endif #endif
ret = OK;
/* Interrupts may now be enabled. */
leave_critical_section(flags);
} }
return ret; /* Check if we need to drop the priority of any threads holding
* this semaphore. The priority could have been boosted while they
* held the semaphore.
*/
#ifdef CONFIG_PRIORITY_INHERITANCE
nxsem_restore_baseprio(stcb, sem);
sched_unlock();
#endif
/* Interrupts may now be enabled. */
leave_critical_section(flags);
return OK;
} }
/**************************************************************************** /****************************************************************************
@ -222,6 +213,14 @@ int sem_post(FAR sem_t *sem)
{ {
int ret; int ret;
/* Make sure we were supplied with a valid semaphore. */
if (sem == NULL)
{
set_errno(EINVAL);
return ERROR;
}
ret = nxsem_post(sem); ret = nxsem_post(sem);
if (ret < 0) if (ret < 0)
{ {

View File

@ -75,41 +75,33 @@ int nxsem_trywait(FAR sem_t *sem)
DEBUGASSERT(sem != NULL && up_interrupt_context() == false); DEBUGASSERT(sem != NULL && up_interrupt_context() == false);
DEBUGASSERT(!OSINIT_IDLELOOP() || !sched_idletask()); DEBUGASSERT(!OSINIT_IDLELOOP() || !sched_idletask());
if (sem != NULL) /* The following operations must be performed with interrupts disabled
* because sem_post() may be called from an interrupt handler.
*/
flags = enter_critical_section();
/* If the semaphore is available, give it to the requesting task */
if (sem->semcount > 0)
{ {
/* The following operations must be performed with interrupts disabled /* It is, let the task take the semaphore */
* because sem_post() may be called from an interrupt handler.
*/
flags = enter_critical_section(); sem->semcount--;
nxsem_add_holder(sem);
/* If the semaphore is available, give it to the requesting task */ rtcb->waitobj = NULL;
ret = OK;
if (sem->semcount > 0)
{
/* It is, let the task take the semaphore */
sem->semcount--;
nxsem_add_holder(sem);
rtcb->waitobj = NULL;
ret = OK;
}
else
{
/* Semaphore is not available */
ret = -EAGAIN;
}
/* Interrupts may now be enabled. */
leave_critical_section(flags);
} }
else else
{ {
ret = -EINVAL; /* Semaphore is not available */
ret = -EAGAIN;
} }
/* Interrupts may now be enabled. */
leave_critical_section(flags);
return ret; return ret;
} }
@ -138,6 +130,12 @@ int sem_trywait(FAR sem_t *sem)
{ {
int ret; int ret;
if (sem == NULL)
{
set_errno(EINVAL);
return ERROR;
}
/* Let nxsem_trywait do the real work */ /* Let nxsem_trywait do the real work */
ret = nxsem_trywait(sem); ret = nxsem_trywait(sem);

View File

@ -72,7 +72,7 @@ int nxsem_wait(FAR sem_t *sem)
{ {
FAR struct tcb_s *rtcb = this_task(); FAR struct tcb_s *rtcb = this_task();
irqstate_t flags; irqstate_t flags;
int ret = -EINVAL; int ret;
/* This API should not be called from interrupt handlers & idleloop */ /* This API should not be called from interrupt handlers & idleloop */
@ -88,108 +88,105 @@ int nxsem_wait(FAR sem_t *sem)
/* Make sure we were supplied with a valid semaphore. */ /* Make sure we were supplied with a valid semaphore. */
if (sem != NULL) /* Check if the lock is available */
if (sem->semcount > 0)
{ {
/* Check if the lock is available */ /* It is, let the task take the semaphore. */
if (sem->semcount > 0) sem->semcount--;
{ nxsem_add_holder(sem);
/* It is, let the task take the semaphore. */ rtcb->waitobj = NULL;
ret = OK;
}
sem->semcount--; /* The semaphore is NOT available, We will have to block the
nxsem_add_holder(sem); * current thread of execution.
rtcb->waitobj = NULL; */
ret = OK;
}
/* The semaphore is NOT available, We will have to block the else
* current thread of execution. {
/* First, verify that the task is not already waiting on a
* semaphore
*/ */
else DEBUGASSERT(rtcb->waitobj == NULL);
{
/* First, verify that the task is not already waiting on a
* semaphore
*/
DEBUGASSERT(rtcb->waitobj == NULL); /* Handle the POSIX semaphore (but don't set the owner yet) */
/* Handle the POSIX semaphore (but don't set the owner yet) */ sem->semcount--;
sem->semcount--; /* Save the waited on semaphore in the TCB */
/* Save the waited on semaphore in the TCB */ rtcb->waitobj = sem;
rtcb->waitobj = sem; /* If priority inheritance is enabled, then check the priority of
* the holder of the semaphore.
/* If priority inheritance is enabled, then check the priority of */
* the holder of the semaphore.
*/
#ifdef CONFIG_PRIORITY_INHERITANCE #ifdef CONFIG_PRIORITY_INHERITANCE
/* Disable context switching. The following operations must be /* Disable context switching. The following operations must be
* atomic with regard to the scheduler. * atomic with regard to the scheduler.
*/ */
sched_lock(); sched_lock();
/* Boost the priority of any threads holding a count on the /* Boost the priority of any threads holding a count on the
* semaphore. * semaphore.
*/ */
nxsem_boost_priority(sem); nxsem_boost_priority(sem);
#endif #endif
/* Set the errno value to zero (preserving the original errno) /* Set the errno value to zero (preserving the original errno)
* value). We reuse the per-thread errno to pass information * value). We reuse the per-thread errno to pass information
* between sem_waitirq() and this functions. * between sem_waitirq() and this functions.
*/ */
rtcb->errcode = OK; rtcb->errcode = OK;
/* Add the TCB to the prioritized semaphore wait queue, after /* Add the TCB to the prioritized semaphore wait queue, after
* checking this is not the idle task - descheduling that * checking this is not the idle task - descheduling that
* isn't going to end well. * isn't going to end well.
*/ */
DEBUGASSERT(!is_idle_task(rtcb)); DEBUGASSERT(!is_idle_task(rtcb));
up_block_task(rtcb, TSTATE_WAIT_SEM); up_block_task(rtcb, TSTATE_WAIT_SEM);
/* When we resume at this point, either (1) the semaphore has been /* When we resume at this point, either (1) the semaphore has been
* assigned to this thread of execution, or (2) the semaphore wait * assigned to this thread of execution, or (2) the semaphore wait
* has been interrupted by a signal or a timeout. We can detect * has been interrupted by a signal or a timeout. We can detect
* these latter cases be examining the per-thread errno value. * these latter cases be examining the per-thread errno value.
* *
* In the event that the semaphore wait was interrupted by a * In the event that the semaphore wait was interrupted by a
* signal or a timeout, certain semaphore clean-up operations have * signal or a timeout, certain semaphore clean-up operations have
* already been performed (see sem_waitirq.c). Specifically: * already been performed (see sem_waitirq.c). Specifically:
* *
* - nxsem_canceled() was called to restore the priority of all * - nxsem_canceled() was called to restore the priority of all
* threads that hold a reference to the semaphore, * threads that hold a reference to the semaphore,
* - The semaphore count was decremented, and * - The semaphore count was decremented, and
* - tcb->waitobj was nullifed. * - tcb->waitobj was nullifed.
* *
* It is necessary to do these things in sem_waitirq.c because a * It is necessary to do these things in sem_waitirq.c because a
* long time may elapse between the time that the signal was issued * long time may elapse between the time that the signal was issued
* and this thread is awakened and this leaves a door open to * and this thread is awakened and this leaves a door open to
* several race conditions. * several race conditions.
*/ */
/* Check if an error occurred while we were sleeping. Expected /* Check if an error occurred while we were sleeping. Expected
* errors include EINTR meaning that we were awakened by a signal * errors include EINTR meaning that we were awakened by a signal
* or ETIMEDOUT meaning that the timer expired for the case of * or ETIMEDOUT meaning that the timer expired for the case of
* sem_timedwait(). * sem_timedwait().
* *
* If we were not awakened by a signal or a timeout, then * If we were not awakened by a signal or a timeout, then
* nxsem_add_holder() was called by logic in sem_wait() fore this * nxsem_add_holder() was called by logic in sem_wait() fore this
* thread was restarted. * thread was restarted.
*/ */
ret = rtcb->errcode != OK ? -rtcb->errcode : OK; ret = rtcb->errcode != OK ? -rtcb->errcode : OK;
#ifdef CONFIG_PRIORITY_INHERITANCE #ifdef CONFIG_PRIORITY_INHERITANCE
sched_unlock(); sched_unlock();
#endif #endif
}
} }
leave_critical_section(flags); leave_critical_section(flags);
@ -254,6 +251,12 @@ int sem_wait(FAR sem_t *sem)
int errcode; int errcode;
int ret; int ret;
if (sem == NULL)
{
set_errno(EINVAL);
return ERROR;
}
/* sem_wait() is a cancellation point */ /* sem_wait() is a cancellation point */
if (enter_cancellation_point()) if (enter_cancellation_point())