2008-10-11 15:29:18

by Andrey Borzenkov

[permalink] [raw]
Subject: when spin_lock_irq (as opposed to spin_lock_irqsave) is appropriate?

Logically, one piece of kernel code has no way to know whether another
piece of kernel code (or may be hard-/firmware) has disabled some
interrupt line. So it looks like spin_lock_irq should not even exist,
except may be for very specific cases (where we are sure no other piece
of kernel code may run concurrently)?

Sorry for stupid question, I an not actually a HW type of person ...


Attachments:
(No filename) (387.00 B)
signature.asc (197.00 B)
This is a digitally signed message part.
Download all attachments

2008-10-11 15:43:30

by Oliver Neukum

[permalink] [raw]
Subject: Re: when spin_lock_irq (as opposed to spin_lock_irqsave) is appropriate?

Am Samstag, 11. Oktober 2008 17:29:01 schrieb Andrey Borzenkov:
> Logically, one piece of kernel code has no way to know whether another
> piece of kernel code (or may be hard-/firmware) has disabled some
> interrupt line. So it looks like spin_lock_irq should not even exist,
> except may be for very specific cases (where we are sure no other piece
> of kernel code may run concurrently)?
>
> Sorry for stupid question, I an not actually a HW type of person ...
>

This has no connection with individual irq lines. It's about being able
to sleep. Kernel code usually knows whether it can sleep.
If it knows to be able to sleep it can use spin_lock_irq() which is
more efficient than spin_lock_irqsave()

Regards
Oliver

2008-10-11 15:55:29

by Andrey Borzenkov

[permalink] [raw]
Subject: Re: when spin_lock_irq (as opposed to spin_lock_irqsave) is appropriate?

On Saturday 11 October 2008, Oliver Neukum wrote:
> Am Samstag, 11. Oktober 2008 17:29:01 schrieb Andrey Borzenkov:
> > Logically, one piece of kernel code has no way to know whether another
> > piece of kernel code (or may be hard-/firmware) has disabled some
> > interrupt line. So it looks like spin_lock_irq should not even exist,
> > except may be for very specific cases (where we are sure no other piece
> > of kernel code may run concurrently)?
> >
> > Sorry for stupid question, I an not actually a HW type of person ...
> >
>
> This has no connection with individual irq lines. It's about being able
> to sleep. Kernel code usually knows whether it can sleep.
> If it knows to be able to sleep it can use spin_lock_irq() which is
> more efficient than spin_lock_irqsave()
>

Sorry? I can't sleep under spinlock ... *any* spinlock? Or has this changed?

May I be I was not clear with question. spin_lock_irq implies spin_unlock_irq,
which unconditionally enables interrupts. But I have no idea which interrupts
were disabled before spin_lock_irq; so I may accidentally enable too much?

Or what exactly "irq" in spin_(un-)lock_irq means?

TIA

-andrey


Attachments:
(No filename) (1.14 kB)
signature.asc (197.00 B)
This is a digitally signed message part.
Download all attachments

2008-10-11 16:18:19

by Arjan van de Ven

[permalink] [raw]
Subject: Re: when spin_lock_irq (as opposed to spin_lock_irqsave) is appropriate?

On Sat, 11 Oct 2008 19:29:01 +0400
Andrey Borzenkov <[email protected]> wrote:

> Logically, one piece of kernel code has no way to know whether another
> piece of kernel code (or may be hard-/firmware) has disabled some
> interrupt line. So it looks like spin_lock_irq should not even exist,
> except may be for very specific cases (where we are sure no other
> piece of kernel code may run concurrently)?
>
> Sorry for stupid question, I an not actually a HW type of person ...

Hi,

_irq versus _irqsave has nothing to do with hardware, and everything
with the code design.
_irqsave is a little more expensive than _irq, so for really high
performance critical pieces of code, if you know it's ok (more on that
below), it's nicer to use _irq than _irqsave.

Now.. when can you use the _irq versions? The answer is simple to
write, but hard to do in practice:

If you know that when the lock is always taken in this place in a
condition where interrupts are not disabled, you can use _irq.

This is
because the unlock path for the _irq case will unconditionally enable
interrupts (after all, it didn't save what it was before, so all it can
do is blindly enable it); it's generally not right to enable interrupts
in unlock if they weren't enabled at lock time.
(yes someone could find an exception or two in the kernel, but those
are very very special and carefully crafted places).

Typical cases where interrupts are not enabled when you get called
* You or some other code did a spin_lock_irq/spin_lock_irqsave before,
and this lock just nests inside the outer lock. This can be deliberate
or accidental.
* Your code is used during the suspend/resume paths; these tend to
(partially) run with irqs disabled
* Your code is used in interrupt context; interrupt handlers may run
with interrupts disabled, depending on many conditions.
* During early boot interrupts are off too for some duration

there are more, the list I gave is not exhaustive.

But if you KNOW interrupts will be on (for example, because you just
did a sleeping operation in the same function) you can safely use the
_irq version.


Does this answer your question?



--
Arjan van de Ven Intel Open Source Technology Centre
For development, discussion and tips for power savings,
visit http://www.lesswatts.org

2008-10-12 08:10:44

by Oliver Neukum

[permalink] [raw]
Subject: Re: when spin_lock_irq (as opposed to spin_lock_irqsave) is appropriate?

