[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!

diff --git a/drivers/usb/host/uhci-hcd.c b/drivers/usb/host/uhci-hcd.c
index 5811fd1ae415d234e96993cace87f66b63ddf2a5..b1cb550cbb476e4d1eaeb53cc45c4c4cb8e61b63 100644 (file)
--- a/drivers/usb/host/uhci-hcd.c
+++ b/drivers/usb/host/uhci-hcd.c
@@ -156,6 +156,7 @@ static struct uhci_td *uhci_alloc_td(struct uhci_hcd *uhci, struct usb_device *d
        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 @@ static void uhci_free_td(struct uhci_hcd *uhci, struct uhci_td *td)
 {
        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 @@ static void uhci_destroy_urb_priv(struct uhci_hcd *uhci, struct urb *urb)
 {
        struct list_head *head, *tmp;
        struct urb_priv *urbp;
+       unsigned long flags;
 
        urbp = (struct urb_priv *)urb->hcpriv;
        if (!urbp)
@@ -713,6 +717,13 @@ static void uhci_destroy_urb_priv(struct uhci_hcd *uhci, struct urb *urb)
        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 @@ static void uhci_destroy_urb_priv(struct uhci_hcd *uhci, struct urb *urb)
 
                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 @@ static void uhci_free_pending_qhs(struct uhci_hcd *uhci)
        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 @@ static void uhci_irq(struct usb_hcd *hcd, struct pt_regs *regs)
 
        uhci_free_pending_qhs(uhci);
 
+       uhci_free_pending_tds(uhci);
+
        uhci_remove_pending_qhs(uhci);
 
        uhci_clear_next_interrupt(uhci);
@@ -2207,6 +2242,9 @@ static int uhci_start(struct usb_hcd *hcd)
        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 @@ static void uhci_stop(struct usb_hcd *hcd)
         * 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 --git a/drivers/usb/host/uhci-hcd.h b/drivers/usb/host/uhci-hcd.h
index 0024d239a57f8154f303ca209d0082f0a228767e..1df7ea1f8aeb2b2331fbafff07b3a0e8be56f5f7 100644 (file)
--- a/drivers/usb/host/uhci-hcd.h
+++ b/drivers/usb/host/uhci-hcd.h
@@ -190,6 +190,7 @@ struct uhci_td {
        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 */
@@ -350,6 +351,10 @@ struct uhci_hcd {
        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;
        struct list_head urb_remove_list;       /* P: uhci->urb_remove_list_lock */