ChangeSet 1.2173, 2004/11/19 09:39:04-08:00, [EMAIL PROTECTED]
[PATCH] USB Storage: Remove unnecessary state testing
This patch started as as405 from Alan Stern. It has been re-generated
against the current tip of the BK tree.
For quite a while we've had a bunch of state-transition testing code in the
driver, to report if anything bad ever happens (like the SCSI midlayer
trying to queue a second command before the first one finishes). None of
those tests triggered in a very long time; this aspect of the code appears
to be extremely stable.
So this patch removes all those tests for illegal values of us->sm_state.
It turns out that sm_state was used only for one other purpose: to check
whether a command had timed out and caused a SCSI abort. That piece of
information can easily be stored in a single new bitflag (which is called
calling US_FLIDX_TIMED_OUT) and doing so makes us->sm_state completely
unused. Hence the patch removes it from the structure.
Signed-off-by: Alan Stern <[EMAIL PROTECTED]>
Signed-off-by: Matthew Dharm <[EMAIL PROTECTED]>
Signed-off-by: Greg Kroah-Hartman <[EMAIL PROTECTED]>
drivers/usb/storage/isd200.c | 4 +--
drivers/usb/storage/scsiglue.c | 44 ++++++++--------------------------------
drivers/usb/storage/transport.c | 4 +--
drivers/usb/storage/usb.c | 17 +++++----------
drivers/usb/storage/usb.h | 8 -------
5 files changed, 20 insertions(+), 57 deletions(-)
diff -Nru a/drivers/usb/storage/isd200.c b/drivers/usb/storage/isd200.c
--- a/drivers/usb/storage/isd200.c 2004-11-19 11:42:27 -08:00
+++ b/drivers/usb/storage/isd200.c 2004-11-19 11:42:27 -08:00
@@ -555,7 +555,7 @@
/* if the command gets aborted by the higher layers, we need to
* short-circuit all other processing
*/
- if (us->sm_state == US_STATE_ABORTING) {
+ if (test_bit(US_FLIDX_TIMED_OUT, &us->flags)) {
US_DEBUGP("-- command was aborted\n");
goto Handle_Abort;
}
@@ -602,7 +602,7 @@
if (need_auto_sense) {
result = isd200_read_regs(us);
- if (us->sm_state == US_STATE_ABORTING) {
+ if (test_bit(US_FLIDX_TIMED_OUT, &us->flags)) {
US_DEBUGP("-- auto-sense aborted\n");
goto Handle_Abort;
}
diff -Nru a/drivers/usb/storage/scsiglue.c b/drivers/usb/storage/scsiglue.c
--- a/drivers/usb/storage/scsiglue.c 2004-11-19 11:42:27 -08:00
+++ b/drivers/usb/storage/scsiglue.c 2004-11-19 11:42:27 -08:00
@@ -173,10 +173,9 @@
srb->host_scribble = (unsigned char *)us;
/* check for state-transition errors */
- if (us->sm_state != US_STATE_IDLE || us->srb != NULL) {
- printk(KERN_ERR USB_STORAGE "Error in %s: "
- "state = %d, us->srb = %p\n",
- __FUNCTION__, us->sm_state, us->srb);
+ if (us->srb != NULL) {
+ printk(KERN_ERR USB_STORAGE "Error in %s: us->srb = %p\n",
+ __FUNCTION__, us->srb);
return SCSI_MLQUEUE_HOST_BUSY;
}
@@ -200,7 +199,7 @@
* Error handling functions
***********************************************************************/
-/* Command abort */
+/* Command timeout and abort */
/* This is always called with scsi_lock(srb->host) held */
static int command_abort(struct scsi_cmnd *srb )
{
@@ -215,22 +214,12 @@
return FAILED;
}
- /* Normally the current state is RUNNING. If the control thread
- * hasn't even started processing this command, the state will be
- * IDLE. Anything else is a bug. */
- if (us->sm_state != US_STATE_RUNNING
- && us->sm_state != US_STATE_IDLE) {
- printk(KERN_ERR USB_STORAGE "Error in %s: "
- "invalid state %d\n", __FUNCTION__, us->sm_state);
- return FAILED;
- }
-
- /* Set state to ABORTING and set the ABORTING bit, but only if
+ /* Set the TIMED_OUT bit. Also 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;
+ set_bit(US_FLIDX_TIMED_OUT, &us->flags);
if (!test_bit(US_FLIDX_RESETTING, &us->flags)) {
set_bit(US_FLIDX_ABORTING, &us->flags);
usb_stor_stop_transport(us);
@@ -243,6 +232,7 @@
/* Reacquire the lock and allow USB transfers to resume */
scsi_lock(host);
clear_bit(US_FLIDX_ABORTING, &us->flags);
+ clear_bit(US_FLIDX_TIMED_OUT, &us->flags);
return SUCCESS;
}
@@ -255,14 +245,7 @@
int result;
US_DEBUGP("%s called\n", __FUNCTION__);
- if (us->sm_state != US_STATE_IDLE) {
- printk(KERN_ERR USB_STORAGE "Error in %s: "
- "invalid state %d\n", __FUNCTION__, us->sm_state);
- return FAILED;
- }
- /* set the state and release the lock */
- us->sm_state = US_STATE_RESETTING;
scsi_unlock(srb->device->host);
/* lock the device pointers and do the reset */
@@ -274,9 +257,8 @@
result = us->transport_reset(us);
up(&(us->dev_semaphore));
- /* lock access to the state and clear it */
+ /* lock the host for the return */
scsi_lock(srb->device->host);
- us->sm_state = US_STATE_IDLE;
return result;
}
@@ -290,14 +272,7 @@
int result, rc;
US_DEBUGP("%s called\n", __FUNCTION__);
- if (us->sm_state != US_STATE_IDLE) {
- printk(KERN_ERR USB_STORAGE "Error in %s: "
- "invalid state %d\n", __FUNCTION__, us->sm_state);
- return FAILED;
- }
- /* set the state and release the lock */
- us->sm_state = US_STATE_RESETTING;
scsi_unlock(srb->device->host);
/* The USB subsystem doesn't handle synchronisation between
@@ -325,9 +300,8 @@
}
up(&(us->dev_semaphore));
- /* lock access to the state and clear it */
+ /* lock the host for the return */
scsi_lock(srb->device->host);
- us->sm_state = US_STATE_IDLE;
return result < 0 ? FAILED : SUCCESS;
}
diff -Nru a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c
--- a/drivers/usb/storage/transport.c 2004-11-19 11:42:27 -08:00
+++ b/drivers/usb/storage/transport.c 2004-11-19 11:42:27 -08:00
@@ -537,7 +537,7 @@
/* if the command gets aborted by the higher layers, we need to
* short-circuit all other processing
*/
- if (us->sm_state == US_STATE_ABORTING) {
+ if (test_bit(US_FLIDX_TIMED_OUT, &us->flags)) {
US_DEBUGP("-- command was aborted\n");
goto Handle_Abort;
}
@@ -665,7 +665,7 @@
srb->cmd_len = old_cmd_len;
memcpy(srb->cmnd, old_cmnd, MAX_COMMAND_SIZE);
- if (us->sm_state == US_STATE_ABORTING) {
+ if (test_bit(US_FLIDX_TIMED_OUT, &us->flags)) {
US_DEBUGP("-- auto-sense aborted\n");
goto Handle_Abort;
}
diff -Nru a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
--- a/drivers/usb/storage/usb.c 2004-11-19 11:42:27 -08:00
+++ b/drivers/usb/storage/usb.c 2004-11-19 11:42:27 -08:00
@@ -318,8 +318,8 @@
/* lock access to the state */
scsi_lock(host);
- /* has the command been aborted *already* ? */
- if (us->sm_state == US_STATE_ABORTING) {
+ /* has the command timed out *already* ? */
+ if (test_bit(US_FLIDX_TIMED_OUT, &us->flags)) {
us->srb->result = DID_ABORT << 16;
goto SkipForAbort;
}
@@ -330,8 +330,6 @@
goto SkipForDisconnect;
}
- /* set the state and release the lock */
- us->sm_state = US_STATE_RUNNING;
scsi_unlock(host);
/* reject the command if the direction indicator
@@ -392,16 +390,15 @@
/* If an abort request was received we need to signal that
* the abort has finished. The proper test for this is
- * sm_state == US_STATE_ABORTING, not srb->result == DID_ABORT,
- * because an abort request might be received after all the
+ * the TIMED_OUT flag, not srb->result == DID_ABORT, because
+ * a timeout/abort request might be received after all the
* USB processing was complete. */
- if (us->sm_state == US_STATE_ABORTING)
+ if (test_bit(US_FLIDX_TIMED_OUT, &us->flags))
complete(&(us->notify));
- /* empty the queue, reset the state, and release the lock */
+ /* finished working on this command */
SkipForDisconnect:
us->srb = NULL;
- us->sm_state = US_STATE_IDLE;
scsi_unlock(host);
/* unlock the device pointers */
@@ -801,7 +798,6 @@
us->host->hostdata[0] = (unsigned long) us;
/* Start up our control thread */
- us->sm_state = US_STATE_IDLE;
p = kernel_thread(usb_stor_control_thread, us, CLONE_VM);
if (p < 0) {
printk(KERN_WARNING USB_STORAGE
@@ -829,7 +825,6 @@
/* Wait for the thread to be idle */
down(&us->dev_semaphore);
US_DEBUGP("-- sending exit command to thread\n");
- BUG_ON(us->sm_state != US_STATE_IDLE);
/* If the SCSI midlayer queued a final command just before
* scsi_remove_host() was called, us->srb might not be
diff -Nru a/drivers/usb/storage/usb.h b/drivers/usb/storage/usb.h
--- a/drivers/usb/storage/usb.h 2004-11-19 11:42:27 -08:00
+++ b/drivers/usb/storage/usb.h 2004-11-19 11:42:27 -08:00
@@ -83,14 +83,9 @@
#define ABORTING_OR_DISCONNECTING ((1UL << US_FLIDX_ABORTING) | \
(1UL << US_FLIDX_DISCONNECTING))
#define US_FLIDX_RESETTING 22 /* 0x00400000 device reset in progress */
+#define US_FLIDX_TIMED_OUT 23 /* 0x00800000 SCSI midlayer timed out */
-/* processing state machine states */
-#define US_STATE_IDLE 1
-#define US_STATE_RUNNING 2
-#define US_STATE_RESETTING 3
-#define US_STATE_ABORTING 4
-
#define USB_STOR_STRING_LEN 32
/*
@@ -148,7 +143,6 @@
/* thread information */
int pid; /* control thread */
- int sm_state; /* what we are doing */
/* control and bulk communications data */
struct urb *current_urb; /* USB requests */
-------------------------------------------------------
This SF.Net email is sponsored by: InterSystems CACHE
FREE OODBMS DOWNLOAD - A multidimensional database that combines
robust object and relational technologies, making it a perfect match
for Java, C++,COM, XML, ODBC and JDBC. www.intersystems.com/match8
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
