This patch introduces an improvement to reduce error handle time and a
tidy-up, including:
- postreset() is deleted from sas_sata_ops.
- Do not perform hard reset and delayed retry on a removed SATA disk. This
can effectively reduce the error handle duration of hot unplug a SATA disk
with traffic(reduce about 30s depending on the delay setting of libata).
Both John garry and Jason Yan participated in the review of the solution
and provided good suggestions during the development.
Luo Jiaxing (2):
{topost} scsi: libsas: delete postreset at sas_sata_ops
{topost} scsi: libsas: check link status at ATA prereset() ops
drivers/scsi/libsas/sas_ata.c | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
--
2.7.4
We found out that libata will retry reset even if SATA disk is unpluged. We
should report offline to libata to avoid meaningless reset on the disk.
Libata provide an ops of prereset() for this purpose, it was called by
ata_eh_reset() only and used to decide whether to skip reset base on the
return value of it.
We check status of phy and disk at prereset(). If disk is already offline
or phy is disabled, we return -ENOENT to libata to skip disk reset.
As prereset() should be best-effort, we should continue to try disk reset
beyond the situation we mentioned before.
Signed-off-by: Luo Jiaxing <[email protected]>
Reviewed-by: John Garry <[email protected]>
---
drivers/scsi/libsas/sas_ata.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
index a7d16d2..1b93332 100644
--- a/drivers/scsi/libsas/sas_ata.c
+++ b/drivers/scsi/libsas/sas_ata.c
@@ -507,8 +507,22 @@ void sas_ata_end_eh(struct ata_port *ap)
spin_unlock_irqrestore(&ha->lock, flags);
}
+static int sas_ata_prereset(struct ata_link *link, unsigned long deadline)
+{
+ struct ata_port *ap = link->ap;
+ struct domain_device *dev = ap->private_data;
+ struct sas_phy *local_phy = sas_get_local_phy(dev);
+ int res = 0;
+
+ if (!local_phy->enabled || test_bit(SAS_DEV_GONE, &dev->state))
+ res = -ENOENT;
+ sas_put_local_phy(local_phy);
+
+ return res;
+}
+
static struct ata_port_operations sas_sata_ops = {
- .prereset = ata_std_prereset,
+ .prereset = sas_ata_prereset,
.hardreset = sas_ata_hard_reset,
.error_handler = ata_std_error_handler,
.post_internal_cmd = sas_ata_post_internal,
--
2.7.4
We fill postreset with ata_std_postreset() at sas_sata_ops before, but we
found out that ata_std_postreset() call sata_scr_read()/sata_scr_write()
which need to access SCR register. Actually we don't own these kind of
register, so sata_scr_read()/sata_scr_write always return -EOPNOTSUPP.
We drop ata_std_postreset() at sas_sata_ops.
Signed-off-by: Luo Jiaxing <[email protected]>
Reviewed-by: John Garry <[email protected]>
---
drivers/scsi/libsas/sas_ata.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
index 5d716d3..a7d16d2 100644
--- a/drivers/scsi/libsas/sas_ata.c
+++ b/drivers/scsi/libsas/sas_ata.c
@@ -510,7 +510,6 @@ void sas_ata_end_eh(struct ata_port *ap)
static struct ata_port_operations sas_sata_ops = {
.prereset = ata_std_prereset,
.hardreset = sas_ata_hard_reset,
- .postreset = ata_std_postreset,
.error_handler = ata_std_error_handler,
.post_internal_cmd = sas_ata_post_internal,
.qc_defer = ata_std_qc_defer,
--
2.7.4
?? 2020/7/22 17:04, Luo Jiaxing д??:
> We fill postreset with ata_std_postreset() at sas_sata_ops before, but we
> found out that ata_std_postreset() call sata_scr_read()/sata_scr_write()
> which need to access SCR register. Actually we don't own these kind of
> register, so sata_scr_read()/sata_scr_write always return -EOPNOTSUPP.
>
> We drop ata_std_postreset() at sas_sata_ops.
>
> Signed-off-by: Luo Jiaxing <[email protected]>
> Reviewed-by: John Garry <[email protected]>
> ---
> drivers/scsi/libsas/sas_ata.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
> index 5d716d3..a7d16d2 100644
> --- a/drivers/scsi/libsas/sas_ata.c
> +++ b/drivers/scsi/libsas/sas_ata.c
> @@ -510,7 +510,6 @@ void sas_ata_end_eh(struct ata_port *ap)
> static struct ata_port_operations sas_sata_ops = {
> .prereset = ata_std_prereset,
> .hardreset = sas_ata_hard_reset,
> - .postreset = ata_std_postreset,
> .error_handler = ata_std_error_handler,
> .post_internal_cmd = sas_ata_post_internal,
> .qc_defer = ata_std_qc_defer,
>
Hi Luo,
Please remove the "{topost}" in the subject, other than that:
Reviewed-by: Jason Yan <[email protected]>
?? 2020/7/22 17:04, Luo Jiaxing д??:
> We found out that libata will retry reset even if SATA disk is unpluged. We
> should report offline to libata to avoid meaningless reset on the disk.
> Libata provide an ops of prereset() for this purpose, it was called by
> ata_eh_reset() only and used to decide whether to skip reset base on the
> return value of it.
>
> We check status of phy and disk at prereset(). If disk is already offline
> or phy is disabled, we return -ENOENT to libata to skip disk reset.
>
> As prereset() should be best-effort, we should continue to try disk reset
> beyond the situation we mentioned before.
>
> Signed-off-by: Luo Jiaxing <[email protected]>
> Reviewed-by: John Garry <[email protected]>
> ---
> drivers/scsi/libsas/sas_ata.c | 16 +++++++++++++++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
The same as the first one, after fix the subject:
Reviewed-by: Jason Yan <[email protected]>
On Wed, 22 Jul 2020 17:04:01 +0800, Luo Jiaxing wrote:
> This patch introduces an improvement to reduce error handle time and a
> tidy-up, including:
> - postreset() is deleted from sas_sata_ops.
> - Do not perform hard reset and delayed retry on a removed SATA disk. This
> can effectively reduce the error handle duration of hot unplug a SATA disk
> with traffic(reduce about 30s depending on the delay setting of libata).
>
> [...]
Applied to 5.9/scsi-queue, thanks!
[1/2] scsi: libsas: Remove postreset from sas_sata_ops
https://git.kernel.org/mkp/scsi/c/3a243c2c3500
[2/2] scsi: libsas: Check link status in ATA prereset()
https://git.kernel.org/mkp/scsi/c/386533796574
--
Martin K. Petersen Oracle Linux Engineering