On Fri, Jan 18, 2013 at 6:03 PM, Joe Lawrence <Joe.Lawrence@xxxxxxxxxxx> wrote: > On Fri, 18 Jan 2013, Myron Stowe wrote: > >> [+cc Shaohua and Kenji] >> >> On Fri, Jan 18, 2013 at 4:15 PM, Myron Stowe <myron.stowe@xxxxxxxxx> wrote: >> > Joe: >> > >> > Thanks for the data. So the downstream port of interest has ASPM link >> > capability but it's currently not enabled - see LnkCap and LnkCtl >> > above. >> > >> > I'm still do not understand if PCI Express links would even be >> > involved in a topology where all the devices connected below the >> > downstream port are PCI and not PCI Express. Seems as if the ASPM >> > code is going to a lot of work to put link state structures in place >> > for all these devices that would not be capable of supporting ASPM. >> > >> > I'm still trying to come up to speed understanding ASPM so hopefully >> > someone knowledgeable can help clue me in. >> > >> > Myron > > Scratch my earlier worry about link_state for a device without any > subordinate devices for pcie_aspm_exit_link_state to clean up. I forgot > the check that pcie_aspm_init_link_state makes on the subordinate device > list before allocating link_state. > > What is confusing to me is the asymmetry between these two routines and > how the pcie_aspm_sanity_check return value (blacklist, introduced in > commit 46bbdfa4) is handled. I wonder if pcie_aspm_init_link_state could > simply skip any device who's subordinate device list contains no pcie > devices. (Actually one of the things that pcie_aspm_sanity_check looks > for.) I've looked as aspm.c three times in the last couple months, and I agree, it is a bit difficult to analyze, and the asymmetry you mention is one reason. I'd love it if somebody wanted to overhaul it. I wonder if we could have a very simple rule, e.g., allocate link_state for a device IFF the device has a link. Then the init/exit paths could become symmetric, and the question of whether ASPM is enabled could be handled separately. It would become possible to turn ASPM on/off at run-time. I'm a little dubious about the link_list, too. That list is maintained internal to aspm.c, and we have to worry about updating it on hotplugs. I'd rather use some standard way of traversing the PCI hierarchy so it looks like other PCI code and the locking is easier to verify. I guess this is just blue-sky thinking, but thanks for fixing an issue and also pointing out one of the sources of confusion. Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
