Re: [PATCH net] r8169: switch off ASPM by default and add sysfs attribute to control ASPM

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux