| Message ID | 20210423113944.42672-4-hare@suse.de |
|---|---|
| State | New |
| Headers | show |
| Series | SCSI result cleanup, part 2 | expand |
On Fri, Apr 23, 2021 at 01:39:08PM +0200, Hannes Reinecke wrote: > Remove the special handling for DRIVER_ERROR; if there is an error > we should just fail the command and don't try anything clever. So this code comes from your commit 40bb61a77347 "scsi_dh_alua: switch to scsi_execute_req_flags()" but that only switches from DRIVER_BUSY to DRIVER_ERROR, which in retrospective looks rather fishy. Some kind of is busy handling here actually does make sense to me, so maybe we should check for that more sensibly?
On 4/26/21 4:54 PM, Christoph Hellwig wrote: > On Fri, Apr 23, 2021 at 01:39:08PM +0200, Hannes Reinecke wrote: >> Remove the special handling for DRIVER_ERROR; if there is an error >> we should just fail the command and don't try anything clever. > > So this code comes from your commit 40bb61a77347 > "scsi_dh_alua: switch to scsi_execute_req_flags()" > > but that only switches from DRIVER_BUSY to DRIVER_ERROR, which in > retrospective looks rather fishy. Some kind of is busy handling here > actually does make sense to me, so maybe we should check for that > more sensibly? > Well, as this patchset nicely demonstrated, no device ever set DRIVER_BUSY, so that particular code was a bit of optimistic coding. But you are correct; we should check for negative errors, as those will be set prior to submission and most likely indicate a temporary error. (I'd rather _not_ latch on distinct errnos here; there's a deep call-chain involved and errors along the way might vary.) Cheers, Hannes -- Dr. Hannes Reinecke Kernel Storage Architect hare@suse.de +49 911 74053 688 SUSE Software Solutions Germany GmbH, 90409 Nürnberg GF: F. Imendörffer, HRB 36809 (AG Nürnberg)
diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c index efa8c0381476..d76c3dccb8cc 100644 --- a/drivers/scsi/device_handler/scsi_dh_alua.c +++ b/drivers/scsi/device_handler/scsi_dh_alua.c @@ -567,8 +567,6 @@ static int alua_rtpg(struct scsi_device *sdev, struct alua_port_group *pg) "%s: rtpg failed, result %d\n", ALUA_DH_NAME, retval); kfree(buff); - if (driver_byte(retval) == DRIVER_ERROR) - return SCSI_DH_DEV_TEMP_BUSY; return SCSI_DH_IO; } @@ -795,8 +793,6 @@ static unsigned alua_stpg(struct scsi_device *sdev, struct alua_port_group *pg) sdev_printk(KERN_INFO, sdev, "%s: stpg failed, result %d", ALUA_DH_NAME, retval); - if (driver_byte(retval) == DRIVER_ERROR) - return SCSI_DH_DEV_TEMP_BUSY; } else { sdev_printk(KERN_INFO, sdev, "%s: stpg failed\n", ALUA_DH_NAME);
Remove the special handling for DRIVER_ERROR; if there is an error we should just fail the command and don't try anything clever. Signed-off-by: Hannes Reinecke <hare@suse.de> --- drivers/scsi/device_handler/scsi_dh_alua.c | 4 ---- 1 file changed, 4 deletions(-)