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/ > > 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. Good point. I will look into adding this. > >> 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) :) 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. -- Damien Le Moal Western Digital Research
