Messages in this thread | | | Subject | Re: [PATCH 0/2] ata: libahci: devslp fixes | From | Hans de Goede <> | Date | Fri, 8 Mar 2019 09:57:53 +0100 |
| |
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
| |