On 09.04.2019 20:20, Bjorn Helgaas wrote: > On Tue, Apr 09, 2019 at 07:32:15PM +0200, Heiner Kallweit wrote: >> On 05.04.2019 21:28, Heiner Kallweit wrote: >>> On 05.04.2019 21:10, Bjorn Helgaas wrote: >>>> On Wed, Apr 03, 2019 at 07:45:29PM +0200, Heiner Kallweit wrote: >>>>> On 03.04.2019 15:14, Bjorn Helgaas wrote: >>>>>> On Wed, Apr 03, 2019 at 07:53:40AM +0200, Heiner Kallweit wrote: >>>>>>> On 02.04.2019 23:57, Bjorn Helgaas wrote: >>>>>>>> On Tue, Apr 02, 2019 at 10:41:20PM +0200, Heiner Kallweit wrote: >>>>>>>>> On 02.04.2019 22:16, Florian Fainelli wrote: >>>>>>>>>> On 4/2/19 12:55 PM, Heiner Kallweit wrote: >>>>>>>>>>> There are numerous reports about different problems caused by >>>>>>>>>>> ASPM incompatibilities between certain network chip versions >>>>>>>>>>> and board chipsets. On the other hand on (especially mobile) >>>>>>>>>>> systems where ASPM works properly it can significantly >>>>>>>>>>> contribute to power-saving and increased battery runtime. >>>>>>>>>>> One problem so far to make ASPM configurable was to find an >>>>>>>>>>> acceptable way of configuration (e.g. module parameters are >>>>>>>>>>> discouraged for that purpose). >>>> >>>>>>>>>>> +Certain combinations of network chip versions and board >>>>>>>>>>> +chipsets result in increased packet latency, PCIe errors, or >>>>>>>>>>> +significantly reduced network performance. Therefore ASPM is >>>>>>>>>>> +off by default. On the other hand ASPM can significantly >>>>>>>>>>> +contribute to power-saving and thus increased battery runtime >>>>>>>>>>> +on notebooks. >>>> >>>>>> That said, I think Frederick has already started working on a plan >>>>>> for the PCI core to expose sysfs files to manage ASPM. This is >>>>>> similar to the link_state files enabled by CONFIG_PCIEASPM_DEBUG, >>>>>> but it will be always enabled and probably structured slightly >>>>>> differently. The idea is that this would be generic and would not >>>>>> require any driver support. >>>> >>>>> Frederick, is there anything you could share already? Or any timeline? >>>>> Based on Bjorns info what seems to be best to me: >>>>> 1. Disable ASPM for r8169 on stable (back to 4.19). >>>>> 2. Once the generic ASPM sysfs attributes are available, reenable ASPM >>>>> for r8169 in net-next. >>>> >>>> This is out of my wheelhouse, but even with a generic sysfs knob, it >>>> doesn't sound like a good idea to me to enable ASPM by default for >>>> r8169 if we think it's unreliable on any significant fraction of >>>> machines. >>>> >>> I was a little bit imprecise. With the second statement I wanted to say: >>> Keep ASPM disabled per default, but make it possible that setting the >>> new sysfs attribute enables ASPM. After digging deeper in the ASPM core >>> code it seems however that we don't even have to touch the driver later. >> >> ASPM has been disabled again for r8169: b75bb8a5b755 ("r8169: disable ASPM >> again"). So, coming back to controlling ASPM via sysfs: >> My first thought would be to extend pci_disable_link_state with support >> for disabling L1.1/L1.2, and then basically expose pci_disable_link_state >> via sysfs (attribute reading being handled with a direct read from >> pcie_link_state->aspm_disable). >> >> Is this what you were planning or do you have some other approach in mind? > > I can't remember the details of what Frederick and I talked about, but > I think that's the general approach. > Thanks, good to hear. Then let's see whether Frederick wants to add something, and based on the feedback I'd set working on this feature on my agenda. > Bjorn > Heiner