Re: [PATCH 4/4] ata: libata-sata: Improve sata_link_debounce()

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

 



On 3/28/22 16:41, Paul Menzel wrote:
> Dear Damien,
> 
> 
> Am 28.03.22 um 09:19 schrieb Damien Le Moal:
>> On 3/25/22 18:10, Paul Menzel wrote:
> 
>>> Am 23.03.22 um 09:17 schrieb Damien Le Moal:
>>>> sata_link_debounce() polls the SStatus register DET field to ensure that
>>>> a stable value is provided, to reliably detect device presence and PHY
>>>> readiness. Polling is done for at least timing->duration if there is no
>>>> device detected. For the device detected case, polling last up to
>>>
>>> last*s*
>>>
>>>> deadline to ensure that the PHY becomes ready.
>>>>
>>>> However, the PHY ready state is actually never checked, leading to the
>>>> poll loop duration always reaching the maximum duration.
>>>>
>>>> For adapters that do not require a debounce delay (link flag
>>>> ATA_LFLAG_DEBOUNCE_DELAY no set), add a check to test if DET indicates
>>>
>>> no*t*?
>>>
>>>> device present *and* PHY ready and bail out of the polling loop if it
>>>> does.
>>>>
>>>> While at it, add comments to clarify the various checks in
>>>> sata_link_debounce() polling loop.
>>>
>>> Were you able to verify that the check now terminates the loop on your
>>> devices earlier?
>>
>> Yes it does for me. The function goes from taking about 100ms to taking
>> only one iteration of the loop, which has only one 5ms sleep.
>>
>>>
>>> Are below the right lines to compare? On the Dell OptiPlex 5055 (AMD FCH
>>> SATA Controller [AHCI mode] [1022:7901]) I see:
>>>
>>> Before (this very patch)
>>>
>>>       [    0.428015] ata1: hard resetting link
>>>       [    0.696316] ata1: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
>>>
>>> After:
>>>
>>>       [    0.428907] ata1: hard resetting link
>>>       [    0.648092] ata1: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
>>>
>>> So a decrease of (268 - 219 = 49) ms. Nice.
>>
>> For a regular boot, sata_deb_timing_normal duration is used. So the save
>> is 100ms almost (there is still one iteration of the loop needed, which
>> takes interval = 5ms).
>>
>> Together with the long 200ms sleep in sata_link_resume, the gain overall
>> should be about 300ms.
> 
> To my knowledge this was a regular boot (`systemctl reboot`). So where 
> did the 50 ms go? ;-)

The device also sometimes need time to reach DET == 3 state... This is an
optimization that reduces the time waited to reach DET == 3. But we still
wait for it :)

On a hot reboot, as the device are already ready, I do see the time
reduced to one interval (5ms). But for a cold boot, it takes longer sometimes.


-- 
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