[SCSI] Update the SCSI state model to allow blocking in the created state

Brian King <brking@linux.vnet.ibm.com> reported that fibre channel devices can oops during scanning if their ports block (because the device goes from CREATED -> BLOCK -> RUNNING rather than CREATED -> BLOCK -> CREATED). Fix this by adding a new state: CREATED_BLOCK which can only transition back to CREATED and disallow the CREATED -> BLOCK transition. Now both the created and blocked states that the mid-layer recognises can include CREATED_BLOCK. Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>

[SCSI] Update the SCSI state model to allow blocking in the created state
Brian King <brking@linux.vnet.ibm.com> reported that fibre channel devices can oops during scanning if their ports block (because the device goes from CREATED -> BLOCK -> RUNNING rather than CREATED -> BLOCK -> CREATED). Fix this by adding a new state: CREATED_BLOCK which can only transition back to CREATED and disallow the CREATED -> BLOCK transition. Now both the created and blocked states that the mid-layer recognises can include CREATED_BLOCK. Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
James Bottomley
1 parent 0f1d87a2ac
Showing 4 changed files with 54 additions and 16 deletions Side-by-side Diff
drivers/scsi/scsi_lib.c
drivers/scsi/scsi_scan.c
drivers/scsi/scsi_sysfs.c
include/scsi/scsi_device.h
@@ -1251,6 +1251,7 @@
 			break;
 		case SDEV_QUIESCE:
 		case SDEV_BLOCK:
+		case SDEV_CREATED_BLOCK:
 			/*
 			 * If the devices is blocked we defer normal commands.
 			 */
