aboutsummaryrefslogtreecommitdiffstats
path: root/drivers
diff options
context:
space:
mode:
authorMatthew Dharm <mdharm-usb@one-eyed-alien.net>2005-08-25 23:03:50 -0400
committerGreg Kroah-Hartman <gregkh@suse.de>2005-09-12 15:23:50 -0400
commit226173edae1c49c68ebb723771a02302c85e3475 (patch)
treef1cad01bb076253a9fbb0ef29ddb688b6743ea82 /drivers
parentb789696af8b4102b7cc26dec30c2c51ce51ee18b (diff)
[PATCH] USB: storage: Fix messed-up locking
This is patch as550 from Alan Stern. Apparently someone changed the SCSI core so that it no longer holds the host lock when doing a device or bus reset. usb-storage was updated at the time, but the change was done carelessly. Some of the code depends on that lock being held. This patch reintroduces the host lock where needed and tries to clarify the comments explaining why the lock is necessary. It also moves the code that clears the TIMED_OUT and ABORTING bitflags so that it executes as soon as the timed-out command has completed (and while the host lock is held). Signed-off-by: Alan Stern <stern@rowland.harvard.edu> Signed-off-by: Matthew Dharm <mdharm-usb@one-eyed-alien.net> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
Diffstat (limited to 'drivers')
-rw-r--r--drivers/usb/storage/scsiglue.c20
-rw-r--r--drivers/usb/storage/usb.c11
2 files changed, 17 insertions, 14 deletions
diff --git a/drivers/usb/storage/scsiglue.c b/drivers/usb/storage/scsiglue.c
index d34dc9f417f0..4837524eada7 100644
--- a/drivers/usb/storage/scsiglue.c
+++ b/drivers/usb/storage/scsiglue.c
@@ -227,42 +227,42 @@ static int queuecommand(struct scsi_cmnd *srb,
227 ***********************************************************************/ 227 ***********************************************************************/
228 228
229/* Command timeout and abort */ 229/* Command timeout and abort */
230/* This is always called with scsi_lock(host) held */
231static int command_abort(struct scsi_cmnd *srb) 230static int command_abort(struct scsi_cmnd *srb)
232{ 231{
233 struct us_data *us = host_to_us(srb->device->host); 232 struct us_data *us = host_to_us(srb->device->host);
234 233
235 US_DEBUGP("%s called\n", __FUNCTION__); 234 US_DEBUGP("%s called\n", __FUNCTION__);
236 235
236 /* us->srb together with the TIMED_OUT, RESETTING, and ABORTING
237 * bits are protected by the host lock. */
238 scsi_lock(us_to_host(us));
239
237 /* Is this command still active? */ 240 /* Is this command still active? */
238 if (us->srb != srb) { 241 if (us->srb != srb) {
242 scsi_unlock(us_to_host(us));
239 US_DEBUGP ("-- nothing to abort\n"); 243 US_DEBUGP ("-- nothing to abort\n");
240 return FAILED; 244 return FAILED;
241 } 245 }
242 246
243 /* Set the TIMED_OUT bit. Also set the ABORTING bit, but only if 247 /* Set the TIMED_OUT bit. Also set the ABORTING bit, but only if
244 * a device reset isn't already in progress (to avoid interfering 248 * a device reset isn't already in progress (to avoid interfering
245 * with the reset). To prevent races with auto-reset, we must 249 * with the reset). Note that we must retain the host lock while
246 * stop any ongoing USB transfers while still holding the host 250 * calling usb_stor_stop_transport(); otherwise it might interfere
247 * lock. */ 251 * with an auto-reset that begins as soon as we release the lock. */
248 set_bit(US_FLIDX_TIMED_OUT, &us->flags); 252 set_bit(US_FLIDX_TIMED_OUT, &us->flags);
249 if (!test_bit(US_FLIDX_RESETTING, &us->flags)) { 253 if (!test_bit(US_FLIDX_RESETTING, &us->flags)) {
250 set_bit(US_FLIDX_ABORTING, &us->flags); 254 set_bit(US_FLIDX_ABORTING, &us->flags);
251 usb_stor_stop_transport(us); 255 usb_stor_stop_transport(us);
252 } 256 }
257 scsi_unlock(us_to_host(us));
253 258
254 /* Wait for the aborted command to finish */ 259 /* Wait for the aborted command to finish */
255 wait_for_completion(&us->notify); 260 wait_for_completion(&us->notify);
256
257 /* Reacquire the lock and allow USB transfers to resume */
258 clear_bit(US_FLIDX_ABORTING, &us->flags);
259 clear_bit(US_FLIDX_TIMED_OUT, &us->flags);
260 return SUCCESS; 261 return SUCCESS;
261} 262}
262 263
263/* This invokes the transport reset mechanism to reset the state of the 264/* This invokes the transport reset mechanism to reset the state of the
264 * device */ 265 * device */
265/* This is always called with scsi_lock(host) held */
266static int device_reset(struct scsi_cmnd *srb) 266static int device_reset(struct scsi_cmnd *srb)
267{ 267{
268 struct us_data *us = host_to_us(srb->device->host); 268 struct us_data *us = host_to_us(srb->device->host);
@@ -279,7 +279,6 @@ static int device_reset(struct scsi_cmnd *srb)
279} 279}
280 280
281/* Simulate a SCSI bus reset by resetting the device's USB port. */ 281/* Simulate a SCSI bus reset by resetting the device's USB port. */
282/* This is always called with scsi_lock(host) held */
283static int bus_reset(struct scsi_cmnd *srb) 282static int bus_reset(struct scsi_cmnd *srb)
284{ 283{
285 struct us_data *us = host_to_us(srb->device->host); 284 struct us_data *us = host_to_us(srb->device->host);
@@ -291,7 +290,6 @@ static int bus_reset(struct scsi_cmnd *srb)
291 result = usb_stor_port_reset(us); 290 result = usb_stor_port_reset(us);
292 up(&(us->dev_semaphore)); 291 up(&(us->dev_semaphore));
293 292
294 /* lock the host for the return */
295 return result < 0 ? FAILED : SUCCESS; 293 return result < 0 ? FAILED : SUCCESS;
296} 294}
297 295
diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
index cb4c770baf32..f9a9bfa1aef5 100644
--- a/drivers/usb/storage/usb.c
+++ b/drivers/usb/storage/usb.c
@@ -392,11 +392,16 @@ SkipForAbort:
392 /* If an abort request was received we need to signal that 392 /* If an abort request was received we need to signal that
393 * the abort has finished. The proper test for this is 393 * the abort has finished. The proper test for this is
394 * the TIMED_OUT flag, not srb->result == DID_ABORT, because 394 * the TIMED_OUT flag, not srb->result == DID_ABORT, because
395 * a timeout/abort request might be received after all the 395 * the timeout might have occurred after the command had
396 * USB processing was complete. */ 396 * already completed with a different result code. */
397 if (test_bit(US_FLIDX_TIMED_OUT, &us->flags)) 397 if (test_bit(US_FLIDX_TIMED_OUT, &us->flags)) {
398 complete(&(us->notify)); 398 complete(&(us->notify));
399 399
400 /* Allow USB transfers to resume */
401 clear_bit(US_FLIDX_ABORTING, &us->flags);
402 clear_bit(US_FLIDX_TIMED_OUT, &us->flags);
403 }
404
400 /* finished working on this command */ 405 /* finished working on this command */
401 us->srb = NULL; 406 us->srb = NULL;
402 scsi_unlock(host); 407 scsi_unlock(host);