1 From 87a35aca6c761b392945d5f3b9d6fa94fbfa6527 Mon Sep 17 00:00:00 2001
2 From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
3 Date: Thu, 28 Feb 2019 13:56:27 -0600
4 Subject: PCI/PME: Fix hotplug/sysfs remove deadlock in pcie_pme_remove()
6 [ Upstream commit 95c80bc6952b6a5badc7b702d23e5bf14d251e7c ]
8 Dongdong reported a deadlock triggered by a hotplug event during a sysfs
11 pciehp 0000:00:0c.0:pcie004: Slot(0-1): Link Up
12 # echo 1 > 0000:00:0c.0/remove
14 PME and hotplug share an MSI/MSI-X vector. The sysfs "remove" side is:
17 pci_stop_and_remove_bus_device_locked
18 pci_lock_rescan_remove
19 pci_stop_and_remove_bus_device
23 synchronize_irq # wait for hotplug IRQ handler
24 pci_unlock_rescan_remove
29 pciehp_handle_presence_or_link_change
30 pciehp_configure_device
31 pci_lock_rescan_remove # wait for pci_unlock_rescan_remove()
33 INFO: task bash:10913 blocked for more than 120 seconds.
36 PID TTY STAT TIME COMMAND
37 10913 ttyAMA0 Ds+ 0:00 -bash
38 14022 ? D 0:00 [irq/745-pciehp]
40 # cat /proc/14022/stack
42 pci_lock_rescan_remove+0x20/0x28
43 pciehp_configure_device+0x30/0x140
44 pciehp_handle_presence_or_link_change+0x324/0x458
45 pciehp_ist+0x1dc/0x1e0
47 # cat /proc/10913/stack
49 synchronize_irq+0x8c/0xc0
50 pcie_pme_suspend+0xa4/0x118
51 pcie_pme_remove+0x20/0x40
52 pcie_port_remove_service+0x3c/0x58
54 pcie_port_device_remove+0x2c/0x48
55 pcie_portdrv_remove+0x68/0x78
56 pci_device_remove+0x48/0x120
58 pci_stop_bus_device+0x84/0xc0
59 pci_stop_and_remove_bus_device_locked+0x24/0x40
60 remove_store+0xa4/0xb8
61 dev_attr_store+0x44/0x60
62 sysfs_kf_write+0x58/0x80
64 It is incorrect to call pcie_pme_suspend() from pcie_pme_remove() for two
67 First, pcie_pme_suspend() calls synchronize_irq(), which will wait for the
68 native hotplug interrupt handler as well as for the PME one, because they
69 share one IRQ (as per the spec). That may deadlock if hotplug is signaled
70 while pcie_pme_remove() is running and the latter calls
71 pci_lock_rescan_remove() before the former.
73 Second, if pcie_pme_suspend() figures out that wakeup needs to be enabled
74 for the port, it will return without disabling the interrupt as expected by
75 pcie_pme_remove() which was overlooked by commit c7b5a4e6e8fb ("PCI / PM:
76 Fix native PME handling during system suspend/resume").
78 To fix that, rework pcie_pme_remove() to disable the PME interrupt, clear
79 its status and prevent the PME worker function from re-enabling it before
80 calling free_irq() on it, which should be sufficient.
82 Fixes: c7b5a4e6e8fb ("PCI / PM: Fix native PME handling during system suspend/resume")
83 Link: https://lore.kernel.org/linux-pci/c7697e7c-e1af-13e4-8491-0a3996e6ab5d@huawei.com
84 Reported-by: Dongdong Liu <liudongdong3@huawei.com>
85 Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
86 [bhelgaas: add URL and deadlock details from Dongdong]
87 Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
88 Signed-off-by: Sasha Levin <sashal@kernel.org>
90 drivers/pci/pcie/pme.c | 22 +++++++++++++++-------
91 1 file changed, 15 insertions(+), 7 deletions(-)
93 diff --git a/drivers/pci/pcie/pme.c b/drivers/pci/pcie/pme.c
94 index df290aa58dce..c2e6e3d1073f 100644
95 --- a/drivers/pci/pcie/pme.c
96 +++ b/drivers/pci/pcie/pme.c
97 @@ -367,6 +367,16 @@ static bool pcie_pme_check_wakeup(struct pci_bus *bus)
101 +static void pcie_pme_disable_interrupt(struct pci_dev *port,
102 + struct pcie_pme_service_data *data)
104 + spin_lock_irq(&data->lock);
105 + pcie_pme_interrupt_enable(port, false);
106 + pcie_clear_root_pme_status(port);
107 + data->noirq = true;
108 + spin_unlock_irq(&data->lock);
112 * pcie_pme_suspend - Suspend PCIe PME service device.
113 * @srv: PCIe service device to suspend.
114 @@ -391,11 +401,7 @@ static int pcie_pme_suspend(struct pcie_device *srv)
118 - spin_lock_irq(&data->lock);
119 - pcie_pme_interrupt_enable(port, false);
120 - pcie_clear_root_pme_status(port);
121 - data->noirq = true;
122 - spin_unlock_irq(&data->lock);
123 + pcie_pme_disable_interrupt(port, data);
125 synchronize_irq(srv->irq);
127 @@ -431,9 +437,11 @@ static int pcie_pme_resume(struct pcie_device *srv)
129 static void pcie_pme_remove(struct pcie_device *srv)
131 - pcie_pme_suspend(srv);
132 + struct pcie_pme_service_data *data = get_service_data(srv);
134 + pcie_pme_disable_interrupt(srv->port, data);
135 free_irq(srv->irq, srv);
136 - kfree(get_service_data(srv));
140 static struct pcie_port_service_driver pcie_pme_driver = {