jerpelea opened a new pull request, #20072:
URL: https://github.com/apache/nuttx/pull/20072
## Summary
Two defects in the CONFIG_TICKET_SPINLOCK paths of spinlock.h:
1. spin_trylock_notrace() passed &lock->owner as the "expected" pointer of
atomic_cmpxchg(). A failed compare-exchange writes the current value of the
target object back through that pointer, so a losing trylock stores lock->next
into lock->owner. owner then equals next, which is the unlocked state: a lock
still held by another CPU reports itself as free, spin_is_locked() returns
false and the lock can be taken again. Every later unlock keeps incrementing
owner past next, so the ticket of a real waiter never matches and the lock
stays locked forever. Keep the expected value in a local variable.
2. spin_unlock() was wrapped in #ifdef __SP_UNLOCK_FUNCTION, a macro that is
never defined anywhere in the tree. The function body was therefore dead code
and spin_unlock() always expanded to "do { *(l) = SP_UNLOCKED; } while (0)",
which zeroes both ticket counters instead of releasing one ticket with
atomic_fetch_add(&lock->owner, 1). That drops queued waiters, lets a newcomer
draw ticket 0 and enter the critical section, and also skips the
UP_DMB/UP_DSB/UP_SEV release barriers and the sched_note_spinlock_unlock()
note. Drop the dead #ifdef so spin_unlock() is always the function.
Both were reproduced on qemu-armv7a:smp (cortex-a7 x4) with
CONFIG_TICKET_SPINLOCK=y, where the compare-exchange lowers to native
ldrex/strex. This confirms the root cause is the C-level aliasing of the
expected pointer, not the atomic implementation.
Refs: https://github.com/apache/nuttx/issues/19808
## Impact
RELEASE
## Testing
CI
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
