# This is a BitKeeper generated patch for the following project: # Project Name: Linux kernel tree # This patch format is intended for GNU patch command version 2.5 or higher. # This patch includes the following deltas: # ChangeSet 1.564.2.1 -> 1.564.2.2 # drivers/usb/storage/usb.c 1.36 -> 1.37 # drivers/usb/storage/transport.c 1.30 -> 1.31 # drivers/usb/storage/usb.h 1.17 -> 1.18 # # The following is the BitKeeper ChangeSet Log # -------------------------------------------- # 02/09/16 mdharm-usb@one-eyed-alien.net 1.564.2.2 # [PATCH] USB storage: remove tests against EINPROGRESS # # This patch removes tests of urb->status for EINPROGRESS. As was pointed # out, that's not such a good idea, for a variety of reasons. # # In the process, a semaphore became useless. # -------------------------------------------- # diff -Nru a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c --- a/drivers/usb/storage/transport.c Mon Sep 16 15:01:08 2002 +++ b/drivers/usb/storage/transport.c Mon Sep 16 15:01:08 2002 @@ -370,16 +370,20 @@ * as those occurring during device-specific initialization, must be handled * by a separate code path.) * - * The abort function first sets the machine state, then acquires the lock - * on the current_urb before checking if it needs to be aborted. + * The abort function first sets the machine state, then atomically + * tests-and-clears the CAN_CANCEL bit in us->flags to see if the current_urb + * needs to be aborted. * - * When a function submits the current_urb, it must first grab the - * current_urb_sem to prevent the abort function from trying to cancel the - * URB while the submit call is underway. After a function submits the - * current_urb, it -MUST- test the state to see if we got aborted just before - * the submission. If so, it's essential to abort the URB if it's still in - * progress. Either way, the function must then release the lock and wait - * for the URB to finish. + * The submit function first verifies that the submission completed without + * errors, and only then sets the CAN_CANCEL bit. This prevents the abort + * function from trying to cancel the URB while the submit call is underway. + * Next, the submit function must test the state to see if we got aborted + * before the submission or before setting the CAN_CANCEL bit. If so, it's + * essential to abort the URB if it hasn't been cancelled already (i.e., + * if the CAN_CANCEL bit is still set). Either way, the function must then + * wait for the URB to finish. Note that because the USB_ASYNC_UNLINK flag + * is set, the URB can still be in progress even after a call to + * usb_unlink_urb() returns. * * (It's also permissible, but not necessary, to test the state -before- * submitting the URB. Doing so would prevent an unnecessary submission if @@ -389,7 +393,7 @@ * * The idea is that (1) once the state is changed to ABORTING, either the * aborting function or the submitting function is guaranteed to call - * usb_unlink_urb() for an active URB, and (2) current_urb_sem prevents + * usb_unlink_urb() for an active URB, and (2) test_and_clear_bit() prevents * usb_unlink_urb() from being called more than once or from being called * during usb_submit_urb(). */ @@ -424,28 +428,30 @@ us->current_urb->error_count = 0; us->current_urb->transfer_flags = USB_ASYNC_UNLINK; - /* lock and submit the URB */ - down(&(us->current_urb_sem)); + /* submit the URB */ status = usb_submit_urb(us->current_urb, GFP_NOIO); if (status) { /* something went wrong */ - up(&(us->current_urb_sem)); return status; } + /* since the URB has been submitted successfully, it's now okay + * to cancel it */ + set_bit(US_FLIDX_CAN_CANCEL, &us->flags); + /* has the current command been aborted? */ if (atomic_read(&us->sm_state) == US_STATE_ABORTING) { /* cancel the URB, if it hasn't been cancelled already */ - if (us->current_urb->status == -EINPROGRESS) { + if (test_and_clear_bit(US_FLIDX_CAN_CANCEL, &us->flags)) { US_DEBUGP("-- cancelling URB\n"); usb_unlink_urb(us->current_urb); } } - up(&(us->current_urb_sem)); /* wait for the completion of the URB */ wait_for_completion(&urb_done); + clear_bit(US_FLIDX_CAN_CANCEL, &us->flags); /* return the URB status */ return us->current_urb->status; @@ -867,15 +873,13 @@ /* If the state machine is blocked waiting for an URB or an IRQ, * let's wake it up */ - /* If we have an URB pending, cancel it. Note that we guarantee with - * the current_urb_sem that if a URB has just been submitted, it + /* If we have an URB pending, cancel it. The test_and_clear_bit() + * call guarantees that if a URB has just been submitted, it * won't be cancelled more than once. */ - down(&(us->current_urb_sem)); - if (us->current_urb->status == -EINPROGRESS) { + if (test_and_clear_bit(US_FLIDX_CAN_CANCEL, &us->flags)) { US_DEBUGP("-- cancelling URB\n"); usb_unlink_urb(us->current_urb); } - up(&(us->current_urb_sem)); /* If we are waiting for an IRQ, simulate it */ if (test_bit(US_FLIDX_IP_WANTED, &us->flags)) { diff -Nru a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c --- a/drivers/usb/storage/usb.c Mon Sep 16 15:01:08 2002 +++ b/drivers/usb/storage/usb.c Mon Sep 16 15:01:08 2002 @@ -825,7 +825,6 @@ init_completion(&(ss->notify)); init_MUTEX_LOCKED(&(ss->ip_waitq)); init_MUTEX(&(ss->irq_urb_sem)); - init_MUTEX(&(ss->current_urb_sem)); init_MUTEX_LOCKED(&(ss->dev_semaphore)); /* copy over the subclass and protocol data */ diff -Nru a/drivers/usb/storage/usb.h b/drivers/usb/storage/usb.h --- a/drivers/usb/storage/usb.h Mon Sep 16 15:01:08 2002 +++ b/drivers/usb/storage/usb.h Mon Sep 16 15:01:08 2002 @@ -106,6 +106,7 @@ #define US_FL_DEV_ATTACHED 0x00010000 /* is the device attached? */ #define US_FLIDX_IP_WANTED 17 /* 0x00020000 is an IRQ expected? */ +#define US_FLIDX_CAN_CANCEL 18 /* 0x00040000 okay to cancel current_urb? */ /* processing state machine states */ @@ -177,7 +178,6 @@ unsigned char irqdata[2]; /* data from USB IRQ */ /* control and bulk communications data */ - struct semaphore current_urb_sem; /* protect current_urb */ struct urb *current_urb; /* non-int USB requests */ struct usb_ctrlrequest *dr; /* control requests */