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/ 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. > 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. > 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! > 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) -- Robin Hugh Johnson Gentoo Linux: Dev, Infra Lead, Foundation Treasurer E-Mail : robbat2@xxxxxxxxxx GnuPG FP : 11ACBA4F 4778E3F6 E4EDF38E B27B944E 34884E85 GnuPG FP : 7D0B3CEB E9B85B1F 825BCECF EE05E6F6 A48F6136
