On 4/17/24 00:59, Damien Le Moal wrote: > On 2024/04/16 0:49, Gustav Ekelund wrote: >> On 4/13/24 02:29, Damien Le Moal wrote: >>> On 4/12/24 22:48, Gustav Ekelund wrote: >>>> Expose a new sysfs attribute to userspace that gives root the ability to >>>> lower the link speed in a scsi_device at runtime. The handle enables >>>> programs to, based on external circumstances that may be unbeknownst to >>>> the kernel, determine if a link should slow down to perhaps achieve a >>>> stabler signal. External circumstances could include the mission time >>>> of the connected hardware or observations to temperature trends. >>> >>> may, perhaps, could... This does not sound very deterministic. Do you have an >>> actual practical use case where this patch is useful and solve a real problem ? >>> >>> Strictly speaking, if you are seeing link stability issues due to temperature or >>> other environmental factors (humidity, altitude), then either you are operating >>> your hardware (board and/or HDD) outside of their environmental specifications, >>> or you have some serious hardware issues (which can be a simple as a bad SATA >>> cable or an inappropriate power supply). In both cases, I do not think that this >>> patch will be of any help. >>> >>> Furthermore, libata already lowers a link speed automatically at runtime if it >>> sees too many NCQ errors. Isn't that enough ? And we also have the horkage flags >>> to force a maximum link speed for a device/adapter, which can also be specified >>> as a libata module argument (libata.force). >>> >>>> Writing 1 to /sys/block/*/device/down_link_spd signals the kernel to >>>> first lower the link speed one step with sata_down_spd_limit and then >>>> finish off with sata_link_hardreset. >>> >>> We already have "/sys/class/ata_link/*/hw_sata_spd_limit", which is read-only >>> for now. So if you can really justify this manual link speed tuning for an >>> actual use case (not a hypothetical one), then the way to go would be to make >>> that attribute RW and implement its store() method to lower the link speed at >>> runtime. >>> >>> And by the way, looking at what that attribute says, I always get: >>> <unknown> >>> >>> So it looks like there is an issue with it that went unnoticed (because no one >>> is using it...). This needs some fixing. >>> >> Hello Damien and Niklas, >> >> Thank you for the feedback. >> >> I have a hotplug system, where the links behave differently depending >> on the disk model connected. For some models the kernel emits a lot of >> bus errors, but mostly not enough errors for it to automatically lower >> the link speed, except during high workloads. I have not observed any >> data-loss regarding the errors, but the excessive logging becomes a problem. > > Hot-plugging should not be an issue in itself. When hot-plugged, the port > scanning process should detect the maximum link speed supported by your device > and use that speed for probing the device itself (IDENTIFY etc). If you see bus > errors, then you are either having hardware issues (e.g. a bad cable or power > supply) or some issues with your AHCI controller that may need patching. > > Can you send examples of the errors you are seeing ? That needs to be > investigated first before going the (drastic) route of allowing to manually > lower link speed at run-time. > >> >> So I want to adapt the link, depending on the connected model, in a >> running system because I know that some particular models in this case >> will operate better in SATA2 in this system. >> >> Can I use the libata.force module to make changes to a particular link >> in runtime? > > Nope, libata.force is a module parameter so you can specify it as a kernel boot > parameter, or if you compile libata as a module when loading (modprobe) libata. > At run time, you need to rmmod+modprobe again libata, and so the ahci driver as > well (because of dependencies). > > As I mentioned, if a run-time knob really is necessary (it should not be), using > the ata_link hw_sata_spd_limit would be a better approach. But again, that > really should not be necessary at all. > >> >> Best regards >> Gustav >> > Hi Damien, Unfortunately doing this selectively per link from user space seems to be my only way forward for this particular hardware issue. I understand if you deem this to be too specific to fit into the upstream kernel. I will investigate if running libata as a module might be a way around this peculiar problem. If I need to refine my first approach, I will attempt to modify the hw_sata_spd_limit to be rw. Thank you Damien and Niklas. Best regards Gustav
