ata: libahci: re-enable FBS after error handling on a port multiplier
PATCH v4 1/1 ata: libahci: re-enable FBS after error handling on a port multiplier
ahci_do_softreset() disables FBS before resetting a device behind a port multiplier, as required by AHCI 1.2 section 9.3.9, and re-enables it only when that softreset succeeds. If it fails, for example because a hot-plugged drive is still spinning up ("softreset failed (device not ready)"), EH escalates to a hardreset of the PMP link, and nothing turns FBS back on: only ahci_pmp_attach() does, which runs at the next boot. The port keeps working with command-based switching until then.
Seen on a Marvell 88SE9235 (1b4b:9235) with Marvell 88SM9705 port multipliers (1b4b:9705, in a Synology DX1222 expansion unit): after hot-plugging a drive, the kernel logged "FBS is disabled" followed by "softreset failed (device not ready)" and a PMP hardreset, and never "FBS is enabled" again (twice, on two different ports).
At the end of ahci_error_handler(), re-enable FBS if a PMP is still attached and FBS is supported but disabled.
---
Notes:
Changes in v4 (Niklas Cassel):
- dropped the ata_port_info() print; ahci_enable_fbs() already logs "FBS is enabled"
- moved the testing details out of the commit message Changes in v3: author name only. Changes in v2: author address only.
Tested with the patch built as an out-of-tree libahci module for 7.2.6 (the stock ahci module binds to it, symbol CRCs unchanged) on the 88SE9235 + 88SM9705 setup from the commit message, which is passed through (VFIO) to a KVM guest. Hot-plugging a drive into a free port of the multiplier logged "FBS is disabled", "softreset failed (device not ready)" and a PMP hardreset, then "FBS is enabled" from ahci_enable_fbs(). Afterwards two other drives behind the same multiplier read 259 + 259 MB/s in parallel (O_DIRECT, 4 MiB blocks), against 268 MB/s for one drive alone, so their commands overlap again. The two earlier occurrences without the patch were on ata8 and ata10 of the same controller; this test was on ata7.
drivers/ata/libahci.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
drivers/ata/libahci.c +15 -0
@@ -2233,6 +2233,21 @@ void ahci_error_handler(struct ata_port *ap)sata_pmp_error_handler (ap); +/* +* ahci_do_softreset() turns FBS off before resetting a fan-out link +* and turns it back on only if that reset succeeds. When it fails +* (e.g. a hot-plugged drive is still spinning up), EH escalates to a +* PMP hardreset, which never re-enables FBS, so the port would keep +* command-based switching until the PMP is attached again (reboot). +* Restore FBS once recovery is done and the PMP is still attached. +*/ +if (sata_pmp_attached (ap) &&ata_dev_enabled (ap->link.device)) { +struct ahci_port_priv *pp = ap->private_data; + +if (pp->fbs_supported && !pp->fbs_enabled) +ahci_enable_fbs (ap); + } +if (!ata_dev_enabled (ap->link.device)) hpriv->stop_engine (ap); }
Signature · 1 line
2.43.0
Quoted 9 lines · Kyrylo Biryuk
ahci_do_softreset()disables FBS before resetting a device behind a port multiplier, as required by AHCI 1.2 section 9.3.9, and re-enables it only when that softreset succeeds. If it fails, for example because a hot-plugged drive is still spinning up ("softreset failed (device not ready)"), EH escalates to a hardreset of the PMP link, and nothing turns FBS back on: onlyahci_pmp_attach()does, which runs at the next boot. The port keeps working with command-based switching until then.[...]
Applied to libata/linux.git (for-7.4), thanks!
[1/1] ata: libahci: re-enable FBS after error handling on a port multiplier
https://git.kernel.org/libata/linux/c/21d333ab
Kind regards,
Niklas
Quoted 14 lines · Kyrylo Biryuk
ahci_do_softreset()disables FBS before resetting a device behind a port multiplier, as required by AHCI 1.2 section 9.3.9, and re-enables it only when that softreset succeeds. If it fails, for example because a hot-plugged drive is still spinning up ("softreset failed (device not ready)"), EH escalates to a hardreset of the PMP link, and nothing turns FBS back on: onlyahci_pmp_attach()does, which runs at the next boot. The port keeps working with command-based switching until then.Seen on a Marvell 88SE9235 (1b4b:9235) with Marvell 88SM9705 port multipliers (1b4b:9705, in a Synology DX1222 expansion unit): after hot-plugging a drive, the kernel logged "FBS is disabled" followed by "softreset failed (device not ready)" and a PMP hardreset, and never "FBS is enabled" again (twice, on two different ports).
Quoted 6 lines
At the end of
ahci_error_handler(), re-enable FBS if a PMP is still attached and FBS is supported but disabled.Fixes 89dafa20f3da ("ahci: disabled FBS prior to issuing software reset")Assisted-by LLMSigned-off-by Kyrylo Biryuk <[email protected]>
Looks good.
Signature · 2 lines
Damien Le Moal Western Digital Research
Niklas Cassel
maintainerQuoted 18 lines · Damien Le Moal
Quoted 17 lines · Kyrylo Biryuk
ahci_do_softreset()disables FBS before resetting a device behind a port multiplier, as required by AHCI 1.2 section 9.3.9, and re-enables it only when that softreset succeeds. If it fails, for example because a hot-plugged drive is still spinning up ("softreset failed (device not ready)"), EH escalates to a hardreset of the PMP link, and nothing turns FBS back on: onlyahci_pmp_attach()does, which runs at the next boot. The port keeps working with command-based switching until then.Seen on a Marvell 88SE9235 (1b4b:9235) with Marvell 88SM9705 port multipliers (1b4b:9705, in a Synology DX1222 expansion unit): after hot-plugging a drive, the kernel logged "FBS is disabled" followed by "softreset failed (device not ready)" and a PMP hardreset, and never "FBS is enabled" again (twice, on two different ports).
At the end of
ahci_error_handler(), re-enable FBS if a PMP is still attached and FBS is supported but disabled.
Quoted 7 lines
Quoted 3 lines
Fixes 89dafa20f3da ("ahci: disabled FBS prior to issuing software reset")Assisted-by LLMSigned-off-by Kyrylo Biryuk <[email protected]>Looks good.
Reviewed-by Damien Le Moal <[email protected]>
Commit was updated to pick up Damien's tag:
https://git.kernel.org/pub/scm/linux/kernel/git/libata/linux.git/commit/?id=23eab9e877497397a0e2394ee9174d576816209e
Kind regards,
Niklas