Re: [PATCH V2] scsi: libsas: Directly kick-off EH when ATA device fell off

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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);

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).

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?

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.

That discussion is not exactly the same with our issue. That discussion focused on whether one device's error handling can not suspend the other other devices's IO dispatching on the same host. That is something like parallelize the error handling for different device.

However what we are trying to resolve here is to shorten the timeout handling of a unplugged device. The scsi middle layer doesn't know the device is gone and still waiting for the IO until timeout kicks in and start the error handling. This made the applications stuck for a significant long time.But libsas knows that because it receives the phy down event, it knows that device will not come back and there is no need to wait for the timeout.

It's true that this is a half solution. I'd like to have a complete solution too. So we will try to solve both of them.

Thanks,
Jason



[Index of Archives]     [Linux Filesystems]     [Linux SCSI]     [Linux RAID]     [Git]     [Kernel Newbies]     [Linux Newbie]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Samba]     [Device Mapper]

  Powered by Linux