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

Linus Torvalds
2009-02-03T18:47:22Z

On Tue, 3 Feb 2009, Jesse Barnes wrote:
>
> But won't ->disable point at ->mask in the MSI case (msi_chip doesn't have
> a ->disable, so the IRQ core will make ->disable point at ->mask)?

No. I clarified this in the next message, but basically it boils down to a
simple case: "for edge-triggered interrupts, it's actually stupid to mask
things, because it's both simpler and _cheaper_ to just take change that
the interrupt might happen once" (especially since it almost never
happens).

In fact, for an edge-triggered interrupt it is often a _bug_ to mask the
interrupt source, because you often lose the interrupt. So masking is not
only complex and expensive, it's also often _wrong_. Instead, what
disable_irq() does is to take the interrupt, but instead of calling the
interrupt handlers (it's disabled!) it just sets a flag.

And that is a big _correctness_ issue, because the setting of the flag is
how we know to resend the interrupt when we enable things again - even
though the hardware itself actually lost the edge.

[ Of course, you can hope that the PCI device itself doesn't lose it, and
sees the edge again when unmasking, but the fact that many interrupt
controllers get this fundamental masking issue wrong means that I'd
almost bet that lots of PCI devices get it wrong too! ]

> And in mask we do go poke at PCI regs (msi_set_mask_bits) to mask interrupts
> if possible (though if there's no mask bit in the legacy MSI case we don't do
> anything).

I actually think that's a bug. I think it's just horribly stupid for to
->mask to go down to the device layer (for all the same reasons that we
don't do it for disable), but it doesn't much matter. We only use it for
"shutdown" of the irq controller - we should do the same thing we do for
irq_disable and just set a flag.

The only reason to actually _mask_ an interrupt is if it is
level-triggered and can cause screaming. But even then it's often better
to just wait for the interrupt to happen (and mask it then), because 99%
of the time the interrupt obviously never happens.

But we've never had reason to optimize ->mask/->unmask.

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..

Linus