xiaoxiang781216 commented on code in PR #14827:
URL: https://github.com/apache/nuttx/pull/14827#discussion_r1847499916
##########
arch/sim/src/sim/posix/sim_testset.c:
##########
@@ -58,7 +58,7 @@ uint8_t up_testset(volatile uint8_t *lock)
* following test and set is atomic.
*/
- return atomic_exchange((_Atomic uint8_t *)lock, 1);
+ return atomic_xchg((_Atomic uint8_t *)lock, 1);
Review Comment:
revert this file is compiled in host not target environment
##########
arch/sim/src/sim/sim_heap.c:
##########
@@ -488,17 +489,18 @@ void *mm_memalign(struct mm_heap_s *heap, size_t
alignment, size_t size)
sched_note_heap(NOTE_HEAP_ALLOC, heap, mem, size, 0);
atomic_fetch_add(&heap->aordblks, 1);
atomic_fetch_add(&heap->uordblks, size);
- usmblks = atomic_load(&heap->usmblks);
+ usmblks = atomic_read(&heap->usmblks);
do
{
- uordblks = atomic_load(&heap->uordblks);
+ uordblks = atomic_read(&heap->uordblks);
if (uordblks <= usmblks)
{
break;
}
}
- while (atomic_compare_exchange_weak(&heap->usmblks, &usmblks, uordblks));
+ while (atomic_try_cmpxchg(&heap->usmblks,
+ &usmblks, uordblks));
Review Comment:
ditto
##########
include/nuttx/spinlock.h:
##########
@@ -342,8 +340,8 @@ spin_trylock_wo_note(FAR volatile spinlock_t *lock)
}
};
- if (!atomic_compare_exchange_strong((FAR atomic_uint *)&lock->value,
- &oldval.value, newval.value))
+ if (!atomic_cmpxchg(&lock->value, &oldval.value,
+ newval.value))
Review Comment:
merge into one line
##########
sched/semaphore/sem_destroy.c:
##########
@@ -76,16 +76,14 @@ int nxsem_destroy(FAR sem_t *sem)
do
{
- old = atomic_load(NXSEM_COUNT(sem));
+ old = atomic_read(NXSEM_COUNT(sem));
if (old < 0)
Review Comment:
let's.remove NXSEM_COUNT
##########
sched/semaphore/sem_reset.c:
##########
@@ -101,16 +101,14 @@ int nxsem_reset(FAR sem_t *sem, int16_t count)
do
{
- semcount = atomic_load(NXSEM_COUNT(sem));
+ semcount = atomic_read(NXSEM_COUNT(sem));
if (semcount < 0)
{
break;
}
}
- while (!atomic_compare_exchange_weak_explicit(NXSEM_COUNT(sem),
- &semcount, count,
- memory_order_release,
- memory_order_relaxed));
+ while (!atomic_try_cmpxchg_release(NXSEM_COUNT(sem),
+ &semcount, count));
Review Comment:
ditto
##########
sched/semaphore/sem_trywait.c:
##########
@@ -78,17 +78,15 @@ static int nxsem_trywait_slow(FAR sem_t *sem)
do
{
- semcount = atomic_load(NXSEM_COUNT(sem));
+ semcount = atomic_read(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));
+ while (!atomic_try_cmpxchg_acquire(NXSEM_COUNT(sem),
+ &semcount, semcount - 1));
Review Comment:
ditto
##########
include/nuttx/spinlock.h:
##########
@@ -813,15 +811,14 @@ static inline_function void read_lock(FAR volatile
rwlock_t *lock)
{
while (true)
{
- int old = atomic_load((FAR atomic_int *)lock);
+ long old = atomic_read(lock);
Review Comment:
why change type
##########
arch/sim/src/sim/sim_heap.c:
##########
@@ -398,13 +398,14 @@ void *mm_realloc(struct mm_heap_s *heap, void *oldmem,
do
{
- uordblks = atomic_load(&heap->uordblks);
+ uordblks = atomic_read(&heap->uordblks);
if (uordblks <= usmblks)
{
break;
}
}
- while (atomic_compare_exchange_weak(&heap->usmblks, &usmblks, uordblks));
+ while (atomic_try_cmpxchg(&heap->usmblks,
+ &usmblks, uordblks));
Review Comment:
why split into two lines
##########
sched/semaphore/sem_post.c:
##########
@@ -259,10 +259,9 @@ int nxsem_post(FAR sem_t *sem)
#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))
+ int old = 0;
+ if (atomic_try_cmpxchg_release(NXSEM_COUNT(sem),
+ &old, 1))
Review Comment:
ditto
##########
include/nuttx/spinlock.h:
##########
@@ -68,8 +68,8 @@ union spinlock_u
{
struct
{
- unsigned short owner;
- unsigned short next;
+ atomic_t owner;
+ atomic_t next;
} tickets;
unsigned int value;
};
Review Comment:
can't simple change owner/next to 32bit, the algo aasume them 16bit
##########
include/nuttx/spinlock.h:
##########
@@ -858,14 +855,13 @@ static inline_function bool read_trylock(FAR volatile
rwlock_t *lock)
{
while (true)
{
- int old = atomic_load((FAR atomic_int *)lock);
+ long old = atomic_read(lock);
Review Comment:
ditto
##########
sched/semaphore/sem_wait.c:
##########
@@ -259,10 +259,9 @@ int nxsem_wait(FAR sem_t *sem)
#if !defined(CONFIG_PRIORITY_INHERITANCE) && !defined(CONFIG_PRIORITY_PROTECT)
if (sem->flags & SEM_TYPE_MUTEX)
{
- short old = 1;
- if (atomic_compare_exchange_weak_explicit(NXSEM_COUNT(sem), &old, 0,
- memory_order_acquire,
- memory_order_relaxed))
+ int old = 1;
+ if (atomic_try_cmpxchg_acquire(NXSEM_COUNT(sem),
+ &old, 0))
Review Comment:
ditto
##########
libs/libc/machine/arch_atomic.c:
##########
@@ -323,280 +323,240 @@
****************************************************************************/
STORE(__atomic_store_, 1, uint8_t)
-STORE(nx_atomic_store_, 1, uint8_t)
/****************************************************************************
* Name: __atomic_store_2
****************************************************************************/
STORE(__atomic_store_, 2, uint16_t)
-STORE(nx_atomic_store_, 2, uint16_t)
/****************************************************************************
* Name: __atomic_store_4
****************************************************************************/
STORE(__atomic_store_, 4, uint32_t)
-STORE(nx_atomic_store_, 4, uint32_t)
Review Comment:
need keep 4bytes nx_ version
##########
sched/semaphore/sem_destroy.c:
##########
@@ -76,16 +76,14 @@ int nxsem_destroy(FAR sem_t *sem)
do
{
- old = atomic_load(NXSEM_COUNT(sem));
+ old = atomic_read(NXSEM_COUNT(sem));
if (old < 0)
{
break;
}
}
- while (!atomic_compare_exchange_weak_explicit(NXSEM_COUNT(sem),
- &old, 1,
- memory_order_release,
- memory_order_relaxed));
+ while (!atomic_try_cmpxchg_release(NXSEM_COUNT(sem),
+ &old, 1));
Review Comment:
merge into one line?
##########
sched/semaphore/semaphore.h:
##########
@@ -40,7 +39,7 @@
* Pre-processor Definitions
****************************************************************************/
-#define NXSEM_COUNT(s) ((FAR atomic_short *)&(s)->semcount)
+#define NXSEM_COUNT(s) (&(s)->semcount)
Review Comment:
remove the macro or add atomic_read
##########
sched/semaphore/sem_trywait.c:
##########
@@ -153,10 +151,9 @@ int nxsem_trywait(FAR sem_t *sem)
#if !defined(CONFIG_PRIORITY_INHERITANCE) && !defined(CONFIG_PRIORITY_PROTECT)
if (sem->flags & SEM_TYPE_MUTEX)
{
- short old = 1;
- if (atomic_compare_exchange_weak_explicit(NXSEM_COUNT(sem), &old, 0,
- memory_order_acquire,
- memory_order_relaxed))
+ int old = 1;
+ if (atomic_try_cmpxchg_acquire(NXSEM_COUNT(sem),
+ &old, 0))
Review Comment:
ditto
--
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
