[PATCH] USB: usb-storage driver changes for 2.6.x [1/4]

Patch as239b from Alan Stern:  This patch improves the interaction between
a SCSI reset, an internally generated reset, and an abort.  This improves
our error-recovery in cases where the device is hung (or almost hung) while
we're trying to auto-reset.

drivers/usb/storage/scsiglue.c

@@ -159,13 +159,17 @@ static int command_abort( Scsi_Cmnd *srb )
 		return FAILED;
 	}
 
-	/* Set state to ABORTING, set the ABORTING bit, and release the lock */
+	/* Set state to ABORTING and set the ABORTING bit, but only if
+	 * a device reset isn't already in progress (to avoid interfering
+	 * with the reset).  To prevent races with auto-reset, we must
+	 * stop any ongoing USB transfers while still holding the host
+	 * lock. */
 	us->sm_state = US_STATE_ABORTING;
+	if (!test_bit(US_FLIDX_RESETTING, &us->flags)) {
 		set_bit(US_FLIDX_ABORTING, &us->flags);
-	scsi_unlock(host);
-
-	/* Stop an ongoing USB transfer */
 		usb_stor_stop_transport(us);
+	}
+	scsi_unlock(host);
 
 	/* Wait for the aborted command to finish */
 	wait_for_completion(&us->notify);
@@ -254,18 +258,17 @@ static int bus_reset( Scsi_Cmnd *srb )
 }
 
 /* Report a driver-initiated device reset to the SCSI layer.
- * Calling this for a SCSI-initiated reset is unnecessary but harmless. */
+ * Calling this for a SCSI-initiated reset is unnecessary but harmless.
+ * The caller must own the SCSI host lock. */
 void usb_stor_report_device_reset(struct us_data *us)
 {
 	int i;
 
-	scsi_lock(us->host);
 	scsi_report_device_reset(us->host, 0, 0);
 	if (us->flags & US_FL_SCM_MULT_TARG) {
 		for (i = 1; i < us->host->max_id; ++i)
 			scsi_report_device_reset(us->host, 0, i);
 	}
-	scsi_unlock(us->host);
 }
 
 /***********************************************************************

drivers/usb/storage/transport.c

@@ -137,7 +137,7 @@ static int usb_stor_msg_common(struct us_data *us, int timeout)
 	int status;
 
 	/* don't submit URBs during abort/disconnect processing */
-	if (us->flags & DONT_SUBMIT)
+	if (us->flags & ABORTING_OR_DISCONNECTING)
 		return -EIO;
 
 	/* set up data structures for the wakeup system */
@@ -172,7 +172,7 @@ static int usb_stor_msg_common(struct us_data *us, int timeout)
 	set_bit(US_FLIDX_URB_ACTIVE, &us->flags);
 
 	/* did an abort/disconnect occur during the submission? */
