spin_lock_irqsave and multi-core

spin_lock_irqsave and multi-core

[gshan]

I got a question when reading source code. Hope somebody can give me the 
answer.
On multi-core system, spin_lock_irqsave() can stop all CPU cores 
receiving interrupts?
If the answer is no, what we can do to disable external interrupt for 
all CPU cores?

Thanks,
Gavin

Re: spin_lock_irqsave and multi-core

[Benjamin Herrenschmidt]

On Thu, 2010-04-01 at 09:59 +0800, gshan wrote:
> On multi-core system, spin_lock_irqsave() can stop all CPU cores 
> receiving interrupts?

No.

> If the answer is no, what we can do to disable external interrupt for 
> all CPU cores?

You don't :-)

Really, you generally don't. Why would you want to do that ?

Cheers,
Ben.

Re: spin_lock_irqsave and multi-core

[gshan]

[-- Attachment #1: Type: text/html, Size: 1273 bytes --]

Re: spin_lock_irqsave and multi-core

[Benjamin Herrenschmidt]

On Thu, 2010-04-01 at 10:41 +0800, gshan wrote:

> One of my private driver need it. I don't know how to do it.

Then your driver is most certainly doing something wrong :-) No driver
needs that. Ever.

What is it trying to do more precisely ? I might be able to explain what
the right approach is if you can tell me what the driver is trying to
disable all system IRQs for ? Keep in mind that local_irq_save() does a
lot more than just masking external interrupts from your device. It
masks them from -all- devices, including timer interrupts, perfmon
interrupts, etc... Doing so on all cores is generally a big no-no and
almost never really necessary.

Cheers,
Ben.

Re: spin_lock_irqsave and multi-core

[gshan]

>
> Then your driver is most certainly doing something wrong :-) No driver
> needs that. Ever.
>
> What is it trying to do more precisely ? I might be able to explain what
> the right approach is if you can tell me what the driver is trying to
> disable all system IRQs for ? Keep in mind that local_irq_save() does a
> lot more than just masking external interrupts from your device. It
> masks them from -all- devices, including timer interrupts, perfmon
> interrupts, etc... Doing so on all cores is generally a big no-no and
> almost never really necessary.
>   
I agree with you. Actually, I can disable the individual interrupt via 
disable_irq_sync() to make sure
system cooherence. It's just my concern when reading kernel source. 
Anyway, thanks a lot
for your kindly answer.

Thanks,
Gavin
> Cheers,
> Ben.
>
>
>   

Re: spin_lock_irqsave and multi-core

[Benjamin Herrenschmidt]

On Thu, 2010-04-01 at 11:18 +0800, gshan wrote:
> I agree with you. Actually, I can disable the individual interrupt via
> disable_irq_sync() to make sure
> system cooherence. 

This is also a very heavy hammer and not recommended at all. In most
case, you don't need that either. Also don't forget that if your system
has shared interrupts, you'll also disable whoever else interrupt you
are sharing with. You also cannot call that within a spinlock region for
example.

You don't normally need any of that if you use spinlocks properly, but
again, without more understanding of what your driver is trying to do,
it's hard to explain properly what you should do.

Normally tho, you synchronize your interrupt handler with your other
driver parts using simply a spinlock. You take it with spin_lock/unlock
from the interrupt handler and you use spin_lock_irqsave/restore from
the non-irq path. You don't normally need more. You only need the
irqsave/restore in fact on one CPU because it's goal is purely to avoid
a spin deadlock if your interrupt happens on that same CPU at the same
time.

Only if you want to disable interrupts for an extended period of time
should you consider using disable_irq_* and even then, as I mentioned,
it has drawbacks and is generally not recommended.

Alternatively, most devices have the ability to disable interrupt
emission on the device itself using some kind of MMIO register (though
you still need to synchronize things properly in your driver as the
interrupt might have already been emitted and on its way to the
processor when you whack that register.

But again, without more detailed knowledge of what you driver is
actually doing, it's hard to give you more precise advice.

Cheers,
Ben.

> It's just my concern when reading kernel source. 
> Anyway, thanks a lot
> for your kindly answer.
> 
>