@@ -2064,10 +2065,13 @@
  
 	switch (state) {
 	case SDEV_CREATED:
-		/* There are no legal states that come back to
-		 * created.  This is the manually initialised start
-		 * state */
-		goto illegal;
+		switch (oldstate) {
+		case SDEV_CREATED_BLOCK:
+			break;
+		default:
+			goto illegal;
+		}
+		break;
  
 	case SDEV_RUNNING:
 		switch (oldstate) {
  
  
@@ -2105,14 +2109,23 @@
  
 	case SDEV_BLOCK:
 		switch (oldstate) {
-		case SDEV_CREATED:
 		case SDEV_RUNNING:
+		case SDEV_CREATED_BLOCK:
 			break;
 		default:
 			goto illegal;
 		}
 		break;
  
+	case SDEV_CREATED_BLOCK:
+		switch (oldstate) {
+		case SDEV_CREATED:
+			break;
+		default:
+			goto illegal;
+		}
+		break;
+
 	case SDEV_CANCEL:
 		switch (oldstate) {
 		case SDEV_CREATED:
  
@@ -2394,9 +2407,13 @@
 	int err = 0;
  
 	err = scsi_device_set_state(sdev, SDEV_BLOCK);
-	if (err)
-		return err;
+	if (err) {
+		err = scsi_device_set_state(sdev, SDEV_CREATED_BLOCK);
  
+		if (err)
+			return err;
+	}
+
 	/* 
 	 * The device has transitioned to SDEV_BLOCK.  Stop the
 	 * block layer from calling the midlayer with this device's
@@ -2438,8 +2455,12 @@
 	 * and goose the device queue if successful.  
 	 */
 	err = scsi_device_set_state(sdev, SDEV_RUNNING);
-	if (err)
-		return err;
+	if (err) {
+		err = scsi_device_set_state(sdev, SDEV_CREATED);
+
+		if (err)
+			return err;
+	}
  
 	spin_lock_irqsave(q->queue_lock, flags);
 	blk_start_queue(q);
@@ -730,6 +730,8 @@
 static int scsi_add_lun(struct scsi_device *sdev, unsigned char *inq_result,
 		int *bflags, int async)
 {
+	int ret;
+
 	/*
 	 * XXX do not save the inquiry, since it can change underneath us,
 	 * save just vendor/model/rev.
  
@@ -885,8 +887,18 @@
  
 	/* set the device running here so that slave configure
 	 * may do I/O */
-	scsi_device_set_state(sdev, SDEV_RUNNING);
+	ret = scsi_device_set_state(sdev, SDEV_RUNNING);
+	if (ret) {
+		ret = scsi_device_set_state(sdev, SDEV_BLOCK);
  
+		if (ret) {
+			sdev_printk(KERN_ERR, sdev,
+				    "in wrong state %s to complete scan\n",
+				    scsi_device_state_name(sdev->sdev_state));
+			return SCSI_SCAN_NO_RESPONSE;
+		}
+	}
+
 	if (*bflags & BLIST_MS_192_BYTES_FOR_3F)
 		sdev->use_192_bytes_for_3f = 1;
  
@@ -899,7 +911,7 @@
 	transport_configure_device(&sdev->sdev_gendev);
  
 	if (sdev->host->hostt->slave_configure) {
-		int ret = sdev->host->hostt->slave_configure(sdev);
+		ret = sdev->host->hostt->slave_configure(sdev);
 		if (ret) {
 			/*
 			 * if LLDD reports slave not present, don't clutter
@@ -34,6 +34,7 @@
 	{ SDEV_QUIESCE, "quiesce" },
 	{ SDEV_OFFLINE,	"offline" },
 	{ SDEV_BLOCK,	"blocked" },
+	{ SDEV_CREATED_BLOCK, "created-blocked" },
 };
  
 const char *scsi_device_state_name(enum scsi_device_state state)
@@ -42,9 +42,11 @@
 				 * originate in the mid-layer) */
 	SDEV_OFFLINE,		/* Device offlined (by error handling or
 				 * user request */
-	SDEV_BLOCK,		/* Device blocked by scsi lld.  No scsi 
-				 * commands from user or midlayer should be issued
-				 * to the scsi lld. */
+	SDEV_BLOCK,		/* Device blocked by scsi lld.  No
+				 * scsi commands from user or midlayer
+				 * should be issued to the scsi
+				 * lld. */
+	SDEV_CREATED_BLOCK,	/* same as above but for created devices */
 };
  
 enum scsi_device_event {
  
@@ -393,11 +395,13 @@
 }
 static inline int scsi_device_blocked(struct scsi_device *sdev)
 {
-	return sdev->sdev_state == SDEV_BLOCK;
+	return sdev->sdev_state == SDEV_BLOCK ||
+		sdev->sdev_state == SDEV_CREATED_BLOCK;
 }
 static inline int scsi_device_created(struct scsi_device *sdev)
 {
-	return sdev->sdev_state == SDEV_CREATED;
+	return sdev->sdev_state == SDEV_CREATED ||
+		sdev->sdev_state == SDEV_CREATED_BLOCK;
 }
  
 /* accessor functions for the SCSI parameters */
...	...	@@ -1251,6 +1251,7 @@
1251	1251	break;
1252	1252	case SDEV_QUIESCE:
1253	1253	case SDEV_BLOCK:
	1254	+ case SDEV_CREATED_BLOCK:
1254	1255	/*
1255	1256	* If the devices is blocked we defer normal commands.
1256	1257	*/
...	...	@@ -2064,10 +2065,13 @@
2064	2065
2065	2066	switch (state) {
2066	2067	case SDEV_CREATED:
2067		- /* There are no legal states that come back to
2068		- * created. This is the manually initialised start
2069		- * state */
2070		- goto illegal;
	2068	+ switch (oldstate) {
	2069	+ case SDEV_CREATED_BLOCK:
	2070	+ break;
	2071	+ default:
	2072	+ goto illegal;
	2073	+ }
	2074	+ break;
2071	2075
2072	2076	case SDEV_RUNNING:
2073	2077	switch (oldstate) {
2074	2078
2075	2079
...	...	@@ -2105,14 +2109,23 @@
2105	2109
2106	2110	case SDEV_BLOCK:
2107	2111	switch (oldstate) {
2108		- case SDEV_CREATED:
2109	2112	case SDEV_RUNNING:
	2113	+ case SDEV_CREATED_BLOCK:
2110	2114	break;
2111	2115	default:
2112	2116	goto illegal;
2113	2117	}
2114	2118	break;
2115	2119
	2120	+ case SDEV_CREATED_BLOCK:
	2121	+ switch (oldstate) {
	2122	+ case SDEV_CREATED:
	2123	+ break;
	2124	+ default:
	2125	+ goto illegal;
	2126	+ }
	2127	+ break;
	2128	+
2116	2129	case SDEV_CANCEL:
2117	2130	switch (oldstate) {
2118	2131	case SDEV_CREATED:
2119	2132
...	...	@@ -2394,9 +2407,13 @@
2394	2407	int err = 0;
2395	2408
2396	2409	err = scsi_device_set_state(sdev, SDEV_BLOCK);
2397		- if (err)
2398		- return err;
	2410	+ if (err) {
	2411	+ err = scsi_device_set_state(sdev, SDEV_CREATED_BLOCK);
2399	2412
	2413	+ if (err)
	2414	+ return err;
	2415	+ }
	2416	+
2400	2417	/*
2401	2418	* The device has transitioned to SDEV_BLOCK. Stop the
2402	2419	* block layer from calling the midlayer with this device's
...	...	@@ -2438,8 +2455,12 @@
2438	2455	* and goose the device queue if successful.
2439	2456	*/
2440	2457	err = scsi_device_set_state(sdev, SDEV_RUNNING);
2441		- if (err)
2442		- return err;
	2458	+ if (err) {
	2459	+ err = scsi_device_set_state(sdev, SDEV_CREATED);
	2460	+
	2461	+ if (err)
	2462	+ return err;
	2463	+ }
2443	2464
2444	2465	spin_lock_irqsave(q->queue_lock, flags);
2445	2466	blk_start_queue(q);
...	...	@@ -730,6 +730,8 @@
730	730	static int scsi_add_lun(struct scsi_device sdev, unsigned char inq_result,
731	731	int *bflags, int async)
732	732	{
	733	+ int ret;
	734	+
733	735	/*
734	736	* XXX do not save the inquiry, since it can change underneath us,
735	737	* save just vendor/model/rev.
736	738
...	...	@@ -885,8 +887,18 @@
885	887
886	888	/* set the device running here so that slave configure
887	889	* may do I/O */
888		- scsi_device_set_state(sdev, SDEV_RUNNING);
	890	+ ret = scsi_device_set_state(sdev, SDEV_RUNNING);
	891	+ if (ret) {
	892	+ ret = scsi_device_set_state(sdev, SDEV_BLOCK);
889	893
	894	+ if (ret) {
	895	+ sdev_printk(KERN_ERR, sdev,
	896	+ "in wrong state %s to complete scan\n",
	897	+ scsi_device_state_name(sdev->sdev_state));
	898	+ return SCSI_SCAN_NO_RESPONSE;
	899	+ }
	900	+ }
	901	+
890	902	if (*bflags & BLIST_MS_192_BYTES_FOR_3F)
891	903	sdev->use_192_bytes_for_3f = 1;
892	904
...	...	@@ -899,7 +911,7 @@
899	911	transport_configure_device(&sdev->sdev_gendev);
900	912
901	913	if (sdev->host->hostt->slave_configure) {
902		- int ret = sdev->host->hostt->slave_configure(sdev);
	914	+ ret = sdev->host->hostt->slave_configure(sdev);
903	915	if (ret) {
904	916	/*
905	917	* if LLDD reports slave not present, don't clutter
...	...	@@ -34,6 +34,7 @@
34	34	{ SDEV_QUIESCE, "quiesce" },
35	35	{ SDEV_OFFLINE, "offline" },
36	36	{ SDEV_BLOCK, "blocked" },
	37	+ { SDEV_CREATED_BLOCK, "created-blocked" },
37	38	};
38	39
39	40	const char *scsi_device_state_name(enum scsi_device_state state)
...	...	@@ -42,9 +42,11 @@
42	42	* originate in the mid-layer) */
43	43	SDEV_OFFLINE, /* Device offlined (by error handling or
44	44	* user request */
45		- SDEV_BLOCK, /* Device blocked by scsi lld. No scsi
46		- * commands from user or midlayer should be issued
47		- * to the scsi lld. */
	45	+ SDEV_BLOCK, /* Device blocked by scsi lld. No
	46	+ * scsi commands from user or midlayer
	47	+ * should be issued to the scsi
	48	+ * lld. */
	49	+ SDEV_CREATED_BLOCK, /* same as above but for created devices */
48	50	};
49	51
50	52	enum scsi_device_event {
51	53
...	...	@@ -393,11 +395,13 @@
393	395	}
394	396	static inline int scsi_device_blocked(struct scsi_device *sdev)
395	397	{
396		- return sdev->sdev_state == SDEV_BLOCK;
	398	+ return sdev->sdev_state == SDEV_BLOCK \|\|
	399	+ sdev->sdev_state == SDEV_CREATED_BLOCK;
397	400	}
398	401	static inline int scsi_device_created(struct scsi_device *sdev)
399	402	{
400		- return sdev->sdev_state == SDEV_CREATED;
	403	+ return sdev->sdev_state == SDEV_CREATED \|\|
	404	+ sdev->sdev_state == SDEV_CREATED_BLOCK;
401	405	}
402	406
403	407	/* accessor functions for the SCSI parameters */