Commit 2f67cd5b authored by Alan Stern's avatar Alan Stern Committed by Greg Kroah-Hartman

usb-storage: fix bugs in the disconnect pathway

This patch (as961) fixes a couple of bugs in the disconnect pathway of
usb-storage.

The first problem, which apparently has been around for a while
although nobody noticed it, shows up when an aborted command is still
pending when a disconnect occurs.  The SCSI error-handler will
continue to wait in command_abort() until the us->notify completion is
signalled.  Thus quiesce_and_remove_host() needs to signal it.

The second problem was introduced recently along with autosuspend
support.  Since usb_stor_scan_thread() now calls
usb_autopm_put_interface() before exiting, we can't simply leave the
scanning thread running after a disconnect; we must wait until the
thread exits.  This is solved by adding a new struct completion to the
private data structure.  Fortuitously, it allows the removal of the
rather clunky mechanism used in the past to insure that all threads
have finished before the module is unloaded.
Signed-off-by: default avatarAlan Stern <stern@rowland.harvard.edu>
CC: Matthew Dharm <mdharm-usb@one-eyed-alien.net>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@suse.de>
parent fa0de2b6
...@@ -112,13 +112,6 @@ module_param(delay_use, uint, S_IRUGO | S_IWUSR); ...@@ -112,13 +112,6 @@ module_param(delay_use, uint, S_IRUGO | S_IWUSR);
MODULE_PARM_DESC(delay_use, "seconds to delay before using a new device"); MODULE_PARM_DESC(delay_use, "seconds to delay before using a new device");
/* These are used to make sure the module doesn't unload before all the
* threads have exited.
*/
static atomic_t total_threads = ATOMIC_INIT(0);
static DECLARE_COMPLETION(threads_gone);
/* /*
* The entries in this table correspond, line for line, * The entries in this table correspond, line for line,
* with the entries of us_unusual_dev_list[]. * with the entries of us_unusual_dev_list[].
...@@ -879,9 +872,6 @@ static void quiesce_and_remove_host(struct us_data *us) ...@@ -879,9 +872,6 @@ static void quiesce_and_remove_host(struct us_data *us)
usb_stor_stop_transport(us); usb_stor_stop_transport(us);
wake_up(&us->delay_wait); wake_up(&us->delay_wait);
/* It doesn't matter if the SCSI-scanning thread is still running.
* The thread will exit when it sees the DISCONNECTING flag. */
/* queuecommand won't accept any new commands and the control /* queuecommand won't accept any new commands and the control
* thread won't execute a previously-queued command. If there * thread won't execute a previously-queued command. If there
* is such a command pending, complete it with an error. */ * is such a command pending, complete it with an error. */
...@@ -891,12 +881,16 @@ static void quiesce_and_remove_host(struct us_data *us) ...@@ -891,12 +881,16 @@ static void quiesce_and_remove_host(struct us_data *us)
scsi_lock(host); scsi_lock(host);
us->srb->scsi_done(us->srb); us->srb->scsi_done(us->srb);
us->srb = NULL; us->srb = NULL;
complete(&us->notify); /* in case of an abort */
scsi_unlock(host); scsi_unlock(host);
} }
mutex_unlock(&us->dev_mutex); mutex_unlock(&us->dev_mutex);
/* Now we own no commands so it's safe to remove the SCSI host */ /* Now we own no commands so it's safe to remove the SCSI host */
scsi_remove_host(host); scsi_remove_host(host);
/* Wait for the SCSI-scanning thread to stop */
wait_for_completion(&us->scanning_done);
} }
/* Second stage of disconnect processing: deallocate all resources */ /* Second stage of disconnect processing: deallocate all resources */
...@@ -947,9 +941,8 @@ static int usb_stor_scan_thread(void * __us) ...@@ -947,9 +941,8 @@ static int usb_stor_scan_thread(void * __us)
/* Should we unbind if no devices were detected? */ /* Should we unbind if no devices were detected? */
} }
scsi_host_put(us_to_host(us));
usb_autopm_put_interface(us->pusb_intf); usb_autopm_put_interface(us->pusb_intf);
complete_and_exit(&threads_gone, 0); complete_and_exit(&us->scanning_done, 0);
} }
...@@ -984,6 +977,7 @@ static int storage_probe(struct usb_interface *intf, ...@@ -984,6 +977,7 @@ static int storage_probe(struct usb_interface *intf,
init_MUTEX_LOCKED(&(us->sema)); init_MUTEX_LOCKED(&(us->sema));
init_completion(&(us->notify)); init_completion(&(us->notify));
init_waitqueue_head(&us->delay_wait); init_waitqueue_head(&us->delay_wait);
init_completion(&us->scanning_done);
/* Associate the us_data structure with the USB device */ /* Associate the us_data structure with the USB device */
result = associate_dev(us, intf); result = associate_dev(us, intf);
...@@ -1033,11 +1027,6 @@ static int storage_probe(struct usb_interface *intf, ...@@ -1033,11 +1027,6 @@ static int storage_probe(struct usb_interface *intf,
goto BadDevice; goto BadDevice;
} }
/* Take a reference to the host for the scanning thread and
* count it among all the threads we have launched. Then
* start it up. */
scsi_host_get(us_to_host(us));
atomic_inc(&total_threads);
usb_autopm_get_interface(intf); /* dropped in the scanning thread */ usb_autopm_get_interface(intf); /* dropped in the scanning thread */
wake_up_process(th); wake_up_process(th);
...@@ -1104,16 +1093,6 @@ static void __exit usb_stor_exit(void) ...@@ -1104,16 +1093,6 @@ static void __exit usb_stor_exit(void)
US_DEBUGP("-- calling usb_deregister()\n"); US_DEBUGP("-- calling usb_deregister()\n");
usb_deregister(&usb_storage_driver) ; usb_deregister(&usb_storage_driver) ;
/* Don't return until all of our control and scanning threads
* have exited. Since each thread signals threads_gone as its
* last act, we have to call wait_for_completion the right number
* of times.
*/
while (atomic_read(&total_threads) > 0) {
wait_for_completion(&threads_gone);
atomic_dec(&total_threads);
}
usb_usual_clear_present(USB_US_TYPE_STOR); usb_usual_clear_present(USB_US_TYPE_STOR);
} }
......
...@@ -150,6 +150,7 @@ struct us_data { ...@@ -150,6 +150,7 @@ struct us_data {
struct semaphore sema; /* to sleep thread on */ struct semaphore sema; /* to sleep thread on */
struct completion notify; /* thread begin/end */ struct completion notify; /* thread begin/end */
wait_queue_head_t delay_wait; /* wait during scan, reset */ wait_queue_head_t delay_wait; /* wait during scan, reset */
struct completion scanning_done; /* wait for scan thread */
/* subdriver information */ /* subdriver information */
void *extra; /* Any extra data */ void *extra; /* Any extra data */
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment