Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "sem: change sem wait to atomic operation" #14804

Merged
merged 1 commit into from
Nov 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 2 additions & 12 deletions sched/semaphore/sem_destroy.c
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,6 @@

int nxsem_destroy(FAR sem_t *sem)
{
short old;

DEBUGASSERT(sem != NULL);

/* There is really no particular action that we need
Expand All @@ -74,18 +72,10 @@ int nxsem_destroy(FAR sem_t *sem)
* leave the count unchanged but still return OK.
*/

do
if (sem->semcount >= 0)
{
old = atomic_load(NXSEM_COUNT(sem));
if (old < 0)
{
break;
}
sem->semcount = 1;
}
while (!atomic_compare_exchange_weak_explicit(NXSEM_COUNT(sem),
&old, 1,
memory_order_release,
memory_order_relaxed));

/* Release holders of the semaphore */

Expand Down
4 changes: 2 additions & 2 deletions sched/semaphore/sem_holder.c
Original file line number Diff line number Diff line change
Expand Up @@ -880,7 +880,7 @@ void nxsem_canceled(FAR struct tcb_s *stcb, FAR sem_t *sem)
{
/* Check our assumptions */

DEBUGASSERT(atomic_load(NXSEM_COUNT(sem)) <= 0);
DEBUGASSERT(sem->semcount <= 0);

/* Adjust the priority of every holder as necessary */

Expand Down Expand Up @@ -978,7 +978,7 @@ void nxsem_release_all(FAR struct tcb_s *htcb)
* that was taken by sem_wait() or sem_post().
*/

atomic_fetch_add(NXSEM_COUNT(sem), 1);
sem->semcount++;
}
}

Expand Down
72 changes: 9 additions & 63 deletions sched/semaphore/sem_post.c
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,11 @@
#include "semaphore/semaphore.h"

/****************************************************************************
* Private Functions
* Public Functions
****************************************************************************/

/****************************************************************************
* Name: nxsem_post_slow
* Name: nxsem_post
*
* Description:
* When a kernel thread has finished with a semaphore, it will call
Expand Down Expand Up @@ -69,7 +69,7 @@
*
****************************************************************************/

static int nxsem_post_slow(FAR sem_t *sem)
int nxsem_post(FAR sem_t *sem)
{
FAR struct tcb_s *stcb = NULL;
irqstate_t flags;
Expand All @@ -78,20 +78,21 @@ static int nxsem_post_slow(FAR sem_t *sem)
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.
*/

flags = enter_critical_section();

sem_count = atomic_fetch_add(NXSEM_COUNT(sem), 1);
sem_count = sem->semcount;

/* Check the maximum allowable value */

if (sem_count >= SEM_VALUE_MAX)
{
atomic_fetch_sub(NXSEM_COUNT(sem), 1);
leave_critical_section(flags);
return -EOVERFLOW;
}
Expand All @@ -114,6 +115,8 @@ static int nxsem_post_slow(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
Expand All @@ -135,7 +138,7 @@ static int nxsem_post_slow(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
Expand Down Expand Up @@ -214,60 +217,3 @@ static int nxsem_post_slow(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)
{
short old = 0;
if (atomic_compare_exchange_weak_explicit(NXSEM_COUNT(sem), &old, 1,
memory_order_release,
memory_order_relaxed))
{
return OK;
}
}
#endif

return nxsem_post_slow(sem);
}
4 changes: 2 additions & 2 deletions sched/semaphore/sem_recover.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 && atomic_load(NXSEM_COUNT(sem)) < 0);
DEBUGASSERT(sem != NULL && sem->semcount < 0);

/* Restore the correct priority of all threads that hold references
* to this semaphore.
Expand All @@ -99,7 +99,7 @@ void nxsem_recover(FAR struct tcb_s *tcb)
* place.
*/

atomic_fetch_add(NXSEM_COUNT(sem), 1);
sem->semcount++;
}

/* Release all semphore holders for the task */
Expand Down
15 changes: 3 additions & 12 deletions sched/semaphore/sem_reset.c
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@
int nxsem_reset(FAR sem_t *sem, int16_t count)
{
irqstate_t flags;
short semcount;

DEBUGASSERT(sem != NULL && count >= 0);

Expand All @@ -81,7 +80,7 @@ int nxsem_reset(FAR sem_t *sem, int16_t count)
* out counts to any waiting threads.
*/

while (atomic_load(NXSEM_COUNT(sem)) < 0 && count > 0)
while (sem->semcount < 0 && count > 0)
{
/* Give out one counting, waking up one of the waiting threads
* and, perhaps, kicking off a lot of priority inheritance
Expand All @@ -99,18 +98,10 @@ int nxsem_reset(FAR sem_t *sem, int16_t count)
* value of sem->semcount is already correct in this case.
*/

do
if (sem->semcount >= 0)
{
semcount = atomic_load(NXSEM_COUNT(sem));
if (semcount < 0)
{
break;
}
sem->semcount = count;
}
while (!atomic_compare_exchange_weak_explicit(NXSEM_COUNT(sem),
&semcount, count,
memory_order_release,
memory_order_relaxed));

/* Allow any pending context switches to occur now */

Expand Down
115 changes: 31 additions & 84 deletions sched/semaphore/sem_trywait.c
Original file line number Diff line number Diff line change
Expand Up @@ -38,78 +38,6 @@
#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;
short 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 */

do
{
semcount = atomic_load(NXSEM_COUNT(sem));
if (semcount <= 0)
{
leave_critical_section(flags);
return -EAGAIN;
}
}
while (!atomic_compare_exchange_weak_explicit(NXSEM_COUNT(sem),
&semcount, semcount - 1,
memory_order_acquire,
memory_order_relaxed));

/* 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;
ret = OK;

/* Interrupts may now be enabled. */

leave_critical_section(flags);
return ret;
}

/****************************************************************************
* Public Functions
****************************************************************************/
Expand Down Expand Up @@ -140,30 +68,49 @@ static int nxsem_trywait_slow(FAR sem_t *sem)

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());

/* If this is a mutex, we can try to get the mutex in fast mode,
* else try to get it in slow mode.
/* The following operations must be performed with interrupts disabled
* because sem_post() may be called from an interrupt handler.
*/

#if !defined(CONFIG_PRIORITY_INHERITANCE) && !defined(CONFIG_PRIORITY_PROTECT)
if (sem->flags & SEM_TYPE_MUTEX)
flags = enter_critical_section();

/* If the semaphore is available, give it to the requesting task */

if (sem->semcount > 0)
{
short old = 1;
if (atomic_compare_exchange_weak_explicit(NXSEM_COUNT(sem), &old, 0,
memory_order_acquire,
memory_order_relaxed))
/* It is, let the task take the semaphore */

ret = nxsem_protect_wait(sem);
if (ret < 0)
{
return OK;
leave_critical_section(flags);
return ret;
}

return -EAGAIN;
sem->semcount--;
nxsem_add_holder(sem);
rtcb->waitobj = NULL;
ret = OK;
}
else
{
/* Semaphore is not available */

ret = -EAGAIN;
}
#endif

return nxsem_trywait_slow(sem);
/* Interrupts may now be enabled. */

leave_critical_section(flags);
return ret;
}
Loading
Loading