From 9f728f53dd70396f3183d2f0861022259471824b Mon Sep 17 00:00:00 2001 From: Yinghai Lu Date: Thu, 12 May 2011 17:11:47 -0700 Subject: PCI/e1000e: Add and use pci_disable_link_state_locked() Need to use it in _e1000e_disable_aspm. This routine is used for error recovery, where the pci_bus_sem is already held, and we don't want pci_disable_link_state to try to take it again. So add a locked variant for use in cases like this. Found lock up: [ 2374.654557] kworker/32:1 D ffff881027f6b0f0 0 6075 2 0x00000000 [ 2374.654816] ffff88503f099a68 0000000000000046 ffff88503f098000 0000000000004000 [ 2374.654837] 00000000001d1ec0 ffff88503f099fd8 00000000001d1ec0 ffff88503f099fd8 [ 2374.654860] 0000000000004000 00000000001d1ec0 ffff88503dcc8000 ffff88503f090000 [ 2374.654880] Call Trace: [ 2374.654898] [] ? __lock_acquired+0x3a/0x224 [ 2374.654914] [] ? _raw_spin_unlock_irq+0x30/0x36 [ 2374.654925] [] ? trace_hardirqs_on_caller+0x1f/0x178 [ 2374.654936] [] rwsem_down_failed_common+0xd3/0x103 [ 2374.654945] [] ? __lock_contended+0x3a/0x2a2 [ 2374.654955] [] rwsem_down_read_failed+0x12/0x14 [ 2374.654967] [] call_rwsem_down_read_failed+0x14/0x30 [ 2374.654981] [] ? pci_disable_link_state+0x5f/0xf5 [ 2374.654990] [] ? down_read+0x7e/0x91 [ 2374.654999] [] ? pci_disable_link_state+0x5f/0xf5 [ 2374.655008] [] pci_disable_link_state+0x5f/0xf5 [ 2374.655024] [] e1000e_disable_aspm+0x55/0x5a [ 2374.655037] [] e1000_io_slot_reset+0x59/0xea [ 2374.655048] [] ? report_mmio_enabled+0x5d/0x5d [ 2374.655057] [] report_slot_reset+0x2e/0x5d [ 2374.655072] [] pci_walk_bus+0x8a/0xb7 [ 2374.655081] [] ? report_mmio_enabled+0x5d/0x5d [ 2374.655091] [] broadcast_error_message+0xa4/0xb2 [ 2374.655101] [] ? pci_bus_read_config_dword+0x72/0x80 [ 2374.655110] [] do_recovery+0x9e/0xf9 [ 2374.655120] [] handle_error_source+0x4c/0x51 [ 2374.655129] [] aer_isr_one_error+0x1e9/0x21a [ 2374.655138] [] aer_isr+0xc7/0xcc [ 2374.655147] [] ? aer_isr_one_error+0x21a/0x21a [ 2374.655159] [] process_one_work+0x237/0x3ec [ 2374.655168] [] ? process_one_work+0x1a8/0x3ec [ 2374.655178] [] worker_thread+0x17c/0x240 [ 2374.655186] [] ? trace_hardirqs_on+0xd/0xf [ 2374.655196] [] ? manage_workers+0xab/0xab [ 2374.655209] [] kthread+0xa0/0xa8 [ 2374.655223] [] kernel_thread_helper+0x4/0x10 [ 2374.655232] [] ? retint_restore_args+0xe/0xe [ 2374.655243] [] ? __init_kthread_worker+0x5b/0x5b [ 2374.655252] [] ? gs_change+0xb/0xb when aer happens, pci_walk_bus already have down_read(&pci_bus_sem)... then report_slot_reset ==> e1000_io_slot_reset ==> e1000e_disable_aspm ==> pci_disable_link_state... We can not use pci_disable_link_state, and it will try to hold pci_bus_sem again. Try to have __pci_disable_link_state that will not need to hold pci_bus_sem. -v2: change name to pci_disable_link_state_locked() according to Jesse. [jbarnes: make sure new function is exported for modules] Signed-off-by: Yinghai Lu Signed-off-by: Jesse Barnes diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c index 506a0a0..5fb43f0 100644 --- a/drivers/net/e1000e/netdev.c +++ b/drivers/net/e1000e/netdev.c @@ -5347,7 +5347,7 @@ static void e1000_complete_shutdown(struct pci_dev *pdev, bool sleep, #ifdef CONFIG_PCIEASPM static void __e1000e_disable_aspm(struct pci_dev *pdev, u16 state) { - pci_disable_link_state(pdev, state); + pci_disable_link_state_locked(pdev, state); } #else static void __e1000e_disable_aspm(struct pci_dev *pdev, u16 state) diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c index 3eb667b..6892601 100644 --- a/drivers/pci/pcie/aspm.c +++ b/drivers/pci/pcie/aspm.c @@ -734,7 +734,7 @@ void pcie_aspm_powersave_config_link(struct pci_dev *pdev) * pci_disable_link_state - disable pci device's link state, so the link will * never enter specific states */ -void pci_disable_link_state(struct pci_dev *pdev, int state) +static void __pci_disable_link_state(struct pci_dev *pdev, int state, bool sem) { struct pci_dev *parent = pdev->bus->self; struct pcie_link_state *link; @@ -747,7 +747,8 @@ void pci_disable_link_state(struct pci_dev *pdev, int state) if (!parent || !parent->link_state) return; - down_read(&pci_bus_sem); + if (sem) + down_read(&pci_bus_sem); mutex_lock(&aspm_lock); link = parent->link_state; if (state & PCIE_LINK_STATE_L0S) @@ -761,7 +762,19 @@ void pci_disable_link_state(struct pci_dev *pdev, int state) pcie_set_clkpm(link, 0); } mutex_unlock(&aspm_lock); - up_read(&pci_bus_sem); + if (sem) + up_read(&pci_bus_sem); +} + +void pci_disable_link_state_locked(struct pci_dev *pdev, int state) +{ + __pci_disable_link_state(pdev, state, false); +} +EXPORT_SYMBOL(pci_disable_link_state_locked); + +void pci_disable_link_state(struct pci_dev *pdev, int state) +{ + __pci_disable_link_state(pdev, state, true); } EXPORT_SYMBOL(pci_disable_link_state); diff --git a/include/linux/pci-aspm.h b/include/linux/pci-aspm.h index 67cb3ae..7cea7b6 100644 --- a/include/linux/pci-aspm.h +++ b/include/linux/pci-aspm.h @@ -28,6 +28,7 @@ extern void pcie_aspm_exit_link_state(struct pci_dev *pdev); extern void pcie_aspm_pm_state_change(struct pci_dev *pdev); extern void pcie_aspm_powersave_config_link(struct pci_dev *pdev); extern void pci_disable_link_state(struct pci_dev *pdev, int state); +extern void pci_disable_link_state_locked(struct pci_dev *pdev, int state); extern void pcie_clear_aspm(void); extern void pcie_no_aspm(void); #else -- cgit v0.10.2