Messages in this thread | | | Date | Tue, 3 Feb 2009 20:13:34 +0100 | From | Ingo Molnar <> | Subject | Re: Reworking suspend-resume sequence (was: Re: PCI PM: Restore standard config registers of all devices early) |
| |
* 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
| |