Re: [GIT PULL] ata changes for 5.18-rc1

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

 



On 3/24/22 08:04, Limonciello, Mario wrote:
> [Public]
> 
>> On Mon, Mar 21, 2022 at 11:57 PM Damien Le Moal
>> <damien.lemoal@xxxxxxxxxxxxxxxxxx> wrote:
>>>
>>> * Rename ahci_board_mobile to board_ahci_low_power to be more
>> descriptive
>>>   of the feature as that is also used on PC and server AHCI adapters,
>>>   from Mario.
>>>
>>> Mario Limonciello (3):
>>>       ata: ahci: Rename board_ahci_mobile
>>>       ata: ahci: Rename `AHCI_HFLAG_IS_MOBILE`
>>>       ata: ahci: Rename CONFIG_SATA_LPM_MOBILE_POLICY configuration item
>>
>> So I've pulled this, but it's worth noting that particularly renaming
>> that CONFIG option was probably not a good idea.
>>
>> Why?
>>
>> Because it means that people silently lose their old values. And it has that
>>
>>         range 0 4
>>         default 0
>>
>> with 4 being explicitly marked as very dangerous - but at least Fedora
>> seems to actually have a default of 3 in their kernels:
>>
>>   /boot/config-5.16.13-200.fc35.x86_64:
>>         CONFIG_SATA_MOBILE_LPM_POLICY=3
>>
>> so that "default 0" may actually not be the right one.
>>
>> Now, we're at the point where few enough people likely care about ATA,
>> but the corollary to that is also that these kinds of changes can
>> cause user pain that then developers have *no* idea about.
>> Particularly when the pain ends up being caused by some subtle default
>> config option silently changing that nobody even thought about.
>>
>> Now, that "default 0" is probably the only safe default - and I don't
>> dispute that part. But I also suspect that Fedora chose that value '3'
>> because it probably makes a noticeable power use difference on at
>> least some platforms.
>>
>> I don't know. But I doubt really *anybody* knows, so renaming them and
>> silently likely changing config options for some less-than-critical
>> reason is just not a great idea.
>>
>>                 Linus
> 
> Thanks for pointing out the subtlety of renaming a configuration option hides
> problems because people don't see the new config option and pick the default.
> I wouldn't call this configuration option rename critical, so if you chose to revert
> it I would understand.
> 
> However I think you raise a good point that if distros are picking different "default"
> values and keeping them there a long time that the value in the upstream kernel
> is probably not right anymore.  A while back that default made sense because all the
> power saving stuff was risky at the time.  It's pretty well baked now.
> 
> So maybe a logical thing is to keep this change and send a follow up that also changes
> the default to 3?  If you're supportive of that I'll send something to Damien to do that.

Mario, let's check what other distros do first before deciding. Fedora for
sure has a default of 3 and I have never seen any issue with it (and I
have been using Fedora for a long time with many different drives).

Not sure what distro you are using, but if it is not Fedora, please check.
We should check at least Debian, Ubuntu, SUSE, RHEL and CentOS. I can
check some other minor ones too as I know users.

-- 
Damien Le Moal
Western Digital Research



[Index of Archives]     [Linux Filesystems]     [Linux SCSI]     [Linux RAID]     [Git]     [Kernel Newbies]     [Linux Newbie]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Samba]     [Device Mapper]

  Powered by Linux