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

[Linus Torvalds]

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/