Re: [PATCH] scsi_error fix

[James Bottomley <James.Bottomley@steeleye.com>]

[-- Attachment #1: Type: text/plain, Size: 1061 bytes --]

On Fri, 2003-03-07 at 16:43, Mike Anderson wrote:
> James Bottomley [James.Bottomley@steeleye.com] wrote:
> 
> > @@ -702,8 +701,14 @@
> >  		 * if the result was normal, then just pass it along to the
> >  		 * upper level.
> >  		 */
> > -		if (rtn == SUCCESS)
> > +		if (rtn == SUCCESS) {
> > +			/* we don't want this command reissued, just
> > +			 * finished with the sense data, so set
> > +			 * retries to the max allowed to ensure it
> > +			 * won't get reissued */
> > +			scmd->retries = scmd->allowed;
> >  			scsi_eh_finish_cmd(scmd, done_q);
> > +		}
> >  		if (rtn != NEEDS_RETRY)
> >  			continue;
> >  
> 
> We might need to cover the case down below if we succeed on retry without
> reaching allowed.
> 
> Another option is to look into re-sending the command from the
> scsi_queue_insert handler like we do with timeouts. The interface to the
> device should be the same from the LLDD's point of view just delayed
> some.

The attached should sweep up all of this.  For NEEDS_RETRY, we will
retry until we reach the max retry count.

James


[-- Attachment #2: tmp.diff --]
[-- Type: text/plain, Size: 3600 bytes --]

===== drivers/scsi/scsi_error.c 1.38 vs edited =====
--- 1.38/drivers/scsi/scsi_error.c	Sat Feb 22 10:17:01 2003
+++ edited/drivers/scsi/scsi_error.c	Fri Mar  7 16:15:02 2003
@@ -515,7 +515,7 @@
 	 * actually did complete normally.
 	 */
 	if (rtn == SUCCESS) {
-		int rtn = scsi_eh_completed_normally(scmd);
+		rtn = scsi_eh_completed_normally(scmd);
 		SCSI_LOG_ERROR_RECOVERY(3,
 			printk("%s: scsi_eh_completed_normally %x\n",
 			       __FUNCTION__, rtn));
@@ -545,20 +545,20 @@
 static int scsi_request_sense(struct scsi_cmnd *scmd)
 {
 	static unsigned char generic_sense[6] =
-	{REQUEST_SENSE, 0, 0, 0, 255, 0};
-	unsigned char scsi_result0[256], *scsi_result = &scsi_result0[0];
+	{REQUEST_SENSE, 0, 0, 0, 254, 0};
+	unsigned char *scsi_result;
 	int saved_result;
 	int rtn;
 
 	memcpy(scmd->cmnd, generic_sense, sizeof(generic_sense));
 
-	if (scmd->device->host->hostt->unchecked_isa_dma) {
-		scsi_result = kmalloc(512, GFP_ATOMIC | __GFP_DMA);
-		if (unlikely(!scsi_result)) {
-			printk(KERN_ERR "%s: cannot allocate scsi_result.\n",
-					__FUNCTION__);
-			return FAILED;
-		}
+	scsi_result = kmalloc(254, GFP_ATOMIC | (scmd->device->host->hostt->unchecked_isa_dma) ? __GFP_DMA : 0);
+
+
+	if (unlikely(!scsi_result)) {
+		printk(KERN_ERR "%s: cannot allocate scsi_result.\n",
+		       __FUNCTION__);
+		return FAILED;
 	}
 
 	/*
@@ -568,11 +568,11 @@
 	 * address (db).  0 is not a valid sense code. 
 	 */
 	memset(scmd->sense_buffer, 0, sizeof(scmd->sense_buffer));
-	memset(scsi_result, 0, 256);
+	memset(scsi_result, 0, 254);
 
 	saved_result = scmd->result;
 	scmd->request_buffer = scsi_result;
-	scmd->request_bufflen = 256;
+	scmd->request_bufflen = 254;
 	scmd->use_sg = 0;
 	scmd->cmd_len = COMMAND_SIZE(scmd->cmnd[0]);
 	scmd->sc_data_direction = SCSI_DATA_READ;
@@ -581,13 +581,12 @@
 	rtn = scsi_send_eh_cmnd(scmd, SENSE_TIMEOUT);
 
 	/* last chance to have valid sense data */
-	if (!SCSI_SENSE_VALID(scmd)) {
+	if(!SCSI_SENSE_VALID(scmd)) {
 		memcpy(scmd->sense_buffer, scmd->request_buffer,
-				sizeof(scmd->sense_buffer));
+		       sizeof(scmd->sense_buffer));
 	}
 
-	if (scsi_result != &scsi_result0[0])
-		kfree(scsi_result);
+	kfree(scsi_result);
 
 	/*
 	 * when we eventually call scsi_finish, we really wish to complete
@@ -703,25 +702,14 @@
 		 * upper level.
 		 */
 		if (rtn == SUCCESS)
-			scsi_eh_finish_cmd(scmd, done_q);
-		if (rtn != NEEDS_RETRY)
+			/* we don't want this command reissued, just
+			 * finished with the sense data, so set
+			 * retries to the max allowed to ensure it
+			 * won't get reissued */
+			scmd->retries = scmd->allowed;
+		else if (rtn != NEEDS_RETRY)
 			continue;
 
-		/*
-		 * we only come in here if we want to retry a
-		 * command.  the test to see whether the command
-		 * should be retried should be keeping track of the
-		 * number of tries, so we don't end up looping, of
-		 * course.
-		 */
-		scmd->state = NEEDS_RETRY;
-		rtn = scsi_eh_retry_cmd(scmd);
-		if (rtn != SUCCESS)
-			continue;
-
-		/*
-		 * we eventually hand this one back to the top level.
-		 */
 		scsi_eh_finish_cmd(scmd, done_q);
 	}
 
===== drivers/scsi/scsi_lib.c 1.73 vs edited =====
--- 1.73/drivers/scsi/scsi_lib.c	Sat Feb 22 14:35:33 2003
+++ edited/drivers/scsi/scsi_lib.c	Fri Mar  7 15:19:46 2003
@@ -265,7 +265,6 @@
 	cmd->serial_number = 0;
 	cmd->serial_number_at_timeout = 0;
 	cmd->flags = 0;
-	cmd->retries = 0;
 	cmd->abort_reason = 0;
 
 	memset(cmd->sense_buffer, 0, sizeof cmd->sense_buffer);