Dear Damien, dear Robin,
Am 20.01.22 um 01:14 schrieb Damien Le Moal:
On 2022/01/20 2:57, Robin H. Johnson wrote:
Hi, not originally in the thread, but I've run into hardware where the
delay was bumpy before, when I did early porting around SATA PMP code
(https://dev.gentoo.org/~robbat2/patches/libata-development/ if you want
to see really old patches from 2006)
This series esp of a code approach that didn't get merged might be
interesting, that implements hotplug by polling:
https://dev.gentoo.org/~robbat2/patches/libata-development/2007/00-hp-poll/
Polling and a warning, when polling time exceeds like 10 ms, so users
can contact the hardware vendor, would indeed be the most flexible solution.
Robin, do you remember, why these patches were not applied? Just lack of
time and review, or where there issues?
On Fri, Jan 14, 2022 at 06:23:26PM +0900, Damien Le Moal wrote:
On 1/14/22 00:46, Paul Menzel wrote:
The 200 ms delay before debouncing the PHY was introduced for some buggy
old controllers. To decrease the boot time to come closer do instant
boot, add a parameter so users can override that delay.
The current implementation has several drawbacks, and is just a proof of
concept, which some experienced Linux kernel developer can probably
implement in a better way.
I do not think that a libata module parameter is not the way to go with
this: libata is used by all drivers, so for a system that has multiple
adapters, different delays cannot be specified easily.
I think this is a key thing here; and I like that your patch moves to a
flag.
Indeed, I did not think of that.
I am really thinking that the way to go about this is to remove the
200ms delay by default and add it only for drivers that request it with
a link flag. That is, ATA_LFLAG_NO_DEBOUNCE_DELAY needs to become
ATA_LFLAG_DEBOUNCE_DELAY.
I agree that removing it by default is right, but I'd like to make one
additional request here:
Please add a libata.force= flag that lets users enable/disable the delay
per adapter/link.
I think this would be valuable to rule out issues where the debounce
delay is needed on the drive side more than the controller side, esp. in
cases of poorly implemented port multipliers as Tejun & I found back in
2006.
Maybe libata.force=X.Y:no_debounce_delay & libata.force=X.Y:force_debounce_delay
The ata_parse_force_one function as it stands can't handle a parameter
to the value, so you cannot get libata.force=X.Y:debounce_delay=N
without also improving ata_parse_force_one.
Good point. I will look into adding this.
Awesome.
The other large delay is the link stability check in
sata_link_debounce(). 100ms is added (more for hotplug case) to ensure
that the SStatus register DET field provides a stable value. But I
cannot find any text in the AHCI and SATA IO specs that mandate such
large delay.
Nice find!
Adding back Damien’s answer text:
I tried to address all of the above. Please have a look at the top 4
patches in the sata-timing branch of the libata tree:
git@xxxxxxxxxxxxxxxxxxx:pub/scm/linux/kernel/git/dlemoal/libata
The sata-timing branch is for now based on libata for-5.17 branch.
Thank you for cooking this up. I tested this on the ASUS F2A85-M PRO
(AMD, 1022:0x7801), MSI B350M MORTAR (AMD, 1022:0x7901), and IBM S822LC
(Marvell, 1b4b:9235) with no issues and the expected decrease in boot time.
There are differences between the many HDDs & SSDs I have connected
though. There is a lot of scheduling side effects at play, so the gains
are variable in my case. A system with a single disk attached should be
used for proper evaluation.
That gets likely single-disk worst/best case, but I'm still worried
about port multipliers (sadly I don't have the worst-implemented ones
anymore, I sold them to some Windows users)
:)
I have a e-sata port-multiplier box in the lab. But I need to hook it up to my
test box, which means that I have to get out of home for once and go to the
office :) Will do that. Port-multiplier tests are also needed to complete Hannes
series renaming sysfs fields to match the debug messages.
Kind regards,
Paul
