pthread_cond remove csection

reason:
We decouple semcount from business logic by using an independent counting variable,
which allows us to remove critical sections in many cases.

Signed-off-by: hujun5 <hujun5@xiaomi.com>
This commit is contained in:
hujun5
2024-10-21 17:29:19 +08:00
committed by Alan C. Assis
parent 7f84a64109
commit 2c0e5e872b
5 changed files with 8 additions and 61 deletions
+1
View File
@@ -269,6 +269,7 @@ struct pthread_cond_s
{ {
sem_t sem; sem_t sem;
clockid_t clockid; clockid_t clockid;
volatile int16_t lock_count;
}; };
#ifndef __PTHREAD_COND_T_DEFINED #ifndef __PTHREAD_COND_T_DEFINED
+1
View File
@@ -74,6 +74,7 @@ int pthread_cond_init(FAR pthread_cond_t *cond,
else else
{ {
cond->clockid = attr ? attr->clockid : CLOCK_REALTIME; cond->clockid = attr ? attr->clockid : CLOCK_REALTIME;
cond->lock_count = 0;
} }
sinfo("Returning %d\n", ret); sinfo("Returning %d\n", ret);
+1 -21
View File
@@ -75,7 +75,6 @@ int pthread_cond_clockwait(FAR pthread_cond_t *cond,
clockid_t clockid, clockid_t clockid,
FAR const struct timespec *abstime) FAR const struct timespec *abstime)
{ {
irqstate_t flags;
int ret = OK; int ret = OK;
int status; int status;
@@ -114,14 +113,7 @@ int pthread_cond_clockwait(FAR pthread_cond_t *cond,
sinfo("Give up mutex...\n"); sinfo("Give up mutex...\n");
/* We must disable pre-emption and interrupts here so that cond->lock_count--;
* 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();
/* Give up the mutex */ /* Give up the mutex */
@@ -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). */ /* Reacquire the mutex (retaining the ret). */
sinfo("Re-locking...\n"); sinfo("Re-locking...\n");
@@ -151,12 +137,6 @@ int pthread_cond_clockwait(FAR pthread_cond_t *cond,
{ {
ret = status; 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(); leave_cancellation_point();
+4 -30
View File
@@ -55,7 +55,6 @@
int pthread_cond_signal(FAR pthread_cond_t *cond) int pthread_cond_signal(FAR pthread_cond_t *cond)
{ {
int ret = OK; int ret = OK;
int sval;
sinfo("cond=%p\n", cond); sinfo("cond=%p\n", cond);
@@ -65,36 +64,11 @@ int pthread_cond_signal(FAR pthread_cond_t *cond)
} }
else else
{ {
/* Get the current value of the semaphore */ if (cond->lock_count < 0)
if (nxsem_get_value(&cond->sem, &sval) != OK)
{ {
ret = EINVAL; sinfo("Signalling...\n");
} cond->lock_count++;
ret = -nxsem_post(&cond->sem);
/* 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
{
/* 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);
}
} }
} }
+1 -10
View File
@@ -60,7 +60,6 @@ int pthread_cond_wait(FAR pthread_cond_t *cond, FAR pthread_mutex_t *mutex)
{ {
int status; int status;
int ret; int ret;
irqstate_t flags;
sinfo("cond=%p mutex=%p\n", cond, mutex); sinfo("cond=%p mutex=%p\n", cond, mutex);
@@ -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"); sinfo("Give up mutex / take cond\n");
flags = enter_critical_section(); cond->lock_count--;
sched_lock();
ret = pthread_mutex_breaklock(mutex, &nlocks); 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); status = -nxsem_wait_uninterruptible(&cond->sem);
if (ret == OK) if (ret == OK)
{ {
@@ -105,9 +99,6 @@ int pthread_cond_wait(FAR pthread_cond_t *cond, FAR pthread_mutex_t *mutex)
ret = status; ret = status;
} }
sched_unlock();
leave_critical_section(flags);
/* Reacquire the mutex. /* Reacquire the mutex.
* *
* When cancellation points are enabled, we need to hold the mutex * When cancellation points are enabled, we need to hold the mutex