> From: David Brownell <[EMAIL PROTECTED]>
> To: [EMAIL PROTECTED]
> Reply-To: [EMAIL PROTECTED]
> Date: Sun, 03 Jun 2001 23:32:47 -0700
> > I had to look at usb-uhci and locking in it once again;
> > this time I know what is wrong with David-B's "fix"
> > <http://marc.theaimsgroup.com/?l=linux-usb-devel&m=98580988401128&w=2>
>
> Hey, all I claimed to do was fix an oops I'd been hitting.
> Are you claiming I was imagining the oops, and then its
> removal? :)
No, on the contrary. You had a very real oops, and your fix
removed it ... in your particular case. But it was a wrong
fix for other cases.
Your fix blows up on ia64 and we are going to ship something
like this for the RH 7.1/ia64:
--- linux-2.4.5-0.2.10/drivers/usb/usb-uhci.c Thu May 31 22:05:13 2001
+++ linux-2.4.5-0.2.10-p3/drivers/usb/usb-uhci.c Sun Jun 3 18:03:17 2001
@@ -2708,20 +2713,19 @@
if (urb->complete) {
int was_unlinked = (urb->status == -ENOENT);
urb->dev = NULL;
+ spin_unlock(&urb->lock);
spin_unlock(&s->urb_list_lock);
urb->complete ((struct urb *) urb);
// Re-submit the URB if ring-linked
if (is_ring && !was_unlinked && !contains_killed) {
urb->dev=usb_dev;
uhci_submit_urb (urb);
- } else
- urb = 0;
+ }
spin_lock(&s->urb_list_lock);
- }
-
- usb_dec_dev_use (usb_dev);
- if (urb)
+ } else
spin_unlock(&urb->lock);
+
+ usb_dec_dev_use (usb_dev);
}
}
I would like people to test this. It is not radical enough
for my taste, but release stability is above all.
> > I suggest we get rid of any instances of urb->lock in usb-uhci.
> > It does not serve any useful purpose, and is a terrible bug
> > generator.
>
> The "hcd" layer (used right now only for EHCI) uses that
> lock when it's switching urbs between the generic layer
> (what the USB spec calls the "USBD" ... think "usbcore")
> and the HCD. [...]
> So I'm not quite sure that urb->lock can really disappear.
> Though I'd agree that it's one of several chunks of urb data
> that aren't clearly enough specified.
I do not propose to remove urb->lock, not at all!
Sometimes I wonder if I need to take speech communication
classes - nobody understands what I am saying.
The urb->lock is used intelligently in uhci.c,
but usb-uhci does not use it at all. Everywhere
usb-uhci asserts urb->lock, s->urb_list_lock is
taken already, so urb->lock is totally redundant.
[Side note: process_urb is exception, but
1. it violates lock order by dropping and taking urb_list_lock;
2. urb->lock does not do anything useful there anyways;]
Therefore, I propose, for the 2.4 cycle, to remove
*references* of urb->lock from usb-uhci, but to leave
the structure member for use in more clever drivers.
-- Pete
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
http://lists.sourceforge.net/lists/listinfo/linux-usb-devel