-	if (us->flags & DONT_SUBMIT) {
+	if (us->flags & ABORTING_OR_DISCONNECTING) {
 
 		/* cancel the URB, if it hasn't been cancelled already */
 		if (test_and_clear_bit(US_FLIDX_URB_ACTIVE, &us->flags)) {
@@ -440,7 +440,7 @@ int usb_stor_bulk_transfer_sglist(struct us_data *us, unsigned int pipe,
 	int result;
 
 	/* don't submit s-g requests during abort/disconnect processing */
-	if (us->flags & DONT_SUBMIT)
+	if (us->flags & ABORTING_OR_DISCONNECTING)
 		return USB_STOR_XFER_ERROR;
 
 	/* initialize the scatter-gather request block */
@@ -458,7 +458,7 @@ int usb_stor_bulk_transfer_sglist(struct us_data *us, unsigned int pipe,
 	set_bit(US_FLIDX_SG_ACTIVE, &us->flags);
 
 	/* did an abort/disconnect occur during the submission? */
-	if (us->flags & DONT_SUBMIT) {
+	if (us->flags & ABORTING_OR_DISCONNECTING) {
 
 		/* cancel the request, if it hasn't been cancelled already */
 		if (test_and_clear_bit(US_FLIDX_SG_ACTIVE, &us->flags)) {
@@ -714,12 +714,8 @@ void usb_stor_invoke_transport(Scsi_Cmnd *srb, struct us_data *us)
 	 * following an abort */
   Handle_Abort:
 	srb->result = DID_ABORT << 16;
-	if (us->protocol == US_PR_BULK) {
-
-		/* permit the reset transfer to take place */
-		clear_bit(US_FLIDX_ABORTING, &us->flags);
+	if (us->protocol == US_PR_BULK)
 		us->transport_reset(us);
-	}
 }
 
 /* Stop the current URB transfer */
@@ -1079,20 +1075,28 @@ static int usb_stor_reset_common(struct us_data *us,
 {
 	int result;
 	int result2;
+	int rc = FAILED;
 
-	/* Let the SCSI layer know we are doing a reset */
+	/* Let the SCSI layer know we are doing a reset, set the
+	 * RESETTING bit, and clear the ABORTING bit so that the reset
+	 * may proceed.
+	 */
+	scsi_lock(us->host);
 	usb_stor_report_device_reset(us);
+	set_bit(US_FLIDX_RESETTING, &us->flags);
+	clear_bit(US_FLIDX_ABORTING, &us->flags);
+	scsi_unlock(us->host);
 
 	/* A 20-second timeout may seem rather long, but a LaCie
 	 * StudioDrive USB2 device takes 16+ seconds to get going
-	 *  following a powerup or USB attach event. */
-
+	 * following a powerup or USB attach event.
+	 */
 	result = usb_stor_control_msg(us, us->send_ctrl_pipe,
 			request, requesttype, value, index, data, size,
 			20*HZ);
 	if (result < 0) {
 		US_DEBUGP("Soft reset failed: %d\n", result);
-		return FAILED;
+		goto Done;
 	}
 
 	/* long wait for reset, so unlock to allow disconnects */
@@ -1102,12 +1106,9 @@ static int usb_stor_reset_common(struct us_data *us,
 	down(&us->dev_semaphore);
 	if (test_bit(US_FLIDX_DISCONNECTING, &us->flags)) {
 		US_DEBUGP("Reset interrupted by disconnect\n");
-		return FAILED;
+		goto Done;
 	}
 
-	/* permit the clear-halt transfers to take place */
-	clear_bit(US_FLIDX_ABORTING, &us->flags);
-
 	US_DEBUGP("Soft reset: clearing bulk-in endpoint halt\n");
 	result = usb_stor_clear_halt(us, us->recv_bulk_pipe);
 
@@ -1117,10 +1118,14 @@ static int usb_stor_reset_common(struct us_data *us,
 	/* return a result code based on the result of the control message */
 	if (result < 0 || result2 < 0) {
 		US_DEBUGP("Soft reset failed\n");
-		return FAILED;
+		goto Done;
 	}
 	US_DEBUGP("Soft reset done\n");
-	return SUCCESS;
+	rc = SUCCESS;
+
+  Done:
+	clear_bit(US_FLIDX_RESETTING, &us->flags);
+	return rc;
 }
 
 /* This issues a CB[I] Reset to the device in question

drivers/usb/storage/usb.h

@@ -79,8 +79,9 @@ struct us_unusual_dev {
 #define US_FLIDX_SG_ACTIVE	19  /* 0x00080000  current_sg is in use   */
 #define US_FLIDX_ABORTING	20  /* 0x00100000  abort is in progress   */
 #define US_FLIDX_DISCONNECTING	21  /* 0x00200000  disconnect in progress */
-#define DONT_SUBMIT	((1UL << US_FLIDX_ABORTING) | \
+#define ABORTING_OR_DISCONNECTING	((1UL << US_FLIDX_ABORTING) | \
 					 (1UL << US_FLIDX_DISCONNECTING))
+#define US_FLIDX_RESETTING	22  /* 0x00400000  device reset in progress */
 
 
 /* processing state machine states */