lkml.org 
[lkml]   [2019]   [Mar]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 0/2] ata: libahci: devslp fixes
From
Date
Hi,

On 08-03-19 01:04, Srinivas Pandruvada wrote:
> On Thu, 2019-03-07 at 15:07 -0800, Rajat Jain wrote:
>> Hello,
>>
>> On Thu, Mar 7, 2019 at 12:37 PM Hans de Goede <hdegoede@redhat.com>
>> wrote:
>>>
>>> Hi,
>>>
>>> On 07-03-19 21:27, Gwendal Grignou wrote:
>>>> Srinivas,
>>>>
>>>> I am looking at problem on a laptop machine that suspends to
>>>> S01x, but
>>>> link_management is set to max_performance, because the machine is
>>>> connected to a charger.
>>>
>>> What is setting it to max_performance when charging? I assume
>>> chrome-os is
>>> running something in userspace to do this (like TLP, but I guess
>>> you are not
>>> using TLP) ?
>>
>> Yes, we have a udev script that does this.
>>
>>>
>>> Have you run benchmarks with max_performance vs the default?
>>> I seriously doubt there will be a significant difference, esp.
>>> with a chrome-os style workload.
>>>
>>>> Given DVLSP must be set before the laptop suspends ["""One of the
>>>> requirement for modern x86 system to enter lowest power
>>>> mode (SLP_S0)
>>>> is SATA IP block to be off."""], the machine never reaches S01x.
>>>> Does it make sense to change the target_lpm_policy at suspend
>>>> (ata_port_suspend()) to min_power and bring it back to the
>>>> original
>>>> value on resume?
>>>
>>> If userspace messes with the setting, then userspace should also
>>> put it back before suspending...
>>>
>>> The upstream kernel's default behavior is to have the target level
>>> set
>>> to a fixed level independent of the charging state. Could it be
>>> this
>>> fixed level is actually max-performance ? If that is the default
>>> the
>>> kernel comes up with, that would indicate a kernel bug.
>>
>> Side note: max-performance indeed can be the default forced by the
>> kernel for some (broken) SATA devices:
>>
>> if (dev->horkage & ATA_HORKAGE_NOLPM) {
>> ata_dev_warn(dev, "LPM support broken, forcing
>> max_power\n");
>> dev->link->ap->target_lpm_policy = ATA_LPM_MAX_POWER;
>> }
>>
>> So definitely these systems won't be able to go into S0ix today.
>>
>> But I think the main idea that we are asking is:
>>
>> 1) Yes, we acknowledge that the userspace has set it max-performance.
>>
>> 2) However, given that the kernel already knows that:
>> - while in suspend, there is no real value in retaining the
>> max-performance.
>> - On the contrary, we know system will fail to go into lower
>> power mode because of max-suspend.
>>
>> 3) Does it not make sense to use this knowledge and switch to
>> min_power when we are actually going to suspend (even if user
>> specified max-performance), and restore max-performance on resume?
>
> It is all about regressions. Hence we added multiple conditions for
> setting default to min power.
> It may cause issues for some SATAs, which may not recover once enters
> slumber or DEVSLP. There is also case where user having issues with
> default LPM policy hence he changed policy to max performance. We can't
> detect that.
> So it will be much safer if user space change policy to default before
> calling suspend.

Right, or simply do not mess with the setting in the first place!

I noticed you did not answer this part of my original reply:

"Have you run benchmarks with max_performance vs the default?
I seriously doubt there will be a significant difference, esp.
with a chrome-os style workload."

I seriously doubt the max-performance setting has a user
noticeable impact on performance. So I repeat has someone
actually measured this with real-world chrome-os workloads ?

Regards,

Hans

\
 
 \ /
  Last update: 2019-03-08 09:58    [W:0.060 / U:0.076 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site