I reverted the relevant changes.
https://github.com/apache/nuttx/pull/15767 At 2025-02-05 13:06:29, "chao an" <magicd...@gmail.com> wrote: >Hi, > >The behavior of spin_lock needs everyone's advice > >After PR14578 <https://github.com/apache/nuttx/pull/14578> was merged into >the NuttX, the behavior of spin_lock() and spin_lock_irqsave() added the >feature of disable the preemption: > >https://github.com/apache/nuttx/pull/14578 > >The spin lock behavior changed from: > > >*spin_lock: spin lockspin_lock_irqsave: spin lock + irqsave* > >to: > > >*spin_lock: spin lock + sched_lockspin_lock_irqsave: spin lock + irqsave + >sched_lock* > >This change has two key issues >1. *Crash*: Since spin_lock depends on sched_lock, the code using this type >of API needs to re-evaluate the scope of impact, especially when tcb is not >initialized at the startup stage, sched_lock will cause crash > >https://github.com/apache/nuttx/pull/15728 > >2. *Performance degradation*: spin_lock/spin_lock_irqsave is widely used in >the kernel, and more than 90% of the code protected by spin_lock will not >cause context switching, so after the introduction of sched_lock, the >kernel part has introduced new performance overhead > >https://github.com/apache/nuttx/pull/15684 > > >Due to the change in API semantics, we need to further optimize all the >locations in the kernel that use spin_lock()/spin_lock_irqsave() to solve >performance issue in evolution. >In PR15705 <https://github.com/apache/nuttx/pull/15705>, Xiaoxiang had some >arguments about API naming with me. > >https://github.com/apache/nuttx/pull/15705 > >My point of view is to restore the original behavior of spin_lock() and >spin_lock_irqsave(), which brings the following benefits: >1. The API semantics remain unchanged, and independent developers and >projects that use these APIs outside the nuttx repository can keep their >code without any adjustment >2. The API naming is consistent with the internal implementation, and the >caller can know what is happening inside the function from the API naming > >*Option 1:* > > > >*spin_lock: spin lockspin_lock_nopreempt: >spin_lock + sched_lockspin_lock_irqsave: spin lock + irq >savespin_lock_irqsave_nopreempt: spin_lock + irq save + sched_lock* > >@xiaoxiang suggested changing the semantics of the API to hold sched_lock >by default, consistent with Linux: >1. This means that all locations in the kernel that call spin_lock and >spin_lock_irqsave need to be changed >spin_lock_irqsave -> spin_lock_irqsave_preempt >2. It is impossible to know what happens inside the function from the API >naming > >*Option 2:* > > > >*spin_lock: spin lock + sched_lockspin_lock_preempt: > spin_lockspin_lock_irqsave: spin lock + irq save + >sched_lockspin_lock_irqsave_preempt: spin_lock + irq save* > >I don't know which definition is better, or if any wise person has a better >choice, please give some advice.
