On Fri, 18 Jul 2003, Duncan Sands wrote:
> 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?
Duncan:
I looked at your suggested patch; it seems like it ought to work.
I'm trying a different approach. It's a little more complicated, but it
doesn't need an extra list header added to each TD. The idea is not to
free the urbp as soon as the URB is given back, but to wait until
uhci_free_pending_qhs() has released the QH as well.
Here's the patch. Please try it and let me know if it fixes that
Controller Halted problem. (You will probably have to use a little fuzz
when applying it, because I wrote it on top of a couple of other patches.)
Alan Stern
===== uhci-hcd.c 1.55 vs edited =====
--- 1.55/drivers/usb/host/uhci-hcd.c Tue Jul 22 11:19:19 2003
+++ edited/drivers/usb/host/uhci-hcd.c Wed Jul 23 16:45:35 2003
@@ -403,7 +403,6 @@
/*
* Start removal of QH from schedule; it finishes next frame.
- * TDs should be unlinked before this is called.
*/
static void uhci_remove_qh(struct uhci_hcd *uhci, struct uhci_qh *qh)
{
@@ -413,8 +412,6 @@
if (!qh)
return;
- qh->urbp = NULL;
-
/*
* Only go through the hoops if it's actually linked in
* Queued QHs are removed in uhci_delete_queued_urb,
@@ -666,6 +663,7 @@
list_add_tail(&urbp->urb_list, &uhci->urb_list);
urb->hcpriv = urbp;
+ atomic_set(&urbp->refcount, 1);
return urbp;
}
@@ -696,22 +694,20 @@
}
/*
- * MUST be called with urb->lock acquired
+ * MUST be called with urbp->urb == NULL
*/
-static void uhci_destroy_urb_priv(struct uhci_hcd *uhci, struct urb *urb)
+static void uhci_destroy_urb_priv(struct uhci_hcd *uhci, struct urb_priv *urbp)
{
struct list_head *head, *tmp;
- struct urb_priv *urbp;
- urbp = (struct urb_priv *)urb->hcpriv;
if (!urbp)
return;
if (!list_empty(&urbp->urb_list))
- warn("uhci_destroy_urb_priv: urb %p still on uhci->urb_list or
uhci->remove_list", urb);
+ warn("uhci_destroy_urb_priv: urbp %p still on uhci->urb_list or
uhci->remove_list", urbp);
if (!list_empty(&urbp->complete_list))
- warn("uhci_destroy_urb_priv: urb %p still on uhci->complete_list",
urb);
+ warn("uhci_destroy_urb_priv: urbp %p still on uhci->complete_list",
urbp);
head = &urbp->td_list;
tmp = head->next;
@@ -725,7 +721,6 @@
uhci_free_td(uhci, td);
}
- urb->hcpriv = NULL;
kmem_cache_free(uhci_up_cachep, urbp);
}
@@ -891,6 +886,7 @@
urbp->qh = qh;
qh->urbp = urbp;
+ atomic_inc(&urbp->refcount);
uhci_insert_tds_in_qh(qh, urb, UHCI_PTR_BREADTH);
@@ -926,6 +922,7 @@
urbp->short_control_packet = 1;
/* Create a new QH to avoid pointer overwriting problems */
+ urbp->qh->urbp = NULL;
uhci_remove_qh(uhci, urbp->qh);
/* Delete all of the TD's except for the status TD at the end */
@@ -943,6 +940,7 @@
urbp->qh = uhci_alloc_qh(uhci, urb->dev);
if (!urbp->qh) {
+ atomic_dec(&urbp->refcount);
err("unable to allocate new QH for control retrigger");
return -ENOMEM;
}
@@ -1150,6 +1148,7 @@
urbp->qh = qh;
qh->urbp = urbp;
+ atomic_inc(&urbp->refcount);
/* Always breadth first */
uhci_insert_tds_in_qh(qh, urb, UHCI_PTR_BREADTH);
@@ -1491,7 +1490,9 @@
list_del_init(&urbp->urb_list);
spin_unlock_irqrestore(&uhci->urb_list_lock, flags);
- uhci_destroy_urb_priv (uhci, urb);
+ urb->hcpriv = NULL;
+ urbp->urb = NULL;
+ uhci_destroy_urb_priv (uhci, urbp);
return ret;
}
@@ -1630,9 +1631,8 @@
uhci_delete_queued_urb(uhci, urb);
- /* The interrupt loop will reclaim the QH's */
+ /* The interrupt loop will reclaim the QH's and TD's */
uhci_remove_qh(uhci, urbp->qh);
- urbp->qh = NULL;
}
static int uhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb)
@@ -1781,19 +1781,24 @@
static void uhci_free_pending_qhs(struct uhci_hcd *uhci)
{
- struct list_head *tmp, *head;
+ struct list_head *head;
unsigned long flags;
spin_lock_irqsave(&uhci->qh_remove_list_lock, flags);
head = &uhci->qh_remove_list;
- tmp = head->next;
- while (tmp != head) {
- struct uhci_qh *qh = list_entry(tmp, struct uhci_qh, remove_list);
-
- tmp = tmp->next;
+ while (!list_empty(head)) {
+ struct uhci_qh *qh = list_entry(head->next,
+ struct uhci_qh, remove_list);
+ struct urb_priv *urbp = qh->urbp;
list_del_init(&qh->remove_list);
-
+ if (urbp && atomic_dec_and_test(&urbp->refcount)) {
+ qh->urbp = NULL;
+ urbp->qh = NULL;
+ spin_unlock_irqrestore(&uhci->qh_remove_list_lock, flags);
+ uhci_destroy_urb_priv(uhci, urbp);
+ spin_lock_irqsave(&uhci->qh_remove_list_lock, flags);
+ }
uhci_free_qh(uhci, qh);
}
spin_unlock_irqrestore(&uhci->qh_remove_list_lock, flags);
@@ -1802,10 +1807,14 @@
static void uhci_finish_urb(struct usb_hcd *hcd, struct urb *urb, struct pt_regs
*regs)
{
struct uhci_hcd *uhci = hcd_to_uhci(hcd);
+ struct urb_priv *urbp = (struct urb_priv *) urb->hcpriv;
unsigned long flags;
spin_lock_irqsave(&urb->lock, flags);
- uhci_destroy_urb_priv(uhci, urb);
+ urbp->urb = NULL;
+ urb->hcpriv = NULL;
+ if (atomic_dec_and_test(&urbp->refcount))
+ uhci_destroy_urb_priv(uhci, urbp);
spin_unlock_irqrestore(&urb->lock, flags);
usb_hcd_giveback_urb(hcd, urb, regs);
@@ -1814,24 +1823,21 @@
static void uhci_finish_completion(struct usb_hcd *hcd, struct pt_regs *regs)
{
struct uhci_hcd *uhci = hcd_to_uhci(hcd);
- struct list_head *tmp, *head;
+ struct list_head *head;
unsigned long flags;
spin_lock_irqsave(&uhci->complete_list_lock, flags);
head = &uhci->complete_list;
- tmp = head->next;
- while (tmp != head) {
- struct urb_priv *urbp = list_entry(tmp, struct urb_priv,
complete_list);
- struct urb *urb = urbp->urb;
+ while (!list_empty(head)) {
+ struct urb_priv *urbp = list_entry(head->next,
+ struct urb_priv, complete_list);
list_del_init(&urbp->complete_list);
spin_unlock_irqrestore(&uhci->complete_list_lock, flags);
- uhci_finish_urb(hcd, urb, regs);
+ uhci_finish_urb(hcd, urbp->urb, regs);
spin_lock_irqsave(&uhci->complete_list_lock, flags);
- head = &uhci->complete_list;
- tmp = head->next;
}
spin_unlock_irqrestore(&uhci->complete_list_lock, flags);
}
===== uhci-hcd.h 1.15 vs edited =====
--- 1.15/drivers/usb/host/uhci-hcd.h Tue Jul 22 11:26:32 2003
+++ edited/drivers/usb/host/uhci-hcd.h Wed Jul 23 16:29:16 2003
@@ -382,6 +382,7 @@
unsigned long inserttime; /* In jiffies */
unsigned long fsbrtime; /* In jiffies */
+ atomic_t refcount; /* 1 for URB, 1 for QH */
struct list_head queue_list; /* P: uhci->frame_list_lock */
struct list_head complete_list; /* P: uhci->complete_list_lock */
-------------------------------------------------------
This SF.Net email sponsored by: Free pre-built ASP.NET sites including
Data Reports, E-commerce, Portals, and Forums are available now.
Download today and enter to win an XBOX or Visual Studio .NET.
http://aspnet.click-url.com/go/psa00100003ave/direct;at.aspnet_072303_01/01
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
