On Sun, May 30, 2004 at 11:21:58AM -0400, Alan Stern wrote: > If the device _isn't_ messed up then the outside request and the reset > request can't interfere with each other, because they both go to ep0 and > hence are automatically serialized. The only problem we need to guard > against is an outside request on ep0 arriving during a regular SCSI > command -- so we only need to lock the device while sending regular > commands.
> I guess there's no real harm in reusing ->serialize for this purpose. > (After all, I'm going to do the same sort of thing with the hub driver, > replacing its private semaphore with ->serialize.) So long as the lock is > held only during regular commands and not during resets there shouldn't be > a problem. > P.S.: Vojtech, part of the patch you submitted is already out-of-date! > devio.c has already been changed to lock the device during any nontrivial > operation. So all that needs to be added are the locks in usb-storage. Okay, so can we all agree on this one? -- Vojtech Pavlik SuSE Labs, SuSE CR
[EMAIL PROTECTED], 2004-05-30 18:12:53+02:00, [EMAIL PROTECTED] usb: Based on a 2.4 patch from [EMAIL PROTECTED], this patch serializes usb storage and usbfs operation, so that usbfs cannot disturb storage by seemingly harmless control reads. Signed-off-by: Vojtech Pavlik <[EMAIL PROTECTED]> transport.c | 11 +++++++++++ 1 files changed, 11 insertions(+) diff -Nru a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c --- a/drivers/usb/storage/transport.c 2004-05-30 18:13:33 +02:00 +++ b/drivers/usb/storage/transport.c 2004-05-30 18:13:33 +02:00 @@ -527,9 +527,18 @@ int need_auto_sense; int result; + /* + * Grab device's serialize mutex to prevent /usbfs and others from + * sending out a command in the middle of ours (if libusb sends a + * get_descriptor or something on pipe 0 after our CBW and before + * our CSW, and then we get a stall, we have trouble) + */ + /* send the command to the transport layer */ + down(&(us->pusb_dev->serialize)); srb->resid = 0; result = us->transport(srb, us); + up(&(us->pusb_dev->serialize)); /* if the command gets aborted by the higher layers, we need to * short-circuit all other processing @@ -648,9 +657,11 @@ srb->serial_number ^= 0x80000000; /* issue the auto-sense command */ + down(&(us->pusb_dev->serialize)); old_resid = srb->resid; srb->resid = 0; temp_result = us->transport(us->srb, us); + up(&(us->pusb_dev->serialize)); /* let's clean up right away */ srb->resid = old_resid; Signed-off-by: Vojtech Pavlik <[EMAIL PROTECTED]>