Fix-Point commented on code in PR #21342:
URL: https://github.com/apache/nuttx/pull/21342#discussion_r4442240760
##########
include/nuttx/spinlock.h:
##########
@@ -849,13 +923,18 @@ void spin_unlock_irqrestore_nopreempt(FAR volatile
spinlock_t *lock,
static inline_function
bool rspin_unlock(FAR rspinlock_t *lock)
{
- DEBUGASSERT(lock->owner == this_cpu() + 1);
- DEBUGASSERT(lock->count >= 1);
+ bool ret = false;
+ uint32_t val = lock->local_val;
- if (--lock->count == 0)
+ if (RSPINLOCK_COUNT(val) == 1u)
{
- atomic_set_release((FAR atomic_t *)&lock->val, 0);
- return true;
+ atomic_set_release(&lock->val, 0u);
+ UP_SEV();
+ ret = true;
+ }
+ else
+ {
+ lock->local_val = val - 1u;
}
return false;
Review Comment:
Sorry, this was an issue introduced while resolving a conflict; it has now
been restored.
##########
include/nuttx/spinlock.h:
##########
@@ -608,40 +662,60 @@ irqstate_t rspin_lock_irqsave_nopreempt(FAR rspinlock_t
*lock)
****************************************************************************/
#ifdef CONFIG_SPINLOCK
-static inline_function bool rspin_trylock(FAR rspinlock_t *lock)
+# ifdef CONFIG_SPINLOCK_INLINE
+SPINLOCK_INLINE bool rspin_trylock(FAR rspinlock_t *lock)
{
- rspinlock_t new_val;
- rspinlock_t old_val = RSPINLOCK_INITIALIZER;
- int cpu = this_cpu() + 1;
+ uint32_t old_val;
+ bool ret = false;
+ uint32_t owner = (uint32_t)this_cpu() + 1u;
/* Already owned this lock. */
- if (lock->owner == cpu)
+ old_val = atomic_read(&lock->val);
+
+ if (RSPINLOCK_OWNER(old_val) == owner)
{
- lock->count += 1;
- return true;
+ DEBUGASSERT(RSPINLOCK_COUNT(old_val) + 1u <= UINT16_MAX);
+
+ lock->local_val = old_val + 1u;
+ ret = true;
}
+ else if (old_val == 0u)
+ {
+ /* Try seize the ownership of the lock using CAS. */
+
+ ret = atomic_cmpxchg_acquire(&lock->val, &old_val,
+ RSPINLOCK_VALUE(owner, 1u));
- new_val.count = 1;
- new_val.owner = cpu;
+ /* Mark locked when the lock is successfully acquired. */
- /* Try seize the ownership of the lock. */
+ if (ret)
+ {
+ /* The local value shares the same memory location with the
+ * global atomic value. So if the atomic operation success,
+ * the local value should also change to the new_val.
+ */
- return atomic_cmpxchg_acquire((FAR atomic_t *)&lock->val,
- (FAR atomic_t *)&old_val.val, new_val.val);
+ compiler_assume(lock->local_val == RSPINLOCK_VALUE(owner, 1u));
+ }
+ }
}
Review Comment:
Sorry, this was an issue introduced while resolving a conflict; it has now
been restored.
##########
include/nuttx/spinlock.h:
##########
@@ -314,10 +334,11 @@ 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_SEV();
Review Comment:
Done.
--
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]
