jlaitine opened a new pull request, #16197: URL: https://github.com/apache/nuttx/pull/16197
To correctly protect the counter value in SMP, the atomic_read needs to be inside the while loop. ## Summary This bug is the most probable cause of https://github.com/apache/nuttx/issues/16193 The SMP acquire-release loop is implemented wrong in semaphore code, since the "mutex fast path" was originally introduced. The atomic_read needs to be inside the do - while loop. Even though SMP acquire-release loop is a common pattern, here is a breakdown of why the current implementation fails. Note that even if the full C-code was posted below for clarity, the atomic instruction's access to memory order is the one that counts here. ``` Start with semaphore value == 0: CPU1 in sched/semaphore/sem_post.c: ``` sem_count = atomic_read(NXSEM_COUNT(sem)); -> sem_count = 0 do { if (sem_count >= SEM_VALUE_MAX) { leave_critical_section(flags); return -EOVERFLOW; } ``` CPU2 in libs/libc/semaphore/sem_post: ``` int32_t old = 0; if (atomic_try_cmpxchg_release(NXSEM_COUNT(sem), &old, 1)) -> success, semaphore value is now 1 ``` CPU1 in sched/semaphore/sem_post.c: ``` } while (!atomic_try_cmpxchg_release(NXSEM_COUNT(sem), &sem_count, sem_count + 1)); -> fail, try to xchg prev 0 to 1, but semaphore value is 1. semaphore value remains 1 ``` Outcoume: post happened twice, semaphore value changed from 0 to 1. ``` ## Impact Fixes broken semaphores for all SMP configurations, using mutexes with CONFIG_PRIORITY_INHERITANCE=y but the priority inheritance being disabled for the mutex (e.g. with pthread mutexes set to non priority inheritance with PTHREAD_MUTEX_DEFAULT_PRIO_NONE, or by setting the flag manually). ## Testing Not yet tested, except in qemu, hence draft -- 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
