Re: Reworking suspend-resume sequence (was: Re: PCI PM: Restore standard config registers of all devices early)

[Ingo Molnar <mingo@elte.hu>]

* Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Tue, 3 Feb 2009, Linus Torvalds wrote:
> >
> > That said, we've also never had much reason to _care_ deeply, so it's also 
> > possible that we do mask things over some path. I didn't actually walk 
> > _all_ the paths, and the logic for irq handling has changed enough over 
> > the years that I don't know all the paths any more. Maybe we do that 
> > explicit mask in some path I missed. We _shouldn't_, but who knows..
> 
> Ok, so I decided to actually try to walk it all. Better look at the actual 
> code.
> 
> Hmm. The _normal_ simple irq handler does this the way I described, but 
> for some reason the "handle_edge_irq()" does not. And the reason is 
> actually a buglet: it needs to mask things for the "recursive interrupt" 
> case.
> 
> But that literally just looks like a small implementation detail (the code 
> decided to share the code for IRQ_INPROGRESS and IRQ_DISABLED). We should 
> fix it, so that you _can_ disable irqs and not have to worry about this 
> all.
> 
> I'm really not sure why that handle_edge_irq thing uses "ack_and_mask()" 
> instead of just "desc->chip->ack()"? I'm also totally flummoxed as to why 
> it feels it needs to go all the way out to the device to mask things, 
> instead of just masking at an apic level, which is much simpler and faster 
> (especially since masking should never happen in practice anyway).

Hm, do you mean mask_ack_irq()? The ->mask_ack() irqchip method is just a 
small tweak normally: if we get an irq while the irq was disabled, we can 
mark it pending and masks it for real.

It's optional for a PIC implementation to provide it and the generic code 
does it via ->mask() + ->ack() if the PIC implementation keeps it NULL.

[ In theory this also solves screaming level-triggered irqs that advertise 
  themselves as edge-triggered [due to firmware/BIOS bug - these do happen] 
  and then keep spamming the system. ]

I have not done a deep audit, normally (on x86 at least) ->mask_ack() should 
not touch any lowlevel device bits (only the interrupt controller bits). 
Have you found a case where it does?

That would be arguably broken i think - we should not touch lowlevel device 
bits from the current generation of PIC code really, there's just no point.

	Ingo