Fix-Point commented on code in PR #21342:
URL: https://github.com/apache/nuttx/pull/21342#discussion_r4450247494
##########
include/nuttx/spinlock.h:
##########
@@ -314,10 +334,12 @@ static inline_function void
spin_unlock_notrace(FAR volatile spinlock_t *lock)
{
#ifdef CONFIG_TICKET_SPINLOCK
- atomic_add_release(&lock->owner, 1);
+ atomic_set_release(&lock->owner, lock->owner + 1);
#else
atomic_set_release((FAR atomic_t *)lock, SP_UNLOCKED);
#endif
+ UP_DSB();
Review Comment:
Thanks for the suggestions. The unconditional UP_DSB() calls were removed
from the four unlock sites (`spin_unlock_notrace`, `rspin_unlock`,
`read_unlock`, `write_unlock`), and the `DSB` now lives inside the `UP_SEV()`
definition of the `ARM A/R` profiles (armv7-a/armv7-r/armv8-r barriers.h),
where it is architecturally required: `SEV` is not a memory access and is not
subject to memory ordering, so the release store must be made visible by a
`DSB` before the event is sent (ARM Reference Manual DDI 0406C, "Use of WFE and
SEV with Locks").
On `RISC-V`, `x86_64`, `Xtensa` and `sim` UP_SEV() is empty, so the unlock
path now emits no barrier at all. Note that `arm64` keeps a bare `SEV` without
`DSB`, matching the Linux `arm64` unlock pattern (release store + sev()).
--
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]
