On 2022/12/19 17:23, John Garry wrote:
On 16/12/2022 10:03, Xingui Yang wrote:
If the ATA device fell off, call sas_ata_device_link_abort() directly and
mark all outstanding QCs as failed and kick-off EH Immediately. This
avoids
having to wait for block layer timeouts.
Signed-off-by: Xingui Yang <yangxingui@xxxxxxxxxx>
---
Changes to v1:
- Use dev_is_sata() to check ATA device type
drivers/scsi/libsas/sas_discover.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/scsi/libsas/sas_discover.c
b/drivers/scsi/libsas/sas_discover.c
index d5bc1314c341..a12b65eb4a2a 100644
--- a/drivers/scsi/libsas/sas_discover.c
+++ b/drivers/scsi/libsas/sas_discover.c
@@ -362,6 +362,9 @@ static void sas_destruct_ports(struct asd_sas_port
*port)
void sas_unregister_dev(struct asd_sas_port *port, struct
domain_device *dev)
{
+ if (test_bit(SAS_DEV_GONE, &dev->state) && dev_is_sata(dev))
+ sas_ata_device_link_abort(dev, false);
Hi, John
Firstly, I think that there is a bug in sas_ata_device_link_abort() ->
ata_link_abort() code in that the host lock in not grabbed, as the
comment in ata_port_abort() mentions. Having said that, libsas had
already some dodgy host locking usage - specifically dropping the lock
for the queuing path (that's something else to be fixed up ... I think
that is due to queue command CB calling task_done() in some cases), but
I still think that sas_ata_device_link_abort() should be fixed (to grab
the host lock).
ok, I agree with you very much for this, I had doubts about whether we
needed to grab lock before.
Secondly, this just seems like a half solution to the age-old problem -
that is, EH eventually kicking in only after 30 seconds when a disk is
removed with active IO. I say half solution as SAS disks still have this
issue for libsas. Can we instead push to try to solve both of them now?
Jason said you must have such an opinion "a half solution". As libsas
does not have any interface to mark all outstanding commands as failed
for SAS disk currently and SAS disk support I/O resumable transmission
after intermittent disconnections, so I want to optimize sata disk first.
If we want to achieve a complete solution, perhaps we need to define
such an interface in libsas and implement it by lldd. My current idea is
to call sas_abort_task() for all outstanding commands in lldd. I wonder
if you approve of this?
Thanks,
Xingui
There was a broad previous discussion on this:
https://urldefense.com/v3/__https://lore.kernel.org/linux-scsi/Ykqg0kr0F*2Fyzk2XW@xxxxxxxxxxxxx/__;JQ!!ACWV5N9M2RV99hQ!MwAZFXXIwuP0lv-kuUIJ0ekUiGBWlTBhU3oQjyOf_yuP1rHDJb8UKMzJjndXNQ-W1PQGJXzgc0bQUsHh4NGh21EOc50$
From that discussion, Hannes was doing some related prep work series,
but I don't think it got completed.
Thanks,
John
+
if (!test_bit(SAS_DEV_DESTROY, &dev->state) &&
!list_empty(&dev->disco_list_node)) {
/* this rphy never saw sas_rphy_add */
.
