On 07/01/2025 01:02, Mike Christie wrote:
scsi_check_passthrough is always called
...upon completion?
, but it doesn't check for if a
doesn't properly check
command completed successfully. As a result, if a command was successful
and the caller used SCMD_FAILURE_RESULT_ANY to indicate what failures it
wanted to retry, we will end up retrying the command. This will cause
delays during device discovery because of the command being sent multiple
times. For some USB devices it can also cause the wrong device size to be
used.
This patch adds a check for if the command was successful. If it is we
return immediately instead of trying to match a failure.
Fixes: 994724e6b3f0 ("scsi: core: Allow passthrough to request midlayer retries")
Reported-by: Kris Karas <bugs-a21@xxxxxxxxxxxxxxxx>
Closes: https://urldefense.com/v3/__https://bugzilla.kernel.org/show_bug.cgi?id=219652__;!!ACWV5N9M2RV99hQ!Lvd5QQ8J-3Z1AIBugQywYdRmGcEJdIffF6RBkszOroX8uqGBwmdS0ggKuA7mclKJvk3Cq38QEoOMDRqX3BZ4IOqpAb9Frw$
Signed-off-by: Mike Christie <michael.christie@xxxxxxxxxx>
---
drivers/scsi/scsi_lib.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index adee6f60c966..0cc6a0f77b09 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -210,6 +210,9 @@ static int scsi_check_passthrough(struct scsi_cmnd *scmd,
struct scsi_sense_hdr sshdr;
enum sam_status status;
+ if (!scmd->result)
this could be combined into the !failures check to have a single return,
but that's personal coding preference.
+ return 0;
Would it be messy to drop the separate SCMD_FAILURE_RESULT_ANY check at
the start of the for loop and include that check in the individual
result status and host byte checks? In that way we would not have the
separate !scmd->result check, introduced here. I guess that it would (be
messy).
Regardless of comments:
Reviewed-by: John Garry <john.g.garry@xxxxxxxxxx>
