Prevent infinite retries due to DID_RESET return status

Due to a firmware mismatch between a host and target (names withheld to protect the innocent?), the LLDD was returning DID_RESET for every i/o command. This patch modifies the scsi layer to take into account when the command which received DID_RESET was issued and eventually give up on it instead of unconditionally reissuing it forever when it receives a DID_RESET. With this patch, on my test system, the command receiving the constant DID_RESET times out after about 360 seconds.

The premise for this patch is that no command should have an infinite lifetime. The impetus for this patch was a system which would not reach a command prompt without disconnecting the storage from the host.

The significant change in this patch is to call scsi_retry_command() instead of scsi_requeue_command() if the command which receives a DID_RESET did not complete any i/o (good_bytes==0). scsi_retry_command() does not release the command and regenerate it like scsi_requeue_command() does, hence jiffies_at_alloc reflects when the command was first issued.

Quoted 14 lines · Michael Reed

Due to a firmware mismatch between a host and target (names withheld to protect the innocent?), the LLDD was returning DID_RESET for every i/o command. This patch modifies the scsi layer to take into account when the command which received DID_RESET was issued and eventually give up on it instead of unconditionally reissuing it forever when it receives a DID_RESET. With this patch, on my test system, the command receiving the constant DID_RESET times out after about 360 seconds.

The premise for this patch is that no command should have an infinite lifetime. The impetus for this patch was a system which would not reach a command prompt without disconnecting the storage from the host.

Quoted 5 lines

The significant change in this patch is to call scsi_retry_command() instead of scsi_requeue_command() if the command which receives a DID_RESET did not complete any i/o (good_bytes==0). scsi_retry_command() does not release the command and regenerate it like scsi_requeue_command() does, hence jiffies_at_alloc reflects when the command was first issued.

Generally this patch looks good to me. Some comments:

drivers/scsi/scsi_priv.h extern int scsi_dispatch_cmd(struct scsi · line 31

Quoted 2 lines

drivers/scsi/scsi_priv.h +1 -1

-extern int scsi_retry_command(struct scsi_cmnd *cmd);
+extern int scsi_retry_command(struct scsi_cmnd *cmd, int reason);

I've just sent a patch to kill scsi_retry_command. Use scsi_queue_insert directly instead.

Review

Quoted 1 line

(quoted diff) +1 -0

+	scsi_release_buffers(cmd);

Can you please separate out the moving of the scsi_release_buffer calls into a separate patch so it can be audited better?

drivers/scsi/scsi_lib.c void scsi_io_completion(struct scsi_cmnd · line 998

Quoted 8 lines

drivers/scsi/scsi_lib.c +3 -0

@@ -961,9 +992,20 @@ void scsi_io_completion(struct scsi_cmnd
 		/* Third party bus reset or reset for error recovery
 		 * reasons.  Just retry the request and see what
 		 * happens.
+		 * If no data was transferred, just reissue this
+		 * command.  If data was transferred, regenerate
+		 * the command to transfer only untransferred data.
 		 */

The whole comment should look more like:

/*
* Third party bus reset or reset for error recovery reasons.
* If no data was transferred, just reissue this command.
* If data was transferred, regenerate the command to transfer
* only untransferred data.
*/

drivers/scsi/scsi_lib.c void scsi_io_completion(struct scsi_cmnd · line 1008

Quoted 12 lines

drivers/scsi/scsi_lib.c +10 -2

-		scsi_requeue_command(q, cmd);
-		return;
+		if (!good_bytes) {
+			if (!(scsi_command_expired(cmd))) {
+				scsi_retry_command(cmd, SCSI_MLQUEUE_DID_RESET);
+				return;
+			}
+		}
+		else {
+			scsi_requeue_command(q, cmd);
+			return;
+		}

With this code we now fallthrough if we don't have any good bytes and the command has expired. Is this the expected behaviour? If yes we need a good comment describing it.

-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html