xiaoxiang781216 commented on code in PR #15566:
URL: https://github.com/apache/nuttx/pull/15566#discussion_r1921099464
##########
sched/pthread/pthread_condwait.c:
##########
@@ -88,7 +89,7 @@ int pthread_cond_wait(FAR pthread_cond_t *cond, FAR
pthread_mutex_t *mutex)
sinfo("Give up mutex / take cond\n");
- cond->wait_count++;
+ atomic_fetch_add((FAR atomic_t *)&cond->wait_count, 1);
Review Comment:
let's add COND_WAIT_COUNT macro in sched/pthread/pthread.h like NXSEM_COUNT
to simplify the usage?
##########
libs/libc/pthread/pthread_conddestroy.c:
##########
@@ -74,16 +74,13 @@ int pthread_cond_destroy(FAR pthread_cond_t *cond)
{
ret = get_errno();
}
- else
+ else if (sval < 0)
{
- if (sval < 0)
- {
- ret = EBUSY;
- }
- else if (sem_destroy(&cond->sem) != OK)
- {
- ret = get_errno();
- }
+ ret = EBUSY;
+ }
+ else if (sem_destroy(&cond->sem) != OK)
Review Comment:
change to ret = -nxsem_destroy(&cond->sem)
##########
libs/libc/pthread/pthread_barrierdestroy.c:
##########
@@ -73,9 +73,9 @@ int pthread_barrier_destroy(FAR pthread_barrier_t *barrier)
else
{
ret = sem_getvalue(&barrier->sem, &semcount);
Review Comment:
change to nxsem_get_value? other sem_xxx need be replaced with nxsem_xxx too.
##########
libs/libc/pthread/pthread_conddestroy.c:
##########
@@ -72,7 +72,7 @@ int pthread_cond_destroy(FAR pthread_cond_t *cond)
ret = sem_getvalue(&cond->sem, &sval);
Review Comment:
change to nxsem_get_value?
##########
include/pthread.h:
##########
@@ -269,7 +269,7 @@ struct pthread_cond_s
{
sem_t sem;
clockid_t clockid;
- uint16_t wait_count;
+ unsigned int wait_count;
Review Comment:
OK, but let's change type to int
##########
sched/pthread/pthread_condbroadcast.c:
##########
@@ -68,9 +67,13 @@ int pthread_cond_broadcast(FAR pthread_cond_t *cond)
}
else
{
+ int wcnt = atomic_read((FAR atomic_t *)&cond->wait_count);
+
/* Loop until all of the waiting threads have been restarted. */
- while (cond->wait_count > 0)
+ while (wcnt && atomic_cmpxchg((FAR atomic_t *)&cond->wait_count,
Review Comment:
```
while (wcnt > 0)
{
if (atomic_cmpxchg((FAR atomic_t *)&cond->wait_count,
&wcnt, wcnt - 1))
{
/* If the value is less than zero (meaning that one or more
}
```
##########
sched/pthread/pthread_condsignal.c:
##########
@@ -68,10 +66,13 @@ int pthread_cond_signal(FAR pthread_cond_t *cond)
}
else
{
- if (cond->wait_count > 0)
+ int wcnt = atomic_read((FAR atomic_t *)&cond->wait_count);
+
+ if (wcnt && atomic_cmpxchg((FAR atomic_t *)&cond->wait_count,
Review Comment:
need while loop:
```
while (wcnt > 0)
{
if (atomic_cmpxchg((FAR atomic_t *)&cond->wait_count,
&wcnt, wcnt - 1))
{
sinfo("Signalling...\n");
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
