On Fri, Jun 16, 2017 at 9:45 PM, Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote: > On Thu, May 25, 2017 at 04:49:07PM +0800, Chen Yu wrote: >> Currently we saw a lot of "No irq handler" errors during hibernation, >> which caused the system hang finally: >> >> [ 710.141581] ata4.00: qc timeout (cmd 0xec) >> [ 710.147135] ata4.00: failed to IDENTIFY (I/O error, err_mask=0x4) >> [ 710.154593] ata4.00: revalidation failed (errno=-5) >> [ 710.468124] ata4: SATA link up 6.0 Gbps (SStatus 133 SControl 300) >> [ 710.477746] do_IRQ: 31.151 No irq handler for vector >> >> According to above logs, there is an interrupt triggered and it is >> dispatched to CPU31 with a vector number 151, but there is no handler >> for it, thus this irq will not get acked and caused irq flood which kill >> the system. To be more specific, the 31.151 is an interrupt from the ahci >> host controller. >> >> After some investigation, the reason why this issue is triggered is >> because the thaw_noirq() function does not restore the MSI/MSIX settings >> across hibernation. >> >> The scenario is illustrated below: >> >> 1. Before the hibernation starts, the irq 34 is the handler for the ahci device, >> which is binded on cpu31. >> 2. Hibernation starts, the ahci device is put into low power state. >> 3. All the nonboot CPUs are put offline, so the irq 34 has to be migrated to >> the last alive one - CPU0. >> 4. After the snapshot has been created, all the nonboot CPUs are brought up again, >> the CPU affinity for IRQ 34 remains to be 0. >> 5. ahci device are put into D0. >> 6. The snapshot is written to the disk. >> >> The issue is triggered in step 6, in theory the ahci interrupt should be >> delivered to CPU0, however the actually result is that this interrupt is >> delivered to the original CPU31 instead, which cause the "No irq handler" issue. >> >> Ying Huang has has provided a clue that, in step 3 it is possible that the writing >> to the register might not take effect as the PCI devices have been put suspended. >> Actually it is true: >> In step 3, the irq 34 affinity is supposed to be modified from 31 to 0, >> but actually it did not. In __pci_write_msi_msg(), if the device is already >> in low power state, the low level msi message entry will not be updated >> but cached. So in theory during the device restore process, the cached msi >> modification information should be written back to the hardware, and this >> is what pci_restore_msi_state() do during normal suspend-resume. >> But this is not the case for hibernation, pci_restore_msi_state() is not >> invoked currently, to be more specific, pci_restore_state() is not invoked >> in pci_pm_thaw_noirq(), although pci_save_state() has saved the necessary >> pci cached information in pci_pm_freeze_noirq(). >> >> This patch tries to restore the pci status for the device during hibernation, >> otherwise the status might be lost across hibernation(for example, the MSI/MSIX >> message settings), which might cause problems during hibernation. >> >> Suggested-by: Ying Huang <ying.huang@xxxxxxxxx> >> Suggested-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx> >> Cc: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx> >> Cc: Bjorn Helgaas <bhelgaas@xxxxxxxxxx> >> Cc: Len Brown <len.brown@xxxxxxxxx> >> Cc: Dan Williams <dan.j.williams@xxxxxxxxx> >> Cc: Rui Zhang <rui.zhang@xxxxxxxxx> >> Cc: Ying Huang <ying.huang@xxxxxxxxx> >> Cc: linux-pci@xxxxxxxxxxxxxxx >> Cc: linux-pm@xxxxxxxxxxxxxxx >> Cc: linux-kernel@xxxxxxxxxxxxxxx >> Signed-off-by: Chen Yu <yu.c.chen@xxxxxxxxx> > > Added a stable tag and applied with Rafael's reviewed-by to pci/pm for > v4.13, thanks! > > pci_restore_state() restores a lot of stuff besides MSI/MSI-X: PCIe > device, link, slot control, ATS, VC, BARs, ACS, IOV. I guess I'm a > little surprised that we haven't noticed more issues if all these > things were broken. That's because they weren't broken. :-) None of them is expected to change over the image creation, which is why pci_pm_thaw_noirq() didn't call pci_restore_state(), but we overlooked the fact that taking nonboot CPUs offline changed the configuration of interrupts that needed to be restored afterward. So this one is really exceptional. Thanks, Rafael
