• Rafael J. Wysocki's avatar
    PCI/PME: Fix hotplug/sysfs remove deadlock in pcie_pme_remove() · 9546c366
    Rafael J. Wysocki authored
    [ Upstream commit 95c80bc6 ]
    
    Dongdong reported a deadlock triggered by a hotplug event during a sysfs
    "remove" operation:
    
      pciehp 0000:00:0c.0:pcie004: Slot(0-1): Link Up
      # echo 1 > 0000:00:0c.0/remove
    
      PME and hotplug share an MSI/MSI-X vector.  The sysfs "remove" side is:
    
        remove_store
           pci_stop_and_remove_bus_device_locked
    	 pci_lock_rescan_remove
    	 pci_stop_and_remove_bus_device
    	   ...
    	   pcie_pme_remove
    	     pcie_pme_suspend
    	       synchronize_irq        # wait for hotplug IRQ handler
    	 pci_unlock_rescan_remove
    
      The hotplug side is:
    
        pciehp_ist
           pciehp_handle_presence_or_link_change
    	 pciehp_configure_device
    	   pci_lock_rescan_remove     # wait for pci_unlock_rescan_remove()
    
      INFO: task bash:10913 blocked for more than 120 seconds.
    
      # ps -ax |grep D
       PID TTY      STAT   TIME COMMAND
      10913 ttyAMA0  Ds+    0:00 -bash
      14022 ?        D      0:00 [irq/745-pciehp]
    
      # cat /proc/14022/stack
      __switch_to+0x94/0xd8
      pci_lock_rescan_remove+0x20/0x28
      pciehp_configure_device+0x30/0x140
      pciehp_handle_presence_or_link_change+0x324/0x458
      pciehp_ist+0x1dc/0x1e0
    
      # cat /proc/10913/stack
      __switch_to+0x94/0xd8
      synchronize_irq+0x8c/0xc0
      pcie_pme_suspend+0xa4/0x118
      pcie_pme_remove+0x20/0x40
      pcie_port_remove_service+0x3c/0x58
      ...
      pcie_port_device_remove+0x2c/0x48
      pcie_portdrv_remove+0x68/0x78
      pci_device_remove+0x48/0x120
      ...
      pci_stop_bus_device+0x84/0xc0
      pci_stop_and_remove_bus_device_locked+0x24/0x40
      remove_store+0xa4/0xb8
      dev_attr_store+0x44/0x60
      sysfs_kf_write+0x58/0x80
    
    It is incorrect to call pcie_pme_suspend() from pcie_pme_remove() for two
    reasons.
    
    First, pcie_pme_suspend() calls synchronize_irq(), which will wait for the
    native hotplug interrupt handler as well as for the PME one, because they
    share one IRQ (as per the spec).  That may deadlock if hotplug is signaled
    while pcie_pme_remove() is running and the latter calls
    pci_lock_rescan_remove() before the former.
    
    Second, if pcie_pme_suspend() figures out that wakeup needs to be enabled
    for the port, it will return without disabling the interrupt as expected by
    pcie_pme_remove() which was overlooked by commit c7b5a4e6 ("PCI / PM:
    Fix native PME handling during system suspend/resume").
    
    To fix that, rework pcie_pme_remove() to disable the PME interrupt, clear
    its status and prevent the PME worker function from re-enabling it before
    calling free_irq() on it, which should be sufficient.
    
    Fixes: c7b5a4e6 ("PCI / PM: Fix native PME handling during system suspend/resume")
    Link: https://lore.kernel.org/linux-pci/c7697e7c-e1af-13e4-8491-0a3996e6ab5d@huawei.comReported-by: default avatarDongdong Liu <liudongdong3@huawei.com>
    Signed-off-by: default avatarRafael J. Wysocki <rafael.j.wysocki@intel.com>
    [bhelgaas: add URL and deadlock details from Dongdong]
    Signed-off-by: default avatarBjorn Helgaas <bhelgaas@google.com>
    Signed-off-by: default avatarSasha Levin <sashal@kernel.org>
    9546c366
pme.c 11.1 KB