Commit 9f728f53dd70396f3183d2f0861022259471824b

Authored by Yinghai Lu
Committed by Jesse Barnes
1 parent a3170c1f92

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]  [<ffffffff810b1302>] ? __lock_acquired+0x3a/0x224
[ 2374.654914]  [<ffffffff81c2b59c>] ? _raw_spin_unlock_irq+0x30/0x36
[ 2374.654925]  [<ffffffff810b069d>] ? trace_hardirqs_on_caller+0x1f/0x178
[ 2374.654936]  [<ffffffff81c2ab24>] rwsem_down_failed_common+0xd3/0x103
[ 2374.654945]  [<ffffffff810b158f>] ? __lock_contended+0x3a/0x2a2
[ 2374.654955]  [<ffffffff81c2ab7b>] rwsem_down_read_failed+0x12/0x14
[ 2374.654967]  [<ffffffff813371e4>] call_rwsem_down_read_failed+0x14/0x30
[ 2374.654981]  [<ffffffff8135df20>] ? pci_disable_link_state+0x5f/0xf5
[ 2374.654990]  [<ffffffff81c2a0e6>] ? down_read+0x7e/0x91
[ 2374.654999]  [<ffffffff8135df20>] ? pci_disable_link_state+0x5f/0xf5
[ 2374.655008]  [<ffffffff8135df20>] pci_disable_link_state+0x5f/0xf5
[ 2374.655024]  [<ffffffff81661796>] e1000e_disable_aspm+0x55/0x5a
[ 2374.655037]  [<ffffffff816677eb>] e1000_io_slot_reset+0x59/0xea
[ 2374.655048]  [<ffffffff8135fe0d>] ? report_mmio_enabled+0x5d/0x5d
[ 2374.655057]  [<ffffffff8135fe3b>] report_slot_reset+0x2e/0x5d
[ 2374.655072]  [<ffffffff8135369e>] pci_walk_bus+0x8a/0xb7
[ 2374.655081]  [<ffffffff8135fe0d>] ? report_mmio_enabled+0x5d/0x5d
[ 2374.655091]  [<ffffffff813603be>] broadcast_error_message+0xa4/0xb2
[ 2374.655101]  [<ffffffff81352c71>] ? pci_bus_read_config_dword+0x72/0x80
[ 2374.655110]  [<ffffffff813606df>] do_recovery+0x9e/0xf9
[ 2374.655120]  [<ffffffff81360786>] handle_error_source+0x4c/0x51
[ 2374.655129]  [<ffffffff81360974>] aer_isr_one_error+0x1e9/0x21a
[ 2374.655138]  [<ffffffff81360a6c>] aer_isr+0xc7/0xcc
[ 2374.655147]  [<ffffffff813609a5>] ? aer_isr_one_error+0x21a/0x21a
[ 2374.655159]  [<ffffffff81096d9f>] process_one_work+0x237/0x3ec
[ 2374.655168]  [<ffffffff81096d10>] ? process_one_work+0x1a8/0x3ec
[ 2374.655178]  [<ffffffff8109728d>] worker_thread+0x17c/0x240
[ 2374.655186]  [<ffffffff810b0803>] ? trace_hardirqs_on+0xd/0xf
[ 2374.655196]  [<ffffffff81097111>] ? manage_workers+0xab/0xab
[ 2374.655209]  [<ffffffff8109c8ed>] kthread+0xa0/0xa8
[ 2374.655223]  [<ffffffff81c332d4>] kernel_thread_helper+0x4/0x10
[ 2374.655232]  [<ffffffff81c2b880>] ? retint_restore_args+0xe/0xe
[ 2374.655243]  [<ffffffff8109c84d>] ? __init_kthread_worker+0x5b/0x5b
[ 2374.655252]  [<ffffffff81c332d0>] ? 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 <yinghai@kernel.org>
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>

Showing 3 changed files with 18 additions and 4 deletions Side-by-side Diff

drivers/net/e1000e/netdev.c
... ... @@ -5347,7 +5347,7 @@
5347 5347 #ifdef CONFIG_PCIEASPM
5348 5348 static void __e1000e_disable_aspm(struct pci_dev *pdev, u16 state)
5349 5349 {
5350   - pci_disable_link_state(pdev, state);
  5350 + pci_disable_link_state_locked(pdev, state);
5351 5351 }
5352 5352 #else
5353 5353 static void __e1000e_disable_aspm(struct pci_dev *pdev, u16 state)
drivers/pci/pcie/aspm.c
... ... @@ -734,7 +734,7 @@
734 734 * pci_disable_link_state - disable pci device's link state, so the link will
735 735 * never enter specific states
736 736 */
737   -void pci_disable_link_state(struct pci_dev *pdev, int state)
  737 +static void __pci_disable_link_state(struct pci_dev *pdev, int state, bool sem)
738 738 {
739 739 struct pci_dev *parent = pdev->bus->self;
740 740 struct pcie_link_state *link;
... ... @@ -747,7 +747,8 @@
747 747 if (!parent || !parent->link_state)
748 748 return;
749 749  
750   - down_read(&pci_bus_sem);
  750 + if (sem)
  751 + down_read(&pci_bus_sem);
751 752 mutex_lock(&aspm_lock);
752 753 link = parent->link_state;
753 754 if (state & PCIE_LINK_STATE_L0S)
... ... @@ -761,7 +762,19 @@
761 762 pcie_set_clkpm(link, 0);
762 763 }
763 764 mutex_unlock(&aspm_lock);
764   - up_read(&pci_bus_sem);
  765 + if (sem)
  766 + up_read(&pci_bus_sem);
  767 +}
  768 +
  769 +void pci_disable_link_state_locked(struct pci_dev *pdev, int state)
  770 +{
  771 + __pci_disable_link_state(pdev, state, false);
  772 +}
  773 +EXPORT_SYMBOL(pci_disable_link_state_locked);
  774 +
  775 +void pci_disable_link_state(struct pci_dev *pdev, int state)
  776 +{
  777 + __pci_disable_link_state(pdev, state, true);
765 778 }
766 779 EXPORT_SYMBOL(pci_disable_link_state);
767 780  
include/linux/pci-aspm.h
... ... @@ -28,6 +28,7 @@
28 28 extern void pcie_aspm_pm_state_change(struct pci_dev *pdev);
29 29 extern void pcie_aspm_powersave_config_link(struct pci_dev *pdev);
30 30 extern void pci_disable_link_state(struct pci_dev *pdev, int state);
  31 +extern void pci_disable_link_state_locked(struct pci_dev *pdev, int state);
31 32 extern void pcie_clear_aspm(void);
32 33 extern void pcie_no_aspm(void);
33 34 #else