Re: [PATCH 0/2][RFC] Make delay before debouncing configurable

["Robin H. Johnson" <robbat2@gentoo.org>]

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@gentoo.org
GnuPG FP : 11ACBA4F 4778E3F6 E4EDF38E B27B944E 34884E85
GnuPG FP : 7D0B3CEB E9B85B1F 825BCECF EE05E6F6 A48F6136