On 8/1/24 5:47 PM, Damien Le Moal wrote:
On 8/2/24 05:12, Bart Van Assche wrote:
[PATCH] scsi: core: Retry commands submitted by the SCSI core
Pass-through commands either come from user space (SG_IO ioctl,
SCSI_IOCTL_*, BSG), from the SCSI core or from a SCSI driver (ch,
cxlflash, pktcdvd, scsi_scan, scsi_dh_*, scsi_transport_spi, sd,
sg, st, virtio_scsi). All this code sets scsi_cmnd.allowed to the
maximum number of allowed retries. Hence, removing the
blk_rq_is_passthrough() check from scsi_noretry_cmd() has the
effect that scsi_cmnd.allowed is respected for pass-through
commands.
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index 612489afe8d2..c09f65cfa898 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -1855,7 +1855,7 @@ bool scsi_noretry_cmd(struct scsi_cmnd *scmd)
* assume caller has checked sense and determined
* the check condition was retryable.
*/
- if (req->cmd_flags & REQ_FAILFAST_DEV || blk_rq_is_passthrough(req))
+ if (req->cmd_flags & REQ_FAILFAST_DEV)
return true;
But that will cause *all* passthrough requests to be retried, including those
issued from userspace, no ? I do not think that such change would be acceptable
as that can break userspace using passthrough and expecting to deal with errors
and handling them however they see fit, which may not be retrying commands.
In the SG_IO ioctl implementation I found the following assignment:
scmd->allowed = 0;
In drivers/scsi/sg.c I found the following (SG_DEFAULT_RETRIES == 0):
scmd->allowed = SG_DEFAULT_RETRIES;
Isn't that sufficient to prevent retries in the scsi_noretry_cmd() callers that
check scmd->allowed? The only scsi_noretry_cmd() that does not check
scmd->allowed is scsi_io_completion(). I propose to add an explicit
blk_rq_is_passthrough() check in that function next to the scsi_noretry_cmd()
call.
Thanks,
Bart.
