> OK - I implemented this is in a quick and nasty way, and it works.
> (Though maybe I just changed the timing enough to not hit the race
> anymore...) It looks like Alan's analysis was right on target :)
> I'll try to send a clean fix next weekend.
Hi Alan, Johannes, how about this? I tried to make as few changes to the
existing code as possible. In my opinion it would make more sense to have
the list of TDs belong to the QH and not the urb_priv, and free them when the
QH is freed, but that would mean making many more and more invasive changes.
By the way, does the last
+ uhci_free_pending_tds(uhci);
make sense? If so, should there be a
+ uhci_remove_pending_qhs(uhci);
as well?
Since I'm going on holiday, could one of you please forward the patch
to Greg with your blessing if you think its OK.
Thanks,
Duncan.
uhci-hcd.c | 42 +++++++++++++++++++++++++++++++++++++++++-
uhci-hcd.h | 5 +++++
2 files changed, 46 insertions(+), 1 deletion(-)
diff -Nru a/drivers/usb/host/uhci-hcd.c b/drivers/usb/host/uhci-hcd.c
--- a/drivers/usb/host/uhci-hcd.c Fri Jul 18 13:22:32 2003
+++ b/drivers/usb/host/uhci-hcd.c Fri Jul 18 13:22:32 2003
@@ -156,6 +156,7 @@
td->dev = dev;
INIT_LIST_HEAD(&td->list);
+ INIT_LIST_HEAD(&td->remove_list);
INIT_LIST_HEAD(&td->fl_list);
usb_get_dev(dev);
@@ -286,6 +287,8 @@
{
if (!list_empty(&td->list))
dbg("td %p is still in list!", td);
+ if (!list_empty(&td->remove_list))
+ dbg("td %p still in remove_list!", td);
if (!list_empty(&td->fl_list))
dbg("td %p is still in fl_list!", td);
@@ -702,6 +705,7 @@
{
struct list_head *head, *tmp;
struct urb_priv *urbp;
+ unsigned long flags;
urbp = (struct urb_priv *)urb->hcpriv;
if (!urbp)
@@ -713,6 +717,13 @@
if (!list_empty(&urbp->complete_list))
warn("uhci_destroy_urb_priv: urb %p still on uhci->complete_list",
urb);
+ spin_lock_irqsave(&uhci->td_remove_list_lock, flags);
+
+ /* Check to see if the remove list is empty. Set the IOC bit */
+ /* to force an interrupt so we can remove the TD's*/
+ if (list_empty(&uhci->td_remove_list))
+ uhci_set_next_interrupt(uhci);
+
head = &urbp->td_list;
tmp = head->next;
while (tmp != head) {
@@ -722,9 +733,11 @@
uhci_remove_td_from_urb(td);
uhci_remove_td(uhci, td);
- uhci_free_td(uhci, td);
+ list_add(&td->remove_list, &uhci->td_remove_list);
}
+ spin_unlock_irqrestore(&uhci->td_remove_list_lock, flags);
+
urb->hcpriv = NULL;
kmem_cache_free(uhci_up_cachep, urbp);
}
@@ -1801,6 +1814,26 @@
spin_unlock_irqrestore(&uhci->qh_remove_list_lock, flags);
}
+static void uhci_free_pending_tds(struct uhci_hcd *uhci)
+{
+ struct list_head *tmp, *head;
+ unsigned long flags;
+
+ spin_lock_irqsave(&uhci->td_remove_list_lock, flags);
+ head = &uhci->td_remove_list;
+ tmp = head->next;
+ while (tmp != head) {
+ struct uhci_td *td = list_entry(tmp, struct uhci_td, remove_list);
+
+ tmp = tmp->next;
+
+ list_del_init(&td->remove_list);
+
+ uhci_free_td(uhci, td);
+ }
+ spin_unlock_irqrestore(&uhci->td_remove_list_lock, flags);
+}
+
static void uhci_finish_urb(struct usb_hcd *hcd, struct urb *urb, struct pt_regs
*regs)
{
struct urb_priv *urbp = (struct urb_priv *)urb->hcpriv;
@@ -1899,6 +1932,8 @@
uhci_free_pending_qhs(uhci);
+ uhci_free_pending_tds(uhci);
+
uhci_remove_pending_qhs(uhci);
uhci_clear_next_interrupt(uhci);
@@ -2200,6 +2235,9 @@
spin_lock_init(&uhci->qh_remove_list_lock);
INIT_LIST_HEAD(&uhci->qh_remove_list);
+ spin_lock_init(&uhci->td_remove_list_lock);
+ INIT_LIST_HEAD(&uhci->td_remove_list);
+
spin_lock_init(&uhci->urb_remove_list_lock);
INIT_LIST_HEAD(&uhci->urb_remove_list);
@@ -2415,11 +2453,13 @@
* to this bus since there are no more parents
*/
uhci_free_pending_qhs(uhci);
+ uhci_free_pending_tds(uhci);
uhci_remove_pending_qhs(uhci);
reset_hc(uhci);
uhci_free_pending_qhs(uhci);
+ uhci_free_pending_tds(uhci);
release_uhci(uhci);
}
diff -Nru a/drivers/usb/host/uhci-hcd.h b/drivers/usb/host/uhci-hcd.h
--- a/drivers/usb/host/uhci-hcd.h Fri Jul 18 13:22:32 2003
+++ b/drivers/usb/host/uhci-hcd.h Fri Jul 18 13:22:32 2003
@@ -190,6 +190,7 @@
struct urb *urb;
struct list_head list; /* P: urb->lock */
+ struct list_head remove_list; /* P: uhci->td_remove_list_lock */
int frame; /* for iso: what frame? */
struct list_head fl_list; /* P: uhci->frame_list_lock */
@@ -349,6 +350,10 @@
/* List of QH's that are done, but waiting to be unlinked (race) */
spinlock_t qh_remove_list_lock;
struct list_head qh_remove_list; /* P: uhci->qh_remove_list_lock */
+
+ /* List of TD's that are done, but waiting to be freed (race) */
+ spinlock_t td_remove_list_lock;
+ struct list_head td_remove_list; /* P: uhci->td_remove_list_lock */
/* List of asynchronously unlinked URB's */
spinlock_t urb_remove_list_lock;
-------------------------------------------------------
This SF.net email is sponsored by: VM Ware
With VMware you can run multiple operating systems on a single machine.
WITHOUT REBOOTING! Mix Linux / Windows / Novell virtual machines at the
same time. Free trial click here: http://www.vmware.com/wl/offer/345/0
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
