ChangeSet 1.1413, 2003/09/02 16:50:21-07:00, baldrick@wanadoo.fr [PATCH] USB: fix uhci "host controller process error" By the way, let me explain what the problem was with uhci-hcd. The usb hardware directly accesses your computers memory. The bug is that it could still be accessing a bit of memory after uhci-hcd thought it had finished with it and freed up the memory. This bug has always existed, and I guess led to occasional mysterious data corruption, when some other part of the kernel started using that bit of memory while the usb hardware was still playing with it. You turned on the "slab debugging" option, right? With this turned on, when uhci-hcd frees the memory it gets filled with some garbage values. The usb hardware reads this garbage and barfs, giving a "process error". In short, you can also get rid of the process error messages by turning off slab debugging, then the data corruption will be silent again! drivers/usb/host/uhci-hcd.c | 42 +++++++++++++++++++++++++++++++++++++++++- drivers/usb/host/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 Sep 5 17:59:44 2003 +++ b/drivers/usb/host/uhci-hcd.c Fri Sep 5 17:59:44 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); @@ -2207,6 +2242,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); @@ -2418,11 +2456,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 Sep 5 17:59:44 2003 +++ b/drivers/usb/host/uhci-hcd.h Fri Sep 5 17:59:44 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;