You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "hujun260 (via GitHub)" <gi...@apache.org> on 2026/01/20 12:47:42 UTC
[PR] spinlock: add rspinlock interface for recursive spinlock support [nuttx]
hujun260 opened a new pull request, #18049:
URL: https://github.com/apache/nuttx/pull/18049
## Summary
This PR adds comprehensive recursive spinlock (rspinlock) interface functions to support nested spinlock acquisitions. The implementation includes separate lock/unlock operations and helper functions for breaking and restoring locks. This enables proper handling of scenarios where the same CPU needs to acquire a spinlock multiple times without causing deadlocks.
### Changes Made
- Add rspin_lock() function for acquiring recursive spinlock without IRQ handling
- Refactor rspin_lock_irqsave() to use new rspin_lock() internally
- Add rspin_unlock() function with return value indicating lock release
- Refactor rspin_unlock_irqrestore() to use new rspin_unlock()
- Add rspin_breaklock() to temporarily release nested locks
- Add rspin_restorelock() to restore previously broken locks
- Update documentation comments for new function return values
### Impact
• Functionality: Enables recursive spinlock patterns for same-CPU nested locking
• Code Quality: Separates concerns between lock management and IRQ handling
• Flexibility: Provides utility functions for lock breaking and restoration
• Compatibility: Maintains backward compatibility with existing rspin_lock_irqsave/irqrestore
### Testing
Test Environment:
• Host: Linux x86_64
• Board: sim (simulated multi-core environment)
• Configuration: NuttX with spinlock support enabled
Test Procedure:
1. Tested rspin_lock() for single acquisition
2. Tested rspin_lock() for nested acquisition by same CPU
3. Verified lock count increments correctly on nested lock
4. Tested rspin_unlock() return value (true on final unlock, false otherwise)
5. Tested rspin_lock_irqsave() still functions with new implementation
6. Tested rspin_unlock_irqrestore() with new implementation
7. Tested rspin_breaklock() and rspin_restorelock() pair
8. Verified lock state preservation across break/restore operations
9. Stress tested with multiple nested lock levels
Test Results:
Recursive Spinlock Test:
- First lock acquisition: count=1 ✅
- Second lock acquisition (same CPU): count=2 ✅
- Third lock acquisition (same CPU): count=3 ✅
- First unlock (count>1): returns false, count=2 ✅
- Second unlock (count>1): returns false, count=1 ✅
- Final unlock (count==0): returns true, lock released ✅
Break/Restore Test:
- Original count: 3
- After breaklock(): count=1, oldcount saved
- After restorelock(): count=3, restored correctly ✅
IRQ Integration:
- rspin_lock_irqsave() still properly saves/restores IRQ state ✅
- Nested acquisition works with IRQ disabled ✅
Verification:
• ✅ rspin_lock() correctly handles recursive acquisition
• ✅ Lock count properly managed for nested calls
• ✅ rspin_unlock() returns correct values
• ✅ IRQ state properly saved and restored
• ✅ Breaklock/restorelock functionality working
• ✅ No deadlocks in recursive scenarios
• ✅ Backward compatibility maintained
• ✅ OSTest passed without regressions
### Related Issues
Enables recursive spinlock support for NuttX synchronization primitives.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] spinlock: add and optimize recursive spinlock interface for multi-core support [nuttx]
Posted by "GUIDINGLI (via GitHub)" <gi...@apache.org>.
GUIDINGLI merged PR #18049:
URL: https://github.com/apache/nuttx/pull/18049
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] spinlock: add and optimize recursive spinlock interface for multi-core support [nuttx]
Posted by "anchao (via GitHub)" <gi...@apache.org>.
anchao commented on code in PR #18049:
URL: https://github.com/apache/nuttx/pull/18049#discussion_r2715971983
##########
include/nuttx/spinlock.h:
##########
@@ -548,14 +568,22 @@ irqstate_t spin_lock_irqsave_nopreempt(FAR volatile spinlock_t *lock)
*
****************************************************************************/
+#ifdef CONFIG_SPINLOCK
static inline_function
-irqstate_t rspin_lock_irqsave(FAR rspinlock_t *lock)
+void rspin_lock(FAR rspinlock_t *lock)
{
rspinlock_t new_val;
rspinlock_t old_val = RSPINLOCK_INITIALIZER;
- irqstate_t flags = up_irq_save();
int cpu = this_cpu() + 1;
+ /* Already owned this lock. */
+
+ if (lock->owner == cpu)
Review Comment:
emm ... Why was this PR merged? I clearly stated that this change would break the standalone usage of rspin_lock().
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] spinlock: add and optimize recursive spinlock interface for multi-core support [nuttx]
Posted by "hujun260 (via GitHub)" <gi...@apache.org>.
hujun260 commented on code in PR #18049:
URL: https://github.com/apache/nuttx/pull/18049#discussion_r2716229025
##########
include/nuttx/spinlock.h:
##########
@@ -548,14 +568,22 @@ irqstate_t spin_lock_irqsave_nopreempt(FAR volatile spinlock_t *lock)
*
****************************************************************************/
+#ifdef CONFIG_SPINLOCK
static inline_function
-irqstate_t rspin_lock_irqsave(FAR rspinlock_t *lock)
+void rspin_lock(FAR rspinlock_t *lock)
{
rspinlock_t new_val;
rspinlock_t old_val = RSPINLOCK_INITIALIZER;
- irqstate_t flags = up_irq_save();
int cpu = this_cpu() + 1;
+ /* Already owned this lock. */
+
+ if (lock->owner == cpu)
Review Comment:
<img width="504" height="350" alt="image" src="https://github.com/user-attachments/assets/f5931c74-4b5b-4755-bc01-0a7468c67bf3" />
atomic_set_release((FAR atomic_t *)&lock->val, 0);
This statement sets the recursive lock's owner to 0, indicating no owner.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] spinlock: add and optimize recursive spinlock interface for multi-core support [nuttx]
Posted by "anchao (via GitHub)" <gi...@apache.org>.
anchao commented on code in PR #18049:
URL: https://github.com/apache/nuttx/pull/18049#discussion_r2709146041
##########
include/nuttx/spinlock.h:
##########
@@ -548,14 +568,22 @@ irqstate_t spin_lock_irqsave_nopreempt(FAR volatile spinlock_t *lock)
*
****************************************************************************/
+#ifdef CONFIG_SPINLOCK
static inline_function
-irqstate_t rspin_lock_irqsave(FAR rspinlock_t *lock)
+void rspin_lock(FAR rspinlock_t *lock)
{
rspinlock_t new_val;
rspinlock_t old_val = RSPINLOCK_INITIALIZER;
- irqstate_t flags = up_irq_save();
int cpu = this_cpu() + 1;
+ /* Already owned this lock. */
+
+ if (lock->owner == cpu)
Review Comment:
race condition after get cpu
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] spinlock: add and optimize recursive spinlock interface for multi-core support [nuttx]
Posted by "hujun260 (via GitHub)" <gi...@apache.org>.
hujun260 commented on code in PR #18049:
URL: https://github.com/apache/nuttx/pull/18049#discussion_r2711040395
##########
include/nuttx/spinlock.h:
##########
@@ -548,14 +568,22 @@ irqstate_t spin_lock_irqsave_nopreempt(FAR volatile spinlock_t *lock)
*
****************************************************************************/
+#ifdef CONFIG_SPINLOCK
static inline_function
-irqstate_t rspin_lock_irqsave(FAR rspinlock_t *lock)
+void rspin_lock(FAR rspinlock_t *lock)
{
rspinlock_t new_val;
rspinlock_t old_val = RSPINLOCK_INITIALIZER;
- irqstate_t flags = up_irq_save();
int cpu = this_cpu() + 1;
+ /* Already owned this lock. */
+
+ if (lock->owner == cpu)
Review Comment:
Yes, rspin_lock cannot be used in interrupt-enabled contexts, just like the spin_lock function.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] spinlock: add and optimize recursive spinlock interface for multi-core support [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #18049:
URL: https://github.com/apache/nuttx/pull/18049#discussion_r2711099625
##########
include/nuttx/spinlock.h:
##########
@@ -548,14 +568,22 @@ irqstate_t spin_lock_irqsave_nopreempt(FAR volatile spinlock_t *lock)
*
****************************************************************************/
+#ifdef CONFIG_SPINLOCK
static inline_function
-irqstate_t rspin_lock_irqsave(FAR rspinlock_t *lock)
+void rspin_lock(FAR rspinlock_t *lock)
{
rspinlock_t new_val;
rspinlock_t old_val = RSPINLOCK_INITIALIZER;
- irqstate_t flags = up_irq_save();
int cpu = this_cpu() + 1;
+ /* Already owned this lock. */
+
+ if (lock->owner == cpu)
Review Comment:
need call up_irq_save before.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] spinlock: add and optimize recursive spinlock interface for multi-core support [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #18049:
URL: https://github.com/apache/nuttx/pull/18049#discussion_r2711099625
##########
include/nuttx/spinlock.h:
##########
@@ -548,14 +568,22 @@ irqstate_t spin_lock_irqsave_nopreempt(FAR volatile spinlock_t *lock)
*
****************************************************************************/
+#ifdef CONFIG_SPINLOCK
static inline_function
-irqstate_t rspin_lock_irqsave(FAR rspinlock_t *lock)
+void rspin_lock(FAR rspinlock_t *lock)
{
rspinlock_t new_val;
rspinlock_t old_val = RSPINLOCK_INITIALIZER;
- irqstate_t flags = up_irq_save();
int cpu = this_cpu() + 1;
+ /* Already owned this lock. */
+
+ if (lock->owner == cpu)
Review Comment:
need call up_irq_save before, it's the same requirement of spin_lock.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] spinlock: add and optimize recursive spinlock interface for multi-core support [nuttx]
Posted by "anchao (via GitHub)" <gi...@apache.org>.
anchao commented on code in PR #18049:
URL: https://github.com/apache/nuttx/pull/18049#discussion_r2716108787
##########
include/nuttx/spinlock.h:
##########
@@ -548,14 +568,22 @@ irqstate_t spin_lock_irqsave_nopreempt(FAR volatile spinlock_t *lock)
*
****************************************************************************/
+#ifdef CONFIG_SPINLOCK
static inline_function
-irqstate_t rspin_lock_irqsave(FAR rspinlock_t *lock)
+void rspin_lock(FAR rspinlock_t *lock)
{
rspinlock_t new_val;
rspinlock_t old_val = RSPINLOCK_INITIALIZER;
- irqstate_t flags = up_irq_save();
int cpu = this_cpu() + 1;
+ /* Already owned this lock. */
+
+ if (lock->owner == cpu)
Review Comment:
and this PR also has a critical bug — the owner isn't restored in rspin_unlock().
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] spinlock: add and optimize recursive spinlock interface for multi-core support [nuttx]
Posted by "hujun260 (via GitHub)" <gi...@apache.org>.
hujun260 commented on code in PR #18049:
URL: https://github.com/apache/nuttx/pull/18049#discussion_r2716229025
##########
include/nuttx/spinlock.h:
##########
@@ -548,14 +568,22 @@ irqstate_t spin_lock_irqsave_nopreempt(FAR volatile spinlock_t *lock)
*
****************************************************************************/
+#ifdef CONFIG_SPINLOCK
static inline_function
-irqstate_t rspin_lock_irqsave(FAR rspinlock_t *lock)
+void rspin_lock(FAR rspinlock_t *lock)
{
rspinlock_t new_val;
rspinlock_t old_val = RSPINLOCK_INITIALIZER;
- irqstate_t flags = up_irq_save();
int cpu = this_cpu() + 1;
+ /* Already owned this lock. */
+
+ if (lock->owner == cpu)
Review Comment:
<img width="504" height="350" alt="image" src="https://github.com/user-attachments/assets/f5931c74-4b5b-4755-bc01-0a7468c67bf3" />
atomic_set_release((FAR atomic_t *)&lock->val, 0);
This statement sets the recursive lock's owner to 0, indicating no owner.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] spinlock: add and optimize recursive spinlock interface for multi-core support [nuttx]
Posted by "hujun260 (via GitHub)" <gi...@apache.org>.
hujun260 commented on code in PR #18049:
URL: https://github.com/apache/nuttx/pull/18049#discussion_r2716010397
##########
include/nuttx/spinlock.h:
##########
@@ -548,14 +568,22 @@ irqstate_t spin_lock_irqsave_nopreempt(FAR volatile spinlock_t *lock)
*
****************************************************************************/
+#ifdef CONFIG_SPINLOCK
static inline_function
-irqstate_t rspin_lock_irqsave(FAR rspinlock_t *lock)
+void rspin_lock(FAR rspinlock_t *lock)
{
rspinlock_t new_val;
rspinlock_t old_val = RSPINLOCK_INITIALIZER;
- irqstate_t flags = up_irq_save();
int cpu = this_cpu() + 1;
+ /* Already owned this lock. */
+
+ if (lock->owner == cpu)
Review Comment:
rspin_lock can be used independently, but only in scenarios where interrupts are masked.
I will submit a PR later to replace enter_critical_section with rspin_lock_irqsave,
which will demonstrate the independent usage of rspin_lock.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org