On Wed, 11 May 2011 13:23:21 -0700 Yinghai Lu <[email protected]> wrote:
> On 05/09/2011 02:35 PM, Jesse Barnes wrote: > > On Sun, 08 May 2011 11:54:32 -0700 > > Yinghai Lu <[email protected]> wrote: > > > >> > >> Need to use it in _e1000e_disable_aspm. > >> 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. > > > > What about the other callers of e1000e_disable_aspm? Do they already > > have the lock held or is it just reset that needs the already locked > > version? > > yes. > > there is another version when aspm is not defined. and it does not use any > lock. > > #ifdef CONFIG_PCIEASPM > static void __e1000e_disable_aspm(struct pci_dev *pdev, u16 state) > { > pci_disable_link_state(pdev, state); > } > #else > static void __e1000e_disable_aspm(struct pci_dev *pdev, u16 state) > { > int pos; > u16 reg16; > > /* > * Both device and parent should have the same ASPM setting. > * Disable ASPM in downstream component first and then upstream. > */ > pos = pci_pcie_cap(pdev); > pci_read_config_word(pdev, pos + PCI_EXP_LNKCTL, ®16); > reg16 &= ~state; > pci_write_config_word(pdev, pos + PCI_EXP_LNKCTL, reg16); > > if (!pdev->bus->self) > return; > > pos = pci_pcie_cap(pdev->bus->self); > pci_read_config_word(pdev->bus->self, pos + PCI_EXP_LNKCTL, ®16); > reg16 &= ~state; > pci_write_config_word(pdev->bus->self, pos + PCI_EXP_LNKCTL, reg16); > } > #endif No, I mean __e1000e_disable_aspm is called from several spots: *** drivers/net/e1000e/82571.c: e1000_get_variants_82571[435] e1000e_disable_aspm(adapter->pdev, PCIE_LINK_STATE_L0S); *** drivers/net/e1000e/netdev.c: e1000_change_mtu[5027] e1000e_disable_aspm(adapter->pdev, PCIE_LINK_STATE_L1); __e1000_resume[5402] e1000e_disable_aspm(pdev, PCIE_LINK_STATE_L1); e1000_io_slot_reset[5650] e1000e_disable_aspm(pdev, PCIE_LINK_STATE_L1); e1000_probe[5797] e1000e_disable_aspm(pdev, PCIE_LINK_STATE_L1); Are all of them safe for the unlocked version of ASPM disable? -- Jesse Barnes, Intel Open Source Technology Center ------------------------------------------------------------------------------ Achieve unprecedented app performance and reliability What every C/C++ and Fortran developer should know. Learn how Intel has extended the reach of its next-generation tools to help boost performance applications - inlcuding clusters. http://p.sf.net/sfu/intel-dev2devmay _______________________________________________ E1000-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/e1000-devel To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
