On Fri, Jun 16, 2017 at 9:45 PM, Bjorn Helgaas <[email protected]> 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 <[email protected]>
>> Suggested-by: Rafael J. Wysocki <[email protected]>
>> Cc: Rafael J. Wysocki <[email protected]>
>> Cc: Bjorn Helgaas <[email protected]>
>> Cc: Len Brown <[email protected]>
>> Cc: Dan Williams <[email protected]>
>> Cc: Rui Zhang <[email protected]>
>> Cc: Ying Huang <[email protected]>
>> Cc: [email protected]
>> Cc: [email protected]
>> Cc: [email protected]
>> Signed-off-by: Chen Yu <[email protected]>
>
> 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

Reply via email to