linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
To: gshan <Gavin.Shan@alcatel-lucent.com>
Cc: linuxppc-dev@lists.ozlabs.org
Subject: Re: spin_lock_irqsave and multi-core
Date: Thu, 01 Apr 2010 15:45:25 +1100	[thread overview]
Message-ID: <1270097125.7101.110.camel@pasglop> (raw)
In-Reply-To: <4BB4109E.8020607@alcatel-lucent.com>

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.
> 
> 

      reply	other threads:[~2010-04-01  4:45 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-04-01  1:59 spin_lock_irqsave and multi-core gshan
2010-04-01  2:39 ` Benjamin Herrenschmidt
2010-04-01  2:41   ` gshan
2010-04-01  2:56     ` Benjamin Herrenschmidt
2010-04-01  3:18       ` gshan
2010-04-01  4:45         ` Benjamin Herrenschmidt [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1270097125.7101.110.camel@pasglop \
    --to=benh@kernel.crashing.org \
    --cc=Gavin.Shan@alcatel-lucent.com \
    --cc=linuxppc-dev@lists.ozlabs.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).