Am Samstag, 11. Oktober 2008 17:55:13 schrieb Andrey Borzenkov:
> On Saturday 11 October 2008, Oliver Neukum wrote:
> > Am Samstag, 11. Oktober 2008 17:29:01 schrieb Andrey Borzenkov:
> > > Logically, one piece of kernel code has no way to know whether another
> > > piece of kernel code (or may be hard-/firmware) has disabled some
> > > interrupt line. So it looks like spin_lock_irq should not even exist,
> > > except may be for very specific cases (where we are sure no other piece
> > > of kernel code may run concurrently)?
> > >
> > > Sorry for stupid question, I an not actually a HW type of person ...
> > >
> >
> > This has no connection with individual irq lines. It's about being able
> > to sleep. Kernel code usually knows whether it can sleep.
> > If it knows to be able to sleep it can use spin_lock_irq() which is
> > more efficient than spin_lock_irqsave()
> >
>
> Sorry? I can't sleep under spinlock ... *any* spinlock? Or has this changed?

You cannot sleep under spinlock.

> May I be I was not clear with question. spin_lock_irq implies spin_unlock_irq,
> which unconditionally enables interrupts. But I have no idea which interrupts
> were disabled before spin_lock_irq; so I may accidentally enable too much?
>
> Or what exactly "irq" in spin_(un-)lock_irq means?

I think I see the source of the problem. The interrupt controller is not
involved. The interrupts are masked in the cpu. This mask switches off
all interrupts (save nmi). When you unmask them again all interrupts
the interrupt controllers allow are active again. This is perfectly safe.

HTH
Oliver

2008-10-12 11:48:22

by Andrey Borzenkov

[permalink] [raw]
Subject: Re: when spin_lock_irq (as opposed to spin_lock_irqsave) is appropriate?

On Saturday 11 October 2008, Arjan van de Ven wrote:
[... very useful explanation omitted ...]
> Does this answer your question?
>

As Oliver pointed out, part of confusion wa my asumption that _irqsave
verion saves actual interrupt mask. It actually does not.

This leaves me with a question - how can I know whether interrupts may
(not) be disabled at particular point? In particular, is it safe to
assume that any place marked at "code may sleep" has interrupts enabled?

Thank you both!

-andrey


Attachments:
(No filename) (502.00 B)
signature.asc (197.00 B)
This is a digitally signed message part.
Download all attachments

2008-10-12 22:21:55

by Arnd Bergmann

[permalink] [raw]
Subject: Re: when spin_lock_irq (as opposed to spin_lock_irqsave) is appropriate?

On Sunday 12 October 2008, Andrey Borzenkov wrote:
> This leaves me with a question - how can I know whether interrupts may
> (not) be disabled at particular point? In particular, is it safe to
> assume that any place marked at "code may sleep" has interrupts enabled?

Yes, that is safe. The only times you know that interrupts are disabled
are:

1. If you have disabled interrupts yourself using local_irq_{disable,save}
or spin_lock_irq{,save}.

2. If you get called from an interface that is documented to have interrupts
disabled. The only common example of this is the interrupt handler function
you register with request_irq().

In all other cases, interrupts are disabled, though in some places you may
not sleep, e.g. because of spin_lock(), preempt_disable() or softirq
context (timer, tasklet, ...). The question of whether you may sleep
or not is irrelevant to whether or not you can use spin_lock_irq.

The rules are:

* If you know that interrupts are disabled, use spin_lock().
* If you know that interrupts are enabled and you might race against
an interrupt handler, use spin_lock_irq().
* If you cannot race against a hard interrupt handler, but can race
against a softirq, use spin_lock_bh().
* If you cannot race against either hardirq or softirq context but cannot
sleep, use spin_lock().
* If you can sleep in all places that take the spinlock, replace the
spinlock with a mutex.
* If you cannot tell whether interrupts are enabled or disabled, but
you can race against a hardirq, use spin_lock_irqsave.

Some people interpret the last rule as "If I can't be bothered to find
out who is calling me, use spin_lock_irqsave", but I much prefer to
be explicit (besides efficient) to give the reader a better indication
of what the lock actually does.

Arnd <><

2008-10-12 23:12:23

by Arjan van de Ven

[permalink] [raw]
Subject: Re: when spin_lock_irq (as opposed to spin_lock_irqsave) is appropriate?

On Sun, 12 Oct 2008 15:48:00 +0400
Andrey Borzenkov <[email protected]> wrote:

> On Saturday 11 October 2008, Arjan van de Ven wrote:
> [... very useful explanation omitted ...]
> > Does this answer your question?
> >
>
> As Oliver pointed out, part of confusion wa my asumption that _irqsave
> verion saves actual interrupt mask. It actually does not.
>
> This leaves me with a question - how can I know whether interrupts may
> (not) be disabled at particular point?

the _irq versions mask the interrupts in the *cpu*!
Not in the hw.
All CPUs have a flag that says "don't give me interrupts right now
please", and the spin_lock_irq(save) functions work on that flag.
And they block all interrupts (except NMI's, which are very special)

> In particular, is it safe to
> assume that any place marked at "code may sleep" has interrupts
> enabled?

yes.
That's a good rule of thumb ;-)
Anything else is a lot of "depends"



--
Arjan van de Ven Intel Open Source Technology Centre
For development, discussion and tips for power savings,
visit http://www.lesswatts.org