scsi: make scsi_eh_scmd_add() always succeed

scsi_eh_scmd_add() currently only will fail if no error handler thread is started (which will never be the case) or if the state machine encounters an illegal transition. But if we're encountering an invalid state transition chances is we cannot fixup things with the error handler. So better add a WARN_ON for illegal host states and make scsi_dh_scmd_add() a void function. Signed-off-by: Hannes Reinecke <hare@suse.de> Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de> Reviewed-by: Bart Van Assche <bart.vanassche@sandisk.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

scsi: make scsi_eh_scmd_add() always succeed
scsi_eh_scmd_add() currently only will fail if no error handler thread is started (which will never be the case) or if the state machine encounters an illegal transition. But if we're encountering an invalid state transition chances is we cannot fixup things with the error handler. So better add a WARN_ON for illegal host states and make scsi_dh_scmd_add() a void function. Signed-off-by: Hannes Reinecke <hare@suse.de> Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de> Reviewed-by: Bart Van Assche <bart.vanassche@sandisk.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Hannes Reinecke · Martin K. Petersen
1 parent 8e8c9d01c5
Showing 3 changed files with 16 additions and 31 deletions Side-by-side Diff
drivers/scsi/scsi_error.c
drivers/scsi/scsi_lib.c
drivers/scsi/scsi_priv.h
@@ -162,13 +162,7 @@
 		}
 	}
  
-	if (!scsi_eh_scmd_add(scmd, 0)) {
-		SCSI_LOG_ERROR_RECOVERY(3,
-			scmd_printk(KERN_WARNING, scmd,
-				    "terminate aborted command\n"));
-		set_host_byte(scmd, DID_TIME_OUT);
-		scsi_finish_command(scmd);
-	}
+	scsi_eh_scmd_add(scmd, 0);
 }
  
 /**
  
  
  
  
  
@@ -228,28 +222,23 @@
  * scsi_eh_scmd_add - add scsi cmd to error handling.
  * @scmd:	scmd to run eh on.
  * @eh_flag:	optional SCSI_EH flag.
- *
- * Return value:
- *	0 on failure.
  */
-int scsi_eh_scmd_add(struct scsi_cmnd *scmd, int eh_flag)
+void scsi_eh_scmd_add(struct scsi_cmnd *scmd, int eh_flag)
 {
 	struct Scsi_Host *shost = scmd->device->host;
 	unsigned long flags;
-	int ret = 0;
+	int ret;
  
-	if (!shost->ehandler)
-		return 0;
+	WARN_ON_ONCE(!shost->ehandler);
  
 	spin_lock_irqsave(shost->host_lock, flags);
-	if (scsi_host_set_state(shost, SHOST_RECOVERY))
-		if (scsi_host_set_state(shost, SHOST_CANCEL_RECOVERY))
-			goto out_unlock;
-
+	if (scsi_host_set_state(shost, SHOST_RECOVERY)) {
+		ret = scsi_host_set_state(shost, SHOST_CANCEL_RECOVERY);
+		WARN_ON_ONCE(ret);
+	}
 	if (shost->eh_deadline != -1 && !shost->last_reset)
 		shost->last_reset = jiffies;
  
-	ret = 1;
 	if (scmd->eh_eflags & SCSI_EH_ABORT_SCHEDULED)
 		eh_flag &= ~SCSI_EH_CANCEL_CMD;
 	scmd->eh_eflags |= eh_flag;
  
@@ -257,9 +246,7 @@
 	list_add_tail(&scmd->eh_entry, &shost->eh_cmd_q);
 	shost->host_failed++;
 	scsi_eh_wakeup(shost);
- out_unlock:
 	spin_unlock_irqrestore(shost->host_lock, flags);
-	return ret;
 }
  
 /**
@@ -288,13 +275,11 @@
 		rtn = host->hostt->eh_timed_out(scmd);
  
 	if (rtn == BLK_EH_NOT_HANDLED) {
-		if (!host->hostt->no_async_abort &&
-		    scsi_abort_command(scmd) == SUCCESS)
-			return BLK_EH_NOT_HANDLED;
-
-		set_host_byte(scmd, DID_TIME_OUT);
-		if (!scsi_eh_scmd_add(scmd, SCSI_EH_CANCEL_CMD))
-			rtn = BLK_EH_HANDLED;
+		if (host->hostt->no_async_abort ||
+		    scsi_abort_command(scmd) != SUCCESS) {
+			set_host_byte(scmd, DID_TIME_OUT);
+			scsi_eh_scmd_add(scmd, SCSI_EH_CANCEL_CMD);
+		}
 	}
  
 	return rtn;
@@ -1593,8 +1593,8 @@
 			scsi_queue_insert(cmd, SCSI_MLQUEUE_DEVICE_BUSY);
 			break;
 		default:
-			if (!scsi_eh_scmd_add(cmd, 0))
-				scsi_finish_command(cmd);
+			scsi_eh_scmd_add(cmd, 0);
+			break;
 	}
 }
  
@@ -72,7 +72,7 @@
 extern int scsi_error_handler(void *host);
 extern int scsi_decide_disposition(struct scsi_cmnd *cmd);
 extern void scsi_eh_wakeup(struct Scsi_Host *shost);
-extern int scsi_eh_scmd_add(struct scsi_cmnd *, int);
+extern void scsi_eh_scmd_add(struct scsi_cmnd *, int);
 void scsi_eh_ready_devs(struct Scsi_Host *shost,
 			struct list_head *work_q,
 			struct list_head *done_q);
...	...	@@ -162,13 +162,7 @@
162	162	}
163	163	}
164	164
165		- if (!scsi_eh_scmd_add(scmd, 0)) {
166		- SCSI_LOG_ERROR_RECOVERY(3,
167		- scmd_printk(KERN_WARNING, scmd,
168		- "terminate aborted command\n"));
169		- set_host_byte(scmd, DID_TIME_OUT);
170		- scsi_finish_command(scmd);
171		- }
	165	+ scsi_eh_scmd_add(scmd, 0);
172	166	}
173	167
174	168	/**
175	169
176	170
177	171
178	172
179	173
...	...	@@ -228,28 +222,23 @@
228	222	* scsi_eh_scmd_add - add scsi cmd to error handling.
229	223	* @scmd: scmd to run eh on.
230	224	* @eh_flag: optional SCSI_EH flag.
231		- *
232		- * Return value:
233		- * 0 on failure.
234	225	*/
235		-int scsi_eh_scmd_add(struct scsi_cmnd *scmd, int eh_flag)
	226	+void scsi_eh_scmd_add(struct scsi_cmnd *scmd, int eh_flag)
