Pete -- This is basically the same thing you sent me earlier, modulo my notes regarding some of the proposed changes, yes?
I was actually assuming you'd create something like this and circulate it
before sending to Marcelo, but hey.... I'm not really complaining.
It looks fine to me. Tho you really should run this by Greg KH, at least
so it's in his notes.....
Matt
On Sun, Apr 18, 2004 at 08:55:38PM -0700, Pete Zaitcev wrote:
> This is what I cooked for lockups in Fedora. Seems to work fine there.
>
> It also makes storage not to reset devices with several interfaces.
> This innovation comes from 2.6.
>
> [I cannot figure if Matt Dharm approves or not, he was kinda quiet lately,
> but I think it's a good patch and it has some testing on it, so no offense
> hopefuly.]
>
> -- Pete
>
> diff -urN -X dontdiff linux-2.4.26/drivers/usb/storage/scsiglue.c
> linux-2.4.26-nip/drivers/usb/storage/scsiglue.c
> --- linux-2.4.26/drivers/usb/storage/scsiglue.c 2004-04-14 17:33:16.000000000
> -0700
> +++ linux-2.4.26-nip/drivers/usb/storage/scsiglue.c 2004-04-18 20:30:27.000000000
> -0700
> @@ -218,7 +218,14 @@
> US_DEBUGP("device_reset() called\n" );
>
> spin_unlock_irq(&io_request_lock);
> + down(&(us->dev_semaphore));
> + if (!us->pusb_dev) {
> + up(&(us->dev_semaphore));
> + spin_lock_irq(&io_request_lock);
> + return SUCCESS;
> + }
> rc = us->transport_reset(us);
> + up(&(us->dev_semaphore));
> spin_lock_irq(&io_request_lock);
> return rc;
> }
> @@ -235,27 +242,44 @@
> /* we use the usb_reset_device() function to handle this for us */
> US_DEBUGP("bus_reset() called\n");
>
> + spin_unlock_irq(&io_request_lock);
> +
> + down(&(us->dev_semaphore));
> +
> /* if the device has been removed, this worked */
> if (!us->pusb_dev) {
> US_DEBUGP("-- device removed already\n");
> + up(&(us->dev_semaphore));
> + spin_lock_irq(&io_request_lock);
> return SUCCESS;
> }
>
> - spin_unlock_irq(&io_request_lock);
> + /* The USB subsystem doesn't handle synchronisation between
> + * a device's several drivers. Therefore we reset only devices
> + * with just one interface, which we of course own. */
> + if (us->pusb_dev->actconfig->bNumInterfaces != 1) {
> + printk(KERN_NOTICE "usb-storage: "
> + "Refusing to reset a multi-interface device\n");
> + up(&(us->dev_semaphore));
> + spin_lock_irq(&io_request_lock);
> + /* XXX Don't just return success, make sure current cmd fails */
> + return SUCCESS;
> + }
>
> /* release the IRQ, if we have one */
> - down(&(us->irq_urb_sem));
> if (us->irq_urb) {
> US_DEBUGP("-- releasing irq URB\n");
> result = usb_unlink_urb(us->irq_urb);
> US_DEBUGP("-- usb_unlink_urb() returned %d\n", result);
> }
> - up(&(us->irq_urb_sem));
>
> /* attempt to reset the port */
> if (usb_reset_device(us->pusb_dev) < 0) {
> - spin_lock_irq(&io_request_lock);
> - return FAILED;
> + /*
> + * Do not return errors, or else the error handler might
> + * invoke host_reset, which is not implemented.
> + */
> + goto bail_out;
> }
>
> /* FIXME: This needs to lock out driver probing while it's working
> @@ -286,17 +310,18 @@
> up(&intf->driver->serialize);
> }
>
> +bail_out:
> /* re-allocate the IRQ URB and submit it to restore connectivity
> * for CBI devices
> */
> if (us->protocol == US_PR_CBI) {
> - down(&(us->irq_urb_sem));
> us->irq_urb->dev = us->pusb_dev;
> result = usb_submit_urb(us->irq_urb);
> US_DEBUGP("usb_submit_urb() returns %d\n", result);
> - up(&(us->irq_urb_sem));
> }
> -
> +
> + up(&(us->dev_semaphore));
> +
> spin_lock_irq(&io_request_lock);
>
> US_DEBUGP("bus_reset() complete\n");
>
> diff -urN -X dontdiff linux-2.4.26/drivers/usb/storage/usb.c
> linux-2.4.26-nip/drivers/usb/storage/usb.c
> --- linux-2.4.26/drivers/usb/storage/usb.c 2004-02-26 14:09:59.000000000 -0800
> +++ linux-2.4.26-nip/drivers/usb/storage/usb.c 2004-04-14 18:13:39.000000000
> -0700
> @@ -501,6 +501,9 @@
> * strucuture is current. This includes the ep_int field, which gives us
> * the endpoint for the interrupt.
> * Returns non-zero on failure, zero on success
> + *
> + * ss->dev_semaphore is expected taken, except for a newly minted,
> + * unregistered device.
> */
> static int usb_stor_allocate_irq(struct us_data *ss)
> {
> @@ -510,13 +513,9 @@
>
> US_DEBUGP("Allocating IRQ for CBI transport\n");
>
> - /* lock access to the data structure */
> - down(&(ss->irq_urb_sem));
> -
> /* allocate the URB */
> ss->irq_urb = usb_alloc_urb(0);
> if (!ss->irq_urb) {
> - up(&(ss->irq_urb_sem));
> US_DEBUGP("couldn't allocate interrupt URB");
> return 1;
> }
> @@ -537,12 +536,9 @@
> US_DEBUGP("usb_submit_urb() returns %d\n", result);
> if (result) {
> usb_free_urb(ss->irq_urb);
> - up(&(ss->irq_urb_sem));
> return 2;
> }
>
> - /* unlock the data structure and return success */
> - up(&(ss->irq_urb_sem));
> return 0;
> }
>
> @@ -772,7 +768,6 @@
> init_completion(&(ss->notify));
> init_MUTEX_LOCKED(&(ss->ip_waitq));
> spin_lock_init(&(ss->queue_exclusion));
> - init_MUTEX(&(ss->irq_urb_sem));
> init_MUTEX(&(ss->current_urb_sem));
> init_MUTEX(&(ss->dev_semaphore));
>
> @@ -1063,7 +1058,6 @@
> down(&(ss->dev_semaphore));
>
> /* release the IRQ, if we have one */
> - down(&(ss->irq_urb_sem));
> if (ss->irq_urb) {
> US_DEBUGP("-- releasing irq URB\n");
> result = usb_unlink_urb(ss->irq_urb);
> @@ -1071,7 +1065,6 @@
> usb_free_urb(ss->irq_urb);
> ss->irq_urb = NULL;
> }
> - up(&(ss->irq_urb_sem));
>
> /* free up the main URB for this device */
> US_DEBUGP("-- releasing main URB\n");
> diff -urN -X dontdiff linux-2.4.26/drivers/usb/storage/usb.h
> linux-2.4.26-nip/drivers/usb/storage/usb.h
> --- linux-2.4.26/drivers/usb/storage/usb.h 2003-11-29 19:23:15.000000000 -0800
> +++ linux-2.4.26-nip/drivers/usb/storage/usb.h 2004-04-18 17:25:19.000000000
> -0700
> @@ -116,7 +116,7 @@
> struct us_data *next; /* next device */
>
> /* the device we're working with */
> - struct semaphore dev_semaphore; /* protect pusb_dev */
> + struct semaphore dev_semaphore; /* protect many things */
> struct usb_device *pusb_dev; /* this usb_device */
>
> unsigned int flags; /* from filter initially */
> @@ -162,7 +162,6 @@
> atomic_t ip_wanted[1]; /* is an IRQ expected? */
>
> /* interrupt communications data */
> - struct semaphore irq_urb_sem; /* to protect irq_urb */
> struct urb *irq_urb; /* for USB int requests */
> unsigned char irqbuf[2]; /* buffer for USB IRQ */
> unsigned char irqdata[2]; /* data from USB IRQ */
--
Matthew Dharm Home: [EMAIL PROTECTED]
Maintainer, Linux USB Mass Storage Driver
C: Like the Furby?
DP: He gives me the creeps. Think the SPCA will take him?
-- Cobb and Dust Puppy
User Friendly, 1/2/1999
pgp00000.pgp
Description: PGP signature
