Skip to content

pthread_cond remove csection #49

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

Merged
merged 3 commits into from
Dec 24, 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
3 changes: 3 additions & 0 deletions include/pthread.h
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,7 @@ struct pthread_cond_s
{
sem_t sem;
clockid_t clockid;
uint16_t wait_count;
};

#ifndef __PTHREAD_COND_T_DEFINED
Expand Down Expand Up @@ -367,6 +368,8 @@ struct pthread_barrier_s
{
sem_t sem;
unsigned int count;
unsigned int wait_count;
mutex_t mutex;
};

#ifndef __PTHREAD_BARRIER_T_DEFINED
Expand Down
2 changes: 2 additions & 0 deletions libs/libc/pthread/pthread_barrierinit.c
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,8 @@ int pthread_barrier_init(FAR pthread_barrier_t *barrier,
{
sem_init(&barrier->sem, 0, 0);
barrier->count = count;
barrier->wait_count = 0;
nxmutex_init(&barrier->mutex);
}

return ret;
Expand Down
1 change: 1 addition & 0 deletions libs/libc/pthread/pthread_condinit.c
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ int pthread_cond_init(FAR pthread_cond_t *cond,
else
{
cond->clockid = attr ? attr->clockid : CLOCK_REALTIME;
cond->wait_count = 0;
}

sinfo("Returning %d\n", ret);
Expand Down
52 changes: 10 additions & 42 deletions sched/pthread/pthread_barrierwait.c
Original file line number Diff line number Diff line change
Expand Up @@ -82,67 +82,35 @@

int pthread_barrier_wait(FAR pthread_barrier_t *barrier)
{
irqstate_t flags;
int semcount;
int ret;

if (barrier == NULL)
{
return EINVAL;
}

/* Disable pre-emption throughout the following */

flags = enter_critical_section();

/* Find out how many threads are already waiting at the barrier */

ret = nxsem_get_value(&barrier->sem, &semcount);
if (ret != OK)
{
leave_critical_section(flags);
return -ret;
}

/* If the number of waiters would be equal to the count, then we are done */

if ((1 - semcount) >= (int)barrier->count)
nxmutex_lock(&barrier->mutex);

if ((barrier->wait_count + 1) >= barrier->count)
{
/* Free all of the waiting threads */

while (semcount < 0)
while (barrier->wait_count > 0)
{
barrier->wait_count--;
nxsem_post(&barrier->sem);
nxsem_get_value(&barrier->sem, &semcount);
}

/* Then return PTHREAD_BARRIER_SERIAL_THREAD to the final thread */

leave_critical_section(flags);
nxmutex_unlock(&barrier->mutex);

return PTHREAD_BARRIER_SERIAL_THREAD;
}

/* Otherwise, this thread must wait as well */

while ((ret = nxsem_wait(&barrier->sem)) != OK)
{
/* If the thread is awakened by a signal, just continue to wait */

if (ret != -EINTR)
{
/* If it is awakened by some other error, then there is a
* problem
*/

break;
}
}
barrier->wait_count++;

/* We will only get here when we are one of the N-1 threads that were
* waiting for the final thread at the barrier. We just need to return
* zero.
*/
nxmutex_unlock(&barrier->mutex);

leave_critical_section(flags);
return -ret;
return -nxsem_wait_uninterruptible(&barrier->sem);
}
37 changes: 15 additions & 22 deletions sched/pthread/pthread_condbroadcast.c
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,10 @@
*
* Description:
* A thread broadcast on a condition variable.
*
* pthread_cond_broadcast shall unblock all threads currently blocked on a
* specified condition variable cond. We need own the mutex that threads
* calling pthread_cond_wait or pthread_cond_timedwait have associated
* with the condition variable during their wait.
* Input Parameters:
* None
*
Expand All @@ -56,7 +59,6 @@
int pthread_cond_broadcast(FAR pthread_cond_t *cond)
{
int ret = OK;
int sval;

sinfo("cond=%p\n", cond);

Expand All @@ -73,31 +75,22 @@ int pthread_cond_broadcast(FAR pthread_cond_t *cond)

sched_lock();

/* Get the current value of the semaphore */
/* Loop until all of the waiting threads have been restarted. */

if (nxsem_get_value(&cond->sem, &sval) != OK)
{
ret = EINVAL;
}
else
while (cond->wait_count > 0)
{
/* Loop until all of the waiting threads have been restarted. */

while (sval < 0)
{
/* If the value is less than zero (meaning that one or more
* thread is waiting), then post the condition semaphore.
* Only the highest priority waiting thread will get to execute
*/
/* If the value is less than zero (meaning that one or more
* thread is waiting), then post the condition semaphore.
* Only the highest priority waiting thread will get to execute
*/

ret = -nxsem_post(&cond->sem);
ret = -nxsem_post(&cond->sem);

/* Increment the semaphore count (as was done by the
* above post).
*/
/* Increment the semaphore count (as was done by the
* above post).
*/

sval++;
}
cond->wait_count--;
}

/* Now we can let the restarted threads run */
Expand Down
22 changes: 1 addition & 21 deletions sched/pthread/pthread_condclockwait.c
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ int pthread_cond_clockwait(FAR pthread_cond_t *cond,
clockid_t clockid,
FAR const struct timespec *abstime)
{
irqstate_t flags;
int ret = OK;
int status;

Expand Down Expand Up @@ -114,14 +113,7 @@ int pthread_cond_clockwait(FAR pthread_cond_t *cond,

sinfo("Give up mutex...\n");

/* We must disable pre-emption and interrupts here so that
* the time stays valid until the wait begins. This adds
* complexity because we assure that interrupts and
* pre-emption are re-enabled correctly.
*/

sched_lock();
flags = enter_critical_section();
cond->wait_count++;

/* Give up the mutex */

Expand All @@ -136,12 +128,6 @@ int pthread_cond_clockwait(FAR pthread_cond_t *cond,
}
}

/* Restore interrupts (pre-emption will be enabled
* when we fall through the if/then/else)
*/

leave_critical_section(flags);

/* Reacquire the mutex (retaining the ret). */

sinfo("Re-locking...\n");
Expand All @@ -151,12 +137,6 @@ int pthread_cond_clockwait(FAR pthread_cond_t *cond,
{
ret = status;
}

/* Re-enable pre-emption (It is expected that interrupts
* have already been re-enabled in the above logic)
*/

sched_unlock();
}

leave_cancellation_point();
Expand Down
38 changes: 8 additions & 30 deletions sched/pthread/pthread_condsignal.c
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@
*
* Description:
* A thread can signal on a condition variable.
* pthread_cond_signal shall unblock a thread currently blocked on a
* specified condition variable cond. We need own the mutex that threads
* calling pthread_cond_wait or pthread_cond_timedwait have associated
* with the condition variable during their wait.
*
* Input Parameters:
* None
Expand All @@ -55,7 +59,6 @@
int pthread_cond_signal(FAR pthread_cond_t *cond)
{
int ret = OK;
int sval;

sinfo("cond=%p\n", cond);

Expand All @@ -65,36 +68,11 @@ int pthread_cond_signal(FAR pthread_cond_t *cond)
}
else
{
/* Get the current value of the semaphore */

if (nxsem_get_value(&cond->sem, &sval) != OK)
{
ret = EINVAL;
}

/* If the value is less than zero (meaning that one or more
* thread is waiting), then post the condition semaphore.
* Only the highest priority waiting thread will get to execute
*/

else
if (cond->wait_count > 0)
{
/* One of my objectives in this design was to make
* pthread_cond_signal() usable from interrupt handlers. However,
* from interrupt handlers, you cannot take the associated mutex
* before signaling the condition. As a result, I think that
* there could be a race condition with the following logic which
* assumes that the if sval < 0 then the thread is waiting.
* Without the mutex, there is no atomic, protected operation that
* will guarantee this to be so.
*/

sinfo("sval=%d\n", sval);
if (sval < 0)
{
sinfo("Signalling...\n");
ret = -nxsem_post(&cond->sem);
}
sinfo("Signalling...\n");
cond->wait_count--;
ret = -nxsem_post(&cond->sem);
}
}

Expand Down
11 changes: 1 addition & 10 deletions sched/pthread/pthread_condwait.c
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ int pthread_cond_wait(FAR pthread_cond_t *cond, FAR pthread_mutex_t *mutex)
{
int status;
int ret;
irqstate_t flags;

sinfo("cond=%p mutex=%p\n", cond, mutex);

Expand Down Expand Up @@ -89,14 +88,9 @@ int pthread_cond_wait(FAR pthread_cond_t *cond, FAR pthread_mutex_t *mutex)

sinfo("Give up mutex / take cond\n");

flags = enter_critical_section();
sched_lock();
cond->wait_count++;
ret = pthread_mutex_breaklock(mutex, &nlocks);

/* Take the semaphore. This may be awakened only be a signal (EINTR)
* or if the thread is canceled (ECANCELED)
*/

status = -nxsem_wait_uninterruptible(&cond->sem);
if (ret == OK)
{
Expand All @@ -105,9 +99,6 @@ int pthread_cond_wait(FAR pthread_cond_t *cond, FAR pthread_mutex_t *mutex)
ret = status;
}

sched_unlock();
leave_critical_section(flags);

/* Reacquire the mutex.
*
* When cancellation points are enabled, we need to hold the mutex
Expand Down
Loading