scsi_transport_srp: Add transport layer error handling
Subsystems (2)
Review
Quoted 12 lines
(quoted diff) +12
-0+ /* + * It can occur that after fast_io_fail_tmo expired and before + * dev_loss_tmo expired that the SCSI error handler has + * offlined one or more devices. scsi_target_unblock() doesn't + * change the state of these devices into running, so do that + * explicitly. + */ + spin_lock_irq(shost->host_lock); + __shost_for_each_device(sdev, shost) + if (sdev->sdev_state == SDEV_OFFLINE) + sdev->sdev_state = SDEV_RUNNING; + spin_unlock_irq(shost->host_lock);
Hello Bart,
Do you have test case to verify this behaviour?
Regards,
Jack
Quoted 6 lines · Jack Wang
Quoted 6 lines
(quoted diff) +6
-0+ /* + * It can occur that after fast_io_fail_tmo expired and before + * dev_loss_tmo expired that the SCSI error handler has + * offlined one or more devices. scsi_target_unblock() doesn't + * change the state of these devices into running, so do that + * explicitly.
Quoted 8 lines
Review
Quoted 6 lines
(quoted diff) +6
-0+ */ + spin_lock_irq(shost->host_lock); + __shost_for_each_device(sdev, shost) + if (sdev->sdev_state == SDEV_OFFLINE) + sdev->sdev_state = SDEV_RUNNING; + spin_unlock_irq(shost->host_lock);Do you have test case to verify this behaviour?
Hello Jack,
This is what I came up with after analyzing why a so-called "port flapping" test failed. The concept of that test is simple: use ibportstate to disable and reenable the proper IB port on the switch with random intervals and check whether I/O starts running again if the path remains operational long enough. When running such a test for a few days with random intervals between a few seconds and a few minutes sooner or later it will occur that scsi_try_host_reset() succeeds and that scsi_eh_test_devices() fails. That will cause the SCSI error handler to offline devices. Hence the above code to change the offline state into running after a reconnect succeeds. I'm not proud of that code but I couldn't find a better solution. Maybe the above code won't be necessary anymore once we switch to Hannes' new SCSI error handler.
Bart.
Quoted 31 lines · Bart Van Assche
Quoted 13 lines · Jack Wang
Review
Quoted 12 lines
(quoted diff) +12
-0+ /* + * It can occur that after fast_io_fail_tmo expired and before + * dev_loss_tmo expired that the SCSI error handler has + * offlined one or more devices. doesn't + * change the state of these devices into running, so do that + * explicitly. + */ + spin_lock_irq(shost->host_lock); + __shost_for_each_device(sdev, shost) + if (sdev->sdev_state == SDEV_OFFLINE) + sdev->sdev_state = SDEV_RUNNING; + spin_unlock_irq(shost->host_lock);Quoted 1 line
Do you have test case to verify this behaviour?
Hello Jack,
This is what I came up with after analyzing why a so-called "port flapping" test failed. The concept of that test is simple: use ibportstate to disable and reenable the proper IB port on the switch with random intervals and check whether I/O starts running again if the path remains operational long enough. When running such a test for a few days with random intervals between a few seconds and a few minutes sooner or later it will occur that
scsi_try_host_reset()succeeds and thatscsi_eh_test_devices()fails. That will cause the SCSI error handler to offline devices. Hence the above code to change the offline state into running after a reconnect succeeds. I'm not proud of that code but I couldn't find a better solution. Maybe the above code won't be necessary anymore once we switch to Hannes' new SCSI error handler.
Quoted 1 line
Bart.
Thanks Bart for reply, in fact we saw same problem you describe here. It's reasonable to set the device back to RUNNING after reconnect succeeds. I'm curious why the scsi_target_unblock() doesn't handle this case.
I'm not sure new SCSI eh from Hannes will avoid scsi eh set device to offline in such situation, but at least it will avoid one bad lun block whole host.
Jack