public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: Jamie Lokier <jamie@shareable.org>
Cc: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
	"Rusty Russell" <rusty@rustcorp.com.au>,
	"David Brownell" <dbrownell@users.sourceforge.net>,
	"Eric Miao" <eric.y.miao@gmail.com>,
	"Peter Zijlstra" <a.p.zijlstra@chello.nl>,
	"John Stultz" <johnstul@us.ibm.com>,
	"Nicolas Pitre" <nico@marvell.com>,
	LKML <linux-kernel@vger.kernel.org>,
	"Andrew Morton" <akpm@linux-foundation.org>,
	"Ingo Molnar" <mingo@elte.hu>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] warn about shared irqs requesting IRQF_DISABLED registered with setup_irq
Date: Mon, 30 Nov 2009 10:54:24 +0100 (CET)	[thread overview]
Message-ID: <alpine.LFD.2.00.0911301033340.24119@localhost.localdomain> (raw)
In-Reply-To: <20091129151840.GA30813@shareable.org>

On Sun, 29 Nov 2009, Jamie Lokier wrote:
> > > Wouldn't it be quite a lot nicer if generic setup moved the
> > > IRQF_DISABLED handler to be first in the list, if that actually works
> > > in a useful way rather than simply being a quirk that irqs are
> > > disabled for the first one?
> > Hmm, what happens if an ISR runs with irqs disabled even though it
> > doesn't expect it?  I wouldn't bet that nothing breaks.
> 
> Moving the IRQF_DISABLED handler to be first will run an ISR with
> interrupts disabled which *does* expect it, so that's good.
> 
> According to this thread, at the moment when you have multiple
> IRQF_DISABLED|IRQF_SHARED ISRs, only the first one is run with
> interrupts disabled.

Wrong. The flags of the first action are checked to decide whether
interrupts are enabled _before_ calling the action handler.

So for shared interrupts we have the following combinations:

     action1            action2
A    -	                -
B    IRQF_DISABLED      -
C    -                  IRQF_DISABLED
D    IRQF_DISABLED      IRQF_DISABLED

A) Correct behaviour for action1 and action2

B) Correct behaviour for action1, but action2 is called with
   interrupts disabled

C) Correct behaviour for action1, but action2 is called with
   interrupts enabled
 
D) Correct behaviour for action1 and action2 in theory

> In fact I don't see why the kernel cannot put _all_ of the
> IRQ_DISABLED handlers at the beginning of the list, traverse those
> with interrupts disabled, then enable interrupts them for the
> remaining handlers.

That does not work reliable, because IRQF_DISABLED is just describing
the calling convention of the interrupt.

It does not say, that the handler is not allowed to enable
interrupts. So for D) there is no guarantee that the action1 handler
keeps interrupts disabled. If it enables interrupts in the handler
then the assumptions of action2 are already violated.

> > IMHO the best is if a warning is printed or registering fails if shared
> > irq actions don't agree about wanting IRQF_DISABLED.

No, we print a warning when IRQF_SHARED is used with IRQF_DISABLED as
there is no point about agreeing on something which can not be
guaranteed at all.

Thanks,

	tglx

  parent reply	other threads:[~2009-11-30  9:55 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20091127195857.GB28193@n2100.arm.linux.org.uk>
2009-11-27 21:10 ` [PATCH] warn about shared irqs requesting IRQF_DISABLED registered with setup_irq Uwe Kleine-König
2009-11-27 22:18   ` Thomas Gleixner
2009-11-28 20:03     ` Uwe Kleine-König
2009-11-28 21:50       ` Thomas Gleixner
2009-11-28 22:13         ` David Brownell
2009-11-29  2:31         ` Jamie Lokier
2009-11-29 10:26           ` Uwe Kleine-König
2009-11-29 15:18             ` Jamie Lokier
2009-11-29 15:27               ` Russell King - ARM Linux
2009-11-30 20:39                 ` Jamie Lokier
2009-11-30  9:28               ` Uwe Kleine-König
2009-11-30  9:54               ` Thomas Gleixner [this message]
2009-11-28 22:09       ` David Brownell
2009-11-30 10:47   ` [PATCH] genirq: warn about IRQF_SHARED|IRQF_DISABLED at the right place Uwe Kleine-König
2009-11-30 13:54     ` Get rid of IRQF_DISABLED - (was [PATCH] genirq: warn about IRQF_SHARED|IRQF_DISABLED) Thomas Gleixner
2009-11-30 14:03       ` Peter Zijlstra
2009-11-30 14:24         ` Thomas Gleixner
2009-11-30 14:47           ` Alan Cox
2009-11-30 15:01             ` Russell King - ARM Linux
2009-11-30 15:32               ` Alan Cox
2009-11-30 15:43                 ` Russell King - ARM Linux
2009-11-30 20:15               ` Andrew Victor
2009-11-30 20:53               ` David Brownell
2009-11-30 20:38             ` David Brownell
2009-12-01  1:42               ` Andy Walls
2009-11-30 19:59           ` Benjamin Herrenschmidt
2009-11-30 21:31             ` Thomas Gleixner
2009-11-30 21:42               ` Benjamin Herrenschmidt
2009-11-30 21:54                 ` Thomas Gleixner
2009-11-30 14:37       ` Russell King - ARM Linux
2009-11-30 14:39         ` Russell King - ARM Linux
2009-11-30 17:48           ` Thomas Gleixner
2009-11-30 14:51         ` Alan Cox
2009-11-30 21:59           ` Thomas Gleixner
2009-11-30 23:30             ` Alan Cox
2009-11-30 15:38         ` Nicolas Pitre
2009-11-30 17:46         ` Thomas Gleixner
2009-11-30 19:51       ` Uwe Kleine-König
2009-11-30 21:23         ` Thomas Gleixner
2009-11-30 20:21     ` [PATCH] genirq: warn about IRQF_SHARED|IRQF_DISABLED at the right place David Brownell
2009-11-30 20:27       ` Uwe Kleine-König
2010-01-12 15:42     ` [RESEND PATCH] " Uwe Kleine-König

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=alpine.LFD.2.00.0911301033340.24119@localhost.localdomain \
    --to=tglx@linutronix.de \
    --cc=a.p.zijlstra@chello.nl \
    --cc=akpm@linux-foundation.org \
    --cc=dbrownell@users.sourceforge.net \
    --cc=eric.y.miao@gmail.com \
    --cc=jamie@shareable.org \
    --cc=johnstul@us.ibm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=nico@marvell.com \
    --cc=rusty@rustcorp.com.au \
    --cc=u.kleine-koenig@pengutronix.de \
    /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