Re: 2.5.64-mm1: Badness in request_irq

[Andrew Morton <akpm@digeo.com>]

Jeremy Fitzhardinge <jeremy@goop.org> wrote:
>
> On Thu, 2003-03-06 at 06:18, Felipe Alfaro Solana wrote:
> > Hello,  
> >   
> > I have just installed and compiled 2.5.64-mm1 using gcc-3.2.2 and,
> > when booting the system, I have found the  
> > following error on the kernel ring:  
> >   
> > Badness in request_irq at arch/i386/kernel/irq.c:475  
> > Call Trace: [<c010afdd>]  [<d08b3d3a>]  [<d08b18c0>]  [<d08b42c8>]  [<d08b024e>]  [<d08b42b0>]   
> > [<d08b49c0>]  [<d08b49e8>]  [<c0191d6e>]  [<d08b496c>]  [<d08b49e8>]  [<c01947f5>]  [<d08b49e8>]   
> > [<c019491c>]  [<d08b49e8>]  [<d08b4a04>]  [<c0194c10>]  [<d08b49e8>]  [<d08bb160>]  [<c0194fff>]   
> > [<d08b49e8>]  [<c0191eab>]  [<d08b49e8>]  [<d08a0019>]  [<d08b49c0>]  [<d08bb160>]  [<c01302cd>]   
> > [<c010969e>]  
> 
> I'm getting this as well on both my desktop and laptop machines.
> 

Yeah, sorry.  Any request_irq(SA_INTERRUPT|SA_SHIRQ) call will do this.

The problem is that when an SA_INTERRUPT handler shares with a
non-SA_INTERRUPT one, the result is basically meaningless.  The SA_INTERRUPT
handler may (or may not) get executed with interrupts off.

So the intent was to drop a warning saying "hey, someone is being silly
here".

And it _is_ silly, because what the caller is saying is "I want my handler to
run with interrupts disabled, but I am also prepared to share the IRQ with
damn near any other interrupt handler in the kernel".

That warning has to go.  My current thinking is as follows:

- Each time a handler is registered or removed from the chain, we walk the
  IRQ chain and propogate some accumulated info into the controlling
  irq_desc_t.

- If all handlers in the chain are SA_INTERRUPT then the entire chain is
  run with irqs disabled.

- If _any_ handler is !SA_INTERRUPT then all handlers are run with IRQs
  enabled.

This gives sane and documentable semantics.  It also means that any
SA_INTERRUPT handler which is also SA_SHIRQ _must_ be prepared to be called
with irq's enabled.   That is already the case: it is merely being formalised.

If a handler MUST be run with interrupts disabled (for atomicity reasons,
say) then it cannot share.  That's the caller's responsibility.

(One might argue that if any handlers in the chain are SA_INTERRUPT then all
handlers should run with irq's off.  Problem is, the !SA_INTERRUPT handlers
could easily be doing foo_lock_irq()/foo_unlock_irq(), because they think
they were called with irq's enabled).

It's all a bit murky.