Skip to contentSkip to conversation

scsi: Handle MLQUEUE busy response in scsi_send_eh_cmnd

PATCH In review Hannes Reinecke 2 participants 3 messages Apr 24, 2013 – Apr 25, 2013 SCSI

lore
Share & tools
lore
#0 lore raw

PATCH v3 1/1 scsi: Handle MLQUEUE busy response in scsi_send_eh_cmnd

scsi_send_eh_cmnd() is calling queuecommand() directly, so it needs to check the return value here.
The only valid return codes for queuecommand() are 'busy' states, so we need to wait for a bit to allow the LLDD to recover.

Based on an earlier patch from Wen Xiong.

drivers/scsi/scsi_error.c +16 -6
index c1b05a8..1fc6da94 100644
@@ -791,22 +791,32 @@ static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,
 	struct scsi_device *sdev = scmd->device;
 	struct Scsi_Host *shost = sdev->host;
 	DECLARE_COMPLETION_ONSTACK(done);
-	unsigned long timeleft;
+	unsigned long timeleft = timeout;
 	struct scsi_eh_save ses;
+	const int stall_for = min(HZ/10, 1);
 	int rtn;
 
+retry:
 	scsi_eh_prep_cmnd(scmd, &ses, cmnd, cmnd_size, sense_bytes);
 	shost->eh_action = &done;
 
 	scsi_log_send(scmd);
 	scmd->scsi_done = scsi_eh_done;
-	shost->hostt->queuecommand(shost, scmd);
-
-	timeleft = wait_for_completion_timeout(&done, timeout);
+	rtn = shost->hostt->queuecommand(shost, scmd);
+	if (rtn) {
+		if (timeleft) {
+			scsi_eh_restore_cmnd(scmd, &ses);
+			timeleft -= stall_for;
+			msleep(stall_for);
+			goto retry;
+		}
+		rtn = NEEDS_RETRY;
+	} else
+		timeleft = wait_for_completion_timeout(&done, timeout);
 
 	shost->eh_action = NULL;
 
-	scsi_log_completion(scmd, SUCCESS);
+	scsi_log_completion(scmd, rtn);
 
 	SCSI_LOG_ERROR_RECOVERY(3,
 		printk("%s: scmd: %p, timeleft: %ld\n",
@@ -837,7 +847,7 @@ static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,
 			rtn = FAILED;
 			break;
 		}
-	} else {
+	} else if (!rtn) {
 		scsi_abort_eh_cmnd(scmd);
 		rtn = FAILED;
 	}
#1 lore raw
Quoted 7 lines · Hannes Reinecke

scsi_send_eh_cmnd() is calling queuecommand() directly, so it needs to check the return value here.
The only valid return codes for queuecommand() are 'busy' states, so we need to wait for a bit to allow the LLDD to recover.

Based on an earlier patch from Wen Xiong.

This looks good, only one minor nitpick:

Quoted 23 lines
drivers/scsi/scsi_error.c +3 -1
index c1b05a8..1fc6da94 100644
@@ -791,22 +791,32 @@ static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,
 	struct scsi_device *sdev = scmd->device;
 	struct Scsi_Host *shost = sdev->host;
 	DECLARE_COMPLETION_ONSTACK(done);
-	unsigned long timeleft;
+	unsigned long timeleft = timeout;
 	struct scsi_eh_save ses;
+	const int stall_for = min(HZ/10, 1);
 	int rtn;
 
+retry:
 	scsi_eh_prep_cmnd(scmd, &ses, cmnd, cmnd_size, sense_bytes);
 	shost->eh_action = &done;
 
 	scsi_log_send(scmd);

drivers/scsi/scsi_error.c static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd, · line 810

Quoted 10 lines
drivers/scsi/scsi_error.c +6 -3
 	scmd->scsi_done = scsi_eh_done;
-	shost->hostt->queuecommand(shost, scmd);
-
-	timeleft = wait_for_completion_timeout(&done, timeout);
+	rtn = shost->hostt->queuecommand(shost, scmd);
+	if (rtn) {
+		if (timeleft) {
+			scsi_eh_restore_cmnd(scmd, &ses);
+			timeleft -= stall_for;
+			msleep(stall_for);

Stall for is in HZ: need to convert to ms, so

			msleep(stall_for * 1000/HZ);

I also don't see a need to restore and reprep the command each time, but I don't see a problem with it either.

James

Quoted 26 lines
drivers/scsi/scsi_error.c +7 -3
+			goto retry;
+		}
+		rtn = NEEDS_RETRY;
+	} else
+		timeleft = wait_for_completion_timeout(&done, timeout);
 
 	shost->eh_action = NULL;
 
-	scsi_log_completion(scmd, SUCCESS);
+	scsi_log_completion(scmd, rtn);
 
 	SCSI_LOG_ERROR_RECOVERY(3,
 		printk("%s: scmd: %p, timeleft: %ld\n",
@@ -837,7 +847,7 @@ static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,
 			rtn = FAILED;
 			break;
 		}
-	} else {
+	} else if (!rtn) {
 		scsi_abort_eh_cmnd(scmd);
 		rtn = FAILED;
 	}
--

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

#2 lore raw
Quoted 47 lines · James Bottomley
Quoted 7 lines · Hannes Reinecke

scsi_send_eh_cmnd() is calling queuecommand() directly, so it needs to check the return value here.
The only valid return codes for queuecommand() are 'busy' states, so we need to wait for a bit to allow the LLDD to recover.

Based on an earlier patch from Wen Xiong.

This looks good, only one minor nitpick:

Quoted 23 lines
drivers/scsi/scsi_error.c +3 -1
index c1b05a8..1fc6da94 100644
@@ -791,22 +791,32 @@ static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,
 	struct scsi_device *sdev = scmd->device;
 	struct Scsi_Host *shost = sdev->host;
 	DECLARE_COMPLETION_ONSTACK(done);
-	unsigned long timeleft;
+	unsigned long timeleft = timeout;
 	struct scsi_eh_save ses;
+	const int stall_for = min(HZ/10, 1);
 	int rtn;
 
+retry:
 	scsi_eh_prep_cmnd(scmd, &ses, cmnd, cmnd_size, sense_bytes);
 	shost->eh_action = &done;
 
 	scsi_log_send(scmd);

drivers/scsi/scsi_error.c static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd, · line 810

Quoted 10 lines
drivers/scsi/scsi_error.c +6 -3
 	scmd->scsi_done = scsi_eh_done;
-	shost->hostt->queuecommand(shost, scmd);
-
-	timeleft = wait_for_completion_timeout(&done, timeout);
+	rtn = shost->hostt->queuecommand(shost, scmd);
+	if (rtn) {
+		if (timeleft) {
+			scsi_eh_restore_cmnd(scmd, &ses);
+			timeleft -= stall_for;
+			msleep(stall_for);

Stall for is in HZ: need to convert to ms, so

Quoted 2 lines
			msleep(stall_for * 1000/HZ);

Right. Will be converting it.

Quoted 3 lines

I also don't see a need to restore and reprep the command each time, but I don't see a problem with it either.

This was mainly thought as a safeguard here; just in case someone else might be fiddling with the command. But yeah, it's not required.

Cheers,

Hannes

Signature · 8 lines
Dr. Hannes Reinecke		      zSeries & Storage
[email protected]			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
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