Hi,
On 14-02-18 16:48, Greg KH wrote:
On Wed, Feb 14, 2018 at 04:03:17PM +0100, Hans de Goede wrote:
Hi All,
On 14-02-18 15:25, gregkh@xxxxxxxxxxxxxxxxxxx wrote:
This is a note to let you know that I've just added the patch titled
ahci: Allow setting a default LPM policy for mobile chipsets
to the 4.14-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
ahci-allow-setting-a-default-lpm-policy-for-mobile-chipsets.patch
and it can be found in the queue-4.14 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.
I wonder how this ended up on the patches-for-stable list? Torvald's
master commits have neither a Cc: stable or a Fixes tag.
By itself this series is harmless, until someone actually sets
the new Kconfig option to something other then 0.
See my response to the 4.15.y patch for "how" this came to be merged.
+config SATA_MOBILE_LPM_POLICY
+ int "Default SATA Link Power Management policy for mobile chipsets"
+ range 0 4
+ default 0
+ depends on SATA_AHCI
+ help
+ Select the Default SATA Link Power Management (LPM) policy to use
+ for mobile / laptop variants of chipsets / "South Bridges".
+
+ The value set has the following meanings:
+ 0 => Keep firmware settings
+ 1 => Maximum performance
+ 2 => Medium power
+ 3 => Medium power with Device Initiated PM enabled
+ 4 => Minimum power
+
+ Note "Minimum power" is known to cause issues, including disk
+ corruption, with some disks and should not be used.
+
AFAIK 4.14 and older do not have med_power_with_dipm, so setting this
to 3 will lead to a setting of min_power. Which leads me to my worry
about this series, as said in itself it is harmless, but as the help
text says setting it to 4 (*) is dangerous. Actually this week I've
received my first bug report that even med_power_with_dipm is causing
issues (disconnects) with some devices. I'm working with the reporter
an a blacklist entry for the specific SSD in question, but given that
we're still figuring this out for master I wonder how wise it is to
add these to stable, esp. since stable lacks med_power_with_dipm.
At a minimum we should fixup the help-text for 4.14 and older
(4.15 does have med_power_with_dipm).
What would the text be for 4.14.y and older?
Drop the:
3 => Medium power with Device Initiated PM enabled
Line and:
4 => Minimum power
Becomes:
3 => Minimum power
Also "range 0 4" should become "range 0 3"
I'll be glad to fix that
up. Or I can drop the whole thing and fit in the device id update "by
hand", if you think this shouldn't go to the stable trees.
I would prefer for this to not go to the stable trees. The problem is
that if people enable this and set it to "Minimum power" combined
with say a Crucial_CT525MX300 SSD then this is know to cause data-
corruption, which is not just a regression but one of the worst
kind of regressions. Note people can already get the same result
(shoot themselves in the foot) by using powertop --auto-tune,
or TLP, or setting this manually through sysfs. I really wonder
if the stable series is a good place to give people one more
way to shoot themselves in the foot though and I'm worried that
some distro kernel maintainer will see this new option in a stable
update and sets it to Minimum power, which would be quite bad.
As said before I've just received my first bug-report of this
causing issues even with the more conservative "med_power_with_dipm"
option, which is known to e.g. not cause issues on that same
Crucial_CT525MX300 SSD. So I will likely be submitting some
ATA_HORKAGE_NOLPM quirk patches to tj soon. TL;DR: I really
believe this is too adventurous for the stable kernels.
Regards,
Hans