Spinlock query

Spinlock query

[Rick Brown]

Hi,

In my driver (Process context), I have written the following code:

--------------------------------------------
spin_lock(lock)
...
//Critical section to manipulate driver data
...
spin_u lock(lock)
---------------------------------------------

I have written similar code in my interrupt handler (Interrupt
context). The driver data is not accessed from anywhere else. Is my
code safe from any potential concurrency issues? Is there a need to
use spin_lock_irqsave()? In both the places?

I intend to run the driver on SMP machine.

Regards,

Rick

Re: Spinlock query

[Rik van Riel]

Rick Brown wrote:
> Hi,
> 
> In my driver (Process context), I have written the following code:
> 
> --------------------------------------------
> spin_lock(lock)
> ...
> //Critical section to manipulate driver data

... interrupt hits here
     interrupt handler tries to grab the spinlock, which is already taken
     *BOOM*

> spin_u lock(lock)
> ---------------------------------------------
> 
> I have written similar code in my interrupt handler (Interrupt
> context). The driver data is not accessed from anywhere else. Is my
> code safe from any potential concurrency issues? Is there a need to
> use spin_lock_irqsave()? In both the places?

You need to use spin_lock_irqsave() from process context.
 From the interrupt handler itself it doesn't hurt, but it
shouldn't matter much since interrupt handlers should not
get preempted.

-- 
What is important?  What you want to be true, or what is true?

Re: Spinlock query

[Rajat Jain]

On 8/30/06, Rik van Riel <riel@surriel.com> wrote:
> Rick Brown wrote:
> > Hi,
> >
> > In my driver (Process context), I have written the following code:
> >
> > --------------------------------------------
> > spin_lock(lock)
> > ...
> > //Critical section to manipulate driver data
>
> ... interrupt hits here
>     interrupt handler tries to grab the spinlock, which is already taken
>     *BOOM*
>
> > spin_u lock(lock)
> > ---------------------------------------------
> >

The interrupt handler TRIES to grab the spinlock, which is already
taken. Why will it "BOOM"? Wouldn't the interrupt handler busy wait,
waiting for the lock?

Am I missing something here?

Rajat

> > I have written similar code in my interrupt handler (Interrupt
> > context). The driver data is not accessed from anywhere else. Is my
> > code safe from any potential concurrency issues? Is there a need to
> > use spin_lock_irqsave()? In both the places?
>
> You need to use spin_lock_irqsave() from process context.
>  From the interrupt handler itself it doesn't hurt, but it
> shouldn't matter much since interrupt handlers should not
> get preempted.

Re: Spinlock query

[Vin]

Hi,

On 8/30/06, Rajat Jain <rajat.noida.india@gmail.com> wrote:
> On 8/30/06, Rik van Riel <riel@surriel.com> wrote:
> > Rick Brown wrote:
> > > Hi,
> > >
> > > In my driver (Process context), I have written the following code:
> > >
> > > --------------------------------------------
> > > spin_lock(lock)
> > > ...
> > > //Critical section to manipulate driver data
> >
> > ... interrupt hits here
> >     interrupt handler tries to grab the spinlock, which is already taken
> >     *BOOM*
> >
> > > spin_u lock(lock)
> > > ---------------------------------------------
> > >
>
> The interrupt handler TRIES to grab the spinlock, which is already
> taken. Why will it "BOOM"? Wouldn't the interrupt handler busy wait,
> waiting for the lock?
>
If you take spin_lock in interrupt as well as in process context then-:
1. On UP non-preempt kernel: There will be no locks taken, so nothing
is gonna protect the shared data between the interrupt handler &
process context thread.
2. On the UP preempt kernel: The lock has been taken by the process
context, and now if the interrupts is issued, it will try to grab the
same lock, but it will never get it ... and nobody is going to leave
that lock. boom* - Deadlock.
3. On SMP machine non-preempt kernel: The locks will be taken but no
protection against the interrupt as such.
4. On SMP preempt kernel: process context threads are synchronised but
not the interrupts.

> > > I have written similar code in my interrupt handler (Interrupt
> > > context). The driver data is not accessed from anywhere else. Is my
> > > code safe from any potential concurrency issues? Is there a need to
> > > use spin_lock_irqsave()? In both the places?
> >
> > You need to use spin_lock_irqsave() from process context.
> >  From the interrupt handler itself it doesn't hurt, but it
> > shouldn't matter much since interrupt handlers should not
> > get preempted.

Yes, AFAIK the correct idea is to take spin_lock() in interrupt
handler (Here I consider that your interrupt is already disabled.)
And take spin_lock_irq() or spin_lock_irqsave() lock from the process
context.

--v

Re: Spinlock query

[Rik van Riel]

Rajat Jain wrote:
> On 8/30/06, Rik van Riel <riel@surriel.com> wrote:
>> Rick Brown wrote:
>> > Hi,
>> >
>> > In my driver (Process context), I have written the following code:
>> >
>> > --------------------------------------------
>> > spin_lock(lock)
>> > ...
>> > //Critical section to manipulate driver data
>>
>> ... interrupt hits here
>>     interrupt handler tries to grab the spinlock, which is already taken
>>     *BOOM*
>>
>> > spin_u lock(lock)
>> > ---------------------------------------------
>> >
> 
> The interrupt handler TRIES to grab the spinlock, which is already
> taken. Why will it "BOOM"? Wouldn't the interrupt handler busy wait,
> waiting for the lock?
> 
> Am I missing something here?

Yes, it will busy wait.  Forever.


-- 
What is important?  What you want to be true, or what is true?

Re: Spinlock query

[Rajat Jain]

>
> > You need to use spin_lock_irqsave() from process context.
> > From the interrupt handler itself it doesn't hurt, but it
> > shouldn't matter much since interrupt handlers should not
> > get preempted.
>
>
> but interrupt handlers run in interrupt context when interrupts are already
> disabled. Is that correct ?
>

AFAIK the interrupt that the handler is serving is guaranteed to be
disabled on all the processors.

In addition, if the interrupt was registered with SA_INTERRUPT flag,
all the interrupts will be disabled on the current processor.

Regards,

Rajat