236	227	{
237	228	struct Scsi_Host *shost = scmd->device->host;
238	229	unsigned long flags;
239		- int ret = 0;
	230	+ int ret;
240	231
241		- if (!shost->ehandler)
242		- return 0;
	232	+ WARN_ON_ONCE(!shost->ehandler);
243	233
244	234	spin_lock_irqsave(shost->host_lock, flags);
245		- if (scsi_host_set_state(shost, SHOST_RECOVERY))
246		- if (scsi_host_set_state(shost, SHOST_CANCEL_RECOVERY))
247		- goto out_unlock;
248		-
	235	+ if (scsi_host_set_state(shost, SHOST_RECOVERY)) {
	236	+ ret = scsi_host_set_state(shost, SHOST_CANCEL_RECOVERY);
	237	+ WARN_ON_ONCE(ret);
	238	+ }
249	239	if (shost->eh_deadline != -1 && !shost->last_reset)
250	240	shost->last_reset = jiffies;
251	241
252		- ret = 1;
253	242	if (scmd->eh_eflags & SCSI_EH_ABORT_SCHEDULED)
254	243	eh_flag &= ~SCSI_EH_CANCEL_CMD;
255	244	scmd->eh_eflags \|= eh_flag;
256	245
...	...	@@ -257,9 +246,7 @@
257	246	list_add_tail(&scmd->eh_entry, &shost->eh_cmd_q);
258	247	shost->host_failed++;
259	248	scsi_eh_wakeup(shost);
260		- out_unlock:
261	249	spin_unlock_irqrestore(shost->host_lock, flags);
262		- return ret;
263	250	}
264	251
265	252	/**
...	...	@@ -288,13 +275,11 @@
288	275	rtn = host->hostt->eh_timed_out(scmd);
289	276
290	277	if (rtn == BLK_EH_NOT_HANDLED) {
291		- if (!host->hostt->no_async_abort &&
292		- scsi_abort_command(scmd) == SUCCESS)
293		- return BLK_EH_NOT_HANDLED;
294		-
295		- set_host_byte(scmd, DID_TIME_OUT);
296		- if (!scsi_eh_scmd_add(scmd, SCSI_EH_CANCEL_CMD))
297		- rtn = BLK_EH_HANDLED;
	278	+ if (host->hostt->no_async_abort \|\|
	279	+ scsi_abort_command(scmd) != SUCCESS) {
	280	+ set_host_byte(scmd, DID_TIME_OUT);
	281	+ scsi_eh_scmd_add(scmd, SCSI_EH_CANCEL_CMD);
	282	+ }
298	283	}
299	284
300	285	return rtn;
...	...	@@ -1593,8 +1593,8 @@
1593	1593	scsi_queue_insert(cmd, SCSI_MLQUEUE_DEVICE_BUSY);
1594	1594	break;
1595	1595	default:
1596		- if (!scsi_eh_scmd_add(cmd, 0))
1597		- scsi_finish_command(cmd);
	1596	+ scsi_eh_scmd_add(cmd, 0);
	1597	+ break;
1598	1598	}
1599	1599	}
1600	1600
...	...	@@ -72,7 +72,7 @@
72	72	extern int scsi_error_handler(void *host);
73	73	extern int scsi_decide_disposition(struct scsi_cmnd *cmd);
74	74	extern void scsi_eh_wakeup(struct Scsi_Host *shost);
75		-extern int scsi_eh_scmd_add(struct scsi_cmnd *, int);
	75	+extern void scsi_eh_scmd_add(struct scsi_cmnd *, int);
76	76	void scsi_eh_ready_devs(struct Scsi_Host *shost,
77	77	struct list_head *work_q,
78	78	struct list_head *done_q);