On Wed, Feb 20, 2002 at 04:48:01PM -0800, Greg KH wrote:
>
> [EMAIL PROTECTED], 2002-02-20 16:00:37-08:00, [EMAIL PROTECTED]
> uhci.c didn't work well with USB storage. It would tend to stall
> relatively quickly and sometimes locked up the system. It usually only
> took me a couple of tries ripping a CD to reproduce the problem.
>
> I took a long hard look at the locking in uhci.c and decided to clean
> it up, fixing a couple of bugs along the way as well as documenting the
> locking strategy.
>
> With this patch applies, where I could only rip a CD a couple of times
> before causing problems, I was able to rip a CD 12,000 times in a row
> successfully, before I stopped it. Not a single error :)
>
> drivers/usb/uhci.c | 156 ++++++++++++++++++++++++++++++++++-------------------
> drivers/usb/uhci.h | 60 +++++++++++++++-----
> 2 files changed, 146 insertions(+), 70 deletions(-)
>
>
> [EMAIL PROTECTED], 2002-02-20 15:59:27-08:00, [EMAIL PROTECTED]
> [PATCH] uhci.c, fix pci dma ordering issue
>
> There was a bug where we unmap the PCI DMA mapping and then sync the
> data afterwards. This reverses the ordering as well as insures we don't
> unmap the region more than once.
>
> drivers/usb/uhci.c | 24 ++++++++++++++----------
> 1 files changed, 14 insertions(+), 10 deletions(-)
>
>
> [EMAIL PROTECTED], 2002-02-20 15:58:24-08:00, [EMAIL PROTECTED]
> [PATCH] uhci.c, interrupt unlink in completion
>
> This patch fixes a bug where an interrupt URB is unlinked in the drivers
> completion handler and we'll try to resubmit it anyway.
>
> drivers/usb/uhci.c | 15 +++++++++++----
> 1 files changed, 11 insertions(+), 4 deletions(-)
>
>
> [EMAIL PROTECTED], 2002-02-20 15:56:59-08:00, [EMAIL PROTECTED]
> [PATCH] uhci.c, one more toggle fix
>
> This patch fixes another toggle bug and reverts the previous bogus
> patch which caused compile warnings.
>
> It also adds a quick comment explaining the criteria.
>
> JE
>
> drivers/usb/uhci.c | 29 +++++++++++++++++++++++------
> 1 files changed, 23 insertions(+), 6 deletions(-)
# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
# ChangeSet 1.378 -> 1.379
# drivers/usb/uhci.c 1.24 -> 1.25
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 02/02/20 [EMAIL PROTECTED] 1.379
# [PATCH] uhci.c, one more toggle fix
#
# This patch fixes another toggle bug and reverts the previous bogus
# patch which caused compile warnings.
#
# It also adds a quick comment explaining the criteria.
#
# JE
# --------------------------------------------
#
diff -Nru a/drivers/usb/uhci.c b/drivers/usb/uhci.c
--- a/drivers/usb/uhci.c Wed Feb 20 16:55:40 2002
+++ b/drivers/usb/uhci.c Wed Feb 20 16:55:40 2002
@@ -1677,6 +1677,7 @@
{
struct list_head *head, *tmp;
struct urb_priv *urbp = urb->hcpriv;
+ int prevactive = 1;
/* We can get called when urbp allocation fails, so check */
if (!urbp)
@@ -1684,6 +1685,19 @@
uhci_dec_fsbr(uhci, urb); /* Safe since it checks */
+ /*
+ * Now we need to find out what the last successful toggle was
+ * so we can update the local data toggle for the next transfer
+ *
+ * There's 3 way's the last successful completed TD is found:
+ *
+ * 1) The TD is NOT active and the actual length < expected length
+ * 2) The TD is NOT active and it's the last TD in the chain
+ * 3) The TD is active and the previous TD is NOT active
+ *
+ * Control and Isochronous ignore the toggle, so this is safe
+ * for all types
+ */
head = &urbp->td_list;
tmp = head->next;
while (tmp != head) {
@@ -1691,15 +1705,18 @@
tmp = tmp->next;
- /* Control and Isochronous ignore the toggle, so this */
- /* is safe for all types */
- if ((!(td->status & TD_CTRL_ACTIVE) &&
- (uhci_actual_length(td->status) < uhci_expected_length(td->info))
||
- tmp == head)) {
+ if (!(td->status & TD_CTRL_ACTIVE) &&
+ (uhci_actual_length(td->status) < uhci_expected_length(td->info) ||
+ tmp == head))
usb_settoggle(urb->dev, uhci_endpoint(td->info),
uhci_packetout(td->info),
uhci_toggle(td->info) ^ 1);
- }
+ else if ((td->status & TD_CTRL_ACTIVE) && !prevactive)
+ usb_settoggle(urb->dev, uhci_endpoint(td->info),
+ uhci_packetout(td->info),
+ uhci_toggle(td->info));
+
+ prevactive = td->status & TD_CTRL_ACTIVE;
}
uhci_delete_queued_urb(uhci, urb);
# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
# ChangeSet 1.379 -> 1.380
# drivers/usb/uhci.c 1.25 -> 1.26
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 02/02/20 [EMAIL PROTECTED] 1.380
# [PATCH] uhci.c, interrupt unlink in completion
#
# This patch fixes a bug where an interrupt URB is unlinked in the drivers
# completion handler and we'll try to resubmit it anyway.
# --------------------------------------------
#
diff -Nru a/drivers/usb/uhci.c b/drivers/usb/uhci.c
--- a/drivers/usb/uhci.c Wed Feb 20 16:55:47 2002
+++ b/drivers/usb/uhci.c Wed Feb 20 16:55:47 2002
@@ -2263,7 +2263,7 @@
killed = (urb->status == -ENOENT || urb->status == -ECONNABORTED ||
urb->status == -ECONNRESET);
resubmit_interrupt = (usb_pipetype(urb->pipe) == PIPE_INTERRUPT &&
- urb->interval && !killed);
+ urb->interval);
nurb = urb->next;
if (nurb && !killed) {
@@ -2289,7 +2289,7 @@
}
status = urbp->status;
- if (!resubmit_interrupt)
+ if (!resubmit_interrupt || killed)
/* We don't need urb_priv anymore */
uhci_destroy_urb_priv(urb);
@@ -2306,10 +2306,17 @@
sizeof(struct usb_ctrlrequest), PCI_DMA_TODEVICE);
urb->dev = NULL;
- if (urb->complete)
+ if (urb->complete) {
urb->complete(urb);
- if (resubmit_interrupt) {
+ /* Recheck the status. The completion handler may have */
+ /* unlinked the resubmitting interrupt URB */
+ killed = (urb->status == -ENOENT ||
+ urb->status == -ECONNABORTED ||
+ urb->status == -ECONNRESET);
+ }
+
+ if (resubmit_interrupt && !killed) {
urb->dev = dev;
uhci_reset_interrupt(urb);
} else {
# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
# ChangeSet 1.380 -> 1.381
# drivers/usb/uhci.c 1.26 -> 1.27
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 02/02/20 [EMAIL PROTECTED] 1.381
# [PATCH] uhci.c, fix pci dma ordering issue
#
# There was a bug where we unmap the PCI DMA mapping and then sync the
# data afterwards. This reverses the ordering as well as insures we don't
# unmap the region more than once.
# --------------------------------------------
#
diff -Nru a/drivers/usb/uhci.c b/drivers/usb/uhci.c
--- a/drivers/usb/uhci.c Wed Feb 20 16:55:54 2002
+++ b/drivers/usb/uhci.c Wed Feb 20 16:55:54 2002
@@ -715,14 +715,18 @@
uhci_free_td(uhci, td);
}
- if (urbp->setup_packet_dma_handle)
+ if (urbp->setup_packet_dma_handle) {
pci_unmap_single(uhci->dev, urbp->setup_packet_dma_handle,
sizeof(struct usb_ctrlrequest), PCI_DMA_TODEVICE);
+ urbp->setup_packet_dma_handle = 0;
+ }
- if (urbp->transfer_buffer_dma_handle)
+ if (urbp->transfer_buffer_dma_handle) {
pci_unmap_single(uhci->dev, urbp->transfer_buffer_dma_handle,
urb->transfer_buffer_length, usb_pipein(urb->pipe) ?
PCI_DMA_FROMDEVICE : PCI_DMA_TODEVICE);
+ urbp->transfer_buffer_dma_handle = 0;
+ }
urb->hcpriv = NULL;
kmem_cache_free(uhci_up_cachep, urbp);
@@ -2288,14 +2292,6 @@
is_ring = (nurb == urb);
}
- status = urbp->status;
- if (!resubmit_interrupt || killed)
- /* We don't need urb_priv anymore */
- uhci_destroy_urb_priv(urb);
-
- if (!killed)
- urb->status = status;
-
if (urbp->transfer_buffer_dma_handle)
pci_dma_sync_single(uhci->dev, urbp->transfer_buffer_dma_handle,
urb->transfer_buffer_length, usb_pipein(urb->pipe) ?
@@ -2304,6 +2300,14 @@
if (urbp->setup_packet_dma_handle)
pci_dma_sync_single(uhci->dev, urbp->setup_packet_dma_handle,
sizeof(struct usb_ctrlrequest), PCI_DMA_TODEVICE);
+
+ status = urbp->status;
+ if (!resubmit_interrupt || killed)
+ /* We don't need urb_priv anymore */
+ uhci_destroy_urb_priv(urb);
+
+ if (!killed)
+ urb->status = status;
urb->dev = NULL;
if (urb->complete) {
# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
# ChangeSet 1.381 -> 1.382
# drivers/usb/uhci.h 1.6 -> 1.7
# drivers/usb/uhci.c 1.27 -> 1.28
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 02/02/20 [EMAIL PROTECTED] 1.382
# uhci.c didn't work well with USB storage. It would tend to stall
# relatively quickly and sometimes locked up the system. It usually only
# took me a couple of tries ripping a CD to reproduce the problem.
#
# I took a long hard look at the locking in uhci.c and decided to clean
# it up, fixing a couple of bugs along the way as well as documenting the
# locking strategy.
#
# With this patch applies, where I could only rip a CD a couple of times
# before causing problems, I was able to rip a CD 12,000 times in a row
# successfully, before I stopped it. Not a single error :)
# --------------------------------------------
#
diff -Nru a/drivers/usb/uhci.c b/drivers/usb/uhci.c
--- a/drivers/usb/uhci.c Wed Feb 20 16:56:03 2002
+++ b/drivers/usb/uhci.c Wed Feb 20 16:56:03 2002
@@ -4,7 +4,7 @@
* Maintainer: Johannes Erdfelt <[EMAIL PROTECTED]>
*
* (C) Copyright 1999 Linus Torvalds
- * (C) Copyright 1999-2001 Johannes Erdfelt, [EMAIL PROTECTED]
+ * (C) Copyright 1999-2002 Johannes Erdfelt, [EMAIL PROTECTED]
* (C) Copyright 1999 Randy Dunlap
* (C) Copyright 1999 Georg Acher, [EMAIL PROTECTED]
* (C) Copyright 1999 Deti Fliegl, [EMAIL PROTECTED]
@@ -240,10 +240,10 @@
unsigned long flags;
/* If it's not inserted, don't remove it */
+ spin_lock_irqsave(&uhci->frame_list_lock, flags);
if (td->frame == -1 && list_empty(&td->fl_list))
- return;
+ goto out;
- spin_lock_irqsave(&uhci->frame_list_lock, flags);
if (td->frame != -1 && uhci->fl->frame_cpu[td->frame] == td) {
if (list_empty(&td->fl_list)) {
uhci->fl->frame[td->frame] = td->link;
@@ -268,6 +268,7 @@
list_del_init(&td->fl_list);
td->frame = -1;
+out:
spin_unlock_irqrestore(&uhci->frame_list_lock, flags);
}
@@ -358,6 +359,9 @@
pci_pool_free(uhci->qh_pool, qh, qh->dma_handle);
}
+/*
+ * MUST be called with uhci->frame_list_lock acquired
+ */
static void _uhci_insert_qh(struct uhci *uhci, struct uhci_qh *skelqh, struct urb
*urb)
{
struct urb_priv *urbp = (struct urb_priv *)urb->hcpriv;
@@ -417,11 +421,10 @@
return;
/* Only go through the hoops if it's actually linked in */
+ spin_lock_irqsave(&uhci->frame_list_lock, flags);
if (!list_empty(&qh->list)) {
qh->urbp = NULL;
- spin_lock_irqsave(&uhci->frame_list_lock, flags);
-
pqh = list_entry(qh->list.prev, struct uhci_qh, list);
if (pqh->urbp) {
@@ -444,9 +447,8 @@
qh->element = qh->link = UHCI_PTR_TERM;
list_del_init(&qh->list);
-
- spin_unlock_irqrestore(&uhci->frame_list_lock, flags);
}
+ spin_unlock_irqrestore(&uhci->frame_list_lock, flags);
spin_lock_irqsave(&uhci->qh_remove_list_lock, flags);
@@ -658,6 +660,9 @@
return urbp;
}
+/*
+ * MUST be called with urb->lock acquired
+ */
static void uhci_add_td_to_urb(struct urb *urb, struct uhci_td *td)
{
struct urb_priv *urbp = (struct urb_priv *)urb->hcpriv;
@@ -667,6 +672,9 @@
list_add_tail(&td->list, &urbp->td_list);
}
+/*
+ * MUST be called with urb->lock acquired
+ */
static void uhci_remove_td_from_urb(struct uhci_td *td)
{
if (list_empty(&td->list))
@@ -677,22 +685,22 @@
td->urb = NULL;
}
+/*
+ * MUST be called with urb->lock acquired
+ */
static void uhci_destroy_urb_priv(struct urb *urb)
{
struct list_head *head, *tmp;
struct urb_priv *urbp;
struct uhci *uhci;
- unsigned long flags;
-
- spin_lock_irqsave(&urb->lock, flags);
urbp = (struct urb_priv *)urb->hcpriv;
if (!urbp)
- goto out;
+ return;
if (!urbp->dev || !urbp->dev->bus || !urbp->dev->bus->hcpriv) {
warn("uhci_destroy_urb_priv: urb %p belongs to disconnected device or
bus?", urb);
- goto out;
+ return;
}
if (!list_empty(&urb->urb_list))
@@ -730,9 +738,6 @@
urb->hcpriv = NULL;
kmem_cache_free(uhci_up_cachep, urbp);
-
-out:
- spin_unlock_irqrestore(&urb->lock, flags);
}
static void uhci_inc_fsbr(struct uhci *uhci, struct urb *urb)
@@ -876,7 +881,7 @@
* It's IN if the pipe is an output pipe or we're not expecting
* data back.
*/
- destination &= ~TD_PID;
+ destination &= ~TD_TOKEN_PID_MASK;
if (usb_pipeout(urb->pipe) || !urb->transfer_buffer_length)
destination |= USB_PID_IN;
else
@@ -1316,9 +1321,7 @@
struct uhci *uhci = (struct uhci *)urb->dev->bus->hcpriv;
struct list_head *tmp, *head;
int ret = 0;
- unsigned long flags;
- spin_lock_irqsave(&uhci->urb_list_lock, flags);
head = &uhci->urb_list;
tmp = head->next;
while (tmp != head) {
@@ -1341,8 +1344,6 @@
} else
ret = -1; /* no previous urb found */
- spin_unlock_irqrestore(&uhci->urb_list_lock, flags);
-
return ret;
}
@@ -1450,34 +1451,30 @@
return ret;
}
+/*
+ * MUST be called with uhci->urb_list_lock acquired
+ */
static struct urb *uhci_find_urb_ep(struct uhci *uhci, struct urb *urb)
{
struct list_head *tmp, *head;
- unsigned long flags;
- struct urb *u = NULL;
/* We don't match Isoc transfers since they are special */
if (usb_pipeisoc(urb->pipe))
return NULL;
- spin_lock_irqsave(&uhci->urb_list_lock, flags);
head = &uhci->urb_list;
tmp = head->next;
while (tmp != head) {
- u = list_entry(tmp, struct urb, urb_list);
+ struct urb *u = list_entry(tmp, struct urb, urb_list);
tmp = tmp->next;
if (u->dev == urb->dev && u->pipe == urb->pipe &&
u->status == -EINPROGRESS)
- goto out;
+ return u;
}
- u = NULL;
-
-out:
- spin_unlock_irqrestore(&uhci->urb_list_lock, flags);
- return u;
+ return NULL;
}
static int uhci_submit_urb(struct urb *urb, int mem_flags)
@@ -1504,13 +1501,15 @@
INIT_LIST_HEAD(&urb->urb_list);
usb_inc_dev_use(urb->dev);
- spin_lock_irqsave(&urb->lock, flags);
+ spin_lock_irqsave(&uhci->urb_list_lock, flags);
+ spin_lock(&urb->lock);
if (urb->status == -EINPROGRESS || urb->status == -ECONNRESET ||
urb->status == -ECONNABORTED) {
dbg("uhci_submit_urb: urb not available to submit (status = %d)",
urb->status);
/* Since we can have problems on the out path */
- spin_unlock_irqrestore(&urb->lock, flags);
+ spin_unlock(&urb->lock);
+ spin_unlock_irqrestore(&uhci->urb_list_lock, flags);
usb_dec_dev_use(urb->dev);
usb_put_urb(urb);
@@ -1580,18 +1579,21 @@
out:
urb->status = ret;
- spin_unlock_irqrestore(&urb->lock, flags);
-
if (ret == -EINPROGRESS) {
- spin_lock_irqsave(&uhci->urb_list_lock, flags);
/* We use _tail to make find_urb_ep more efficient */
list_add_tail(&urb->urb_list, &uhci->urb_list);
+
+ spin_unlock(&urb->lock);
spin_unlock_irqrestore(&uhci->urb_list_lock, flags);
return 0;
}
uhci_unlink_generic(uhci, urb);
+
+ spin_unlock(&urb->lock);
+ spin_unlock_irqrestore(&uhci->urb_list_lock, flags);
+
uhci_call_completion(urb);
return ret;
@@ -1600,7 +1602,7 @@
/*
* Return the result of a transfer
*
- * Must be called with urb_list_lock acquired
+ * MUST be called with urb_list_lock acquired
*/
static void uhci_transfer_result(struct uhci *uhci, struct urb *urb)
{
@@ -1639,10 +1641,10 @@
urbp->status = ret;
- spin_unlock_irqrestore(&urb->lock, flags);
-
- if (ret == -EINPROGRESS)
+ if (ret == -EINPROGRESS) {
+ spin_unlock_irqrestore(&urb->lock, flags);
return;
+ }
switch (usb_pipetype(urb->pipe)) {
case PIPE_CONTROL:
@@ -1672,11 +1674,17 @@
usb_pipetype(urb->pipe), urb);
}
+ /* Remove it from uhci->urb_list */
list_del_init(&urb->urb_list);
uhci_add_complete(urb);
+
+ spin_unlock_irqrestore(&urb->lock, flags);
}
+/*
+ * MUST be called with urb->lock acquired
+ */
static void uhci_unlink_generic(struct uhci *uhci, struct urb *urb)
{
struct list_head *head, *tmp;
@@ -1743,6 +1751,9 @@
uhci = (struct uhci *)urb->dev->bus->hcpriv;
+ spin_lock_irqsave(&uhci->urb_list_lock, flags);
+ spin_lock(&urb->lock);
+
/* Release bandwidth for Interrupt or Isoc. transfers */
/* Spinlock needed ? */
if (urb->bandwidth) {
@@ -1758,35 +1769,41 @@
}
}
- if (urb->status != -EINPROGRESS)
+ if (urb->status != -EINPROGRESS) {
+ spin_unlock(&urb->lock);
+ spin_unlock_irqrestore(&uhci->urb_list_lock, flags);
return 0;
+ }
- spin_lock_irqsave(&uhci->urb_list_lock, flags);
list_del_init(&urb->urb_list);
- spin_unlock_irqrestore(&uhci->urb_list_lock, flags);
uhci_unlink_generic(uhci, urb);
/* Short circuit the virtual root hub */
if (urb->dev == uhci->rh.dev) {
rh_unlink_urb(urb);
+
+ spin_unlock(&urb->lock);
+ spin_unlock_irqrestore(&uhci->urb_list_lock, flags);
+
uhci_call_completion(urb);
} else {
if (urb->transfer_flags & USB_ASYNC_UNLINK) {
- /* urb_list is available now since we called */
- /* uhci_unlink_generic already */
-
urbp->status = urb->status = -ECONNABORTED;
- spin_lock_irqsave(&uhci->urb_remove_list_lock, flags);
+ spin_lock(&uhci->urb_remove_list_lock);
- /* Check to see if the remove list is empty */
+ /* If we're the first, set the next interrupt bit */
if (list_empty(&uhci->urb_remove_list))
uhci_set_next_interrupt(uhci);
list_add(&urb->urb_list, &uhci->urb_remove_list);
- spin_unlock_irqrestore(&uhci->urb_remove_list_lock, flags);
+ spin_unlock(&uhci->urb_remove_list_lock);
+
+ spin_unlock(&urb->lock);
+ spin_unlock_irqrestore(&uhci->urb_list_lock, flags);
+
} else {
urb->status = -ENOENT;
@@ -1799,6 +1816,9 @@
} else
schedule_timeout(1+1*HZ/1000);
+ spin_unlock(&urb->lock);
+ spin_unlock_irqrestore(&uhci->urb_list_lock, flags);
+
uhci_call_completion(urb);
}
}
@@ -1932,12 +1952,14 @@
/* prepare Interrupt pipe transaction data; HUB INTERRUPT ENDPOINT */
static int rh_send_irq(struct urb *urb)
{
- int i, len = 1;
struct uhci *uhci = (struct uhci *)urb->dev->bus->hcpriv;
+ struct urb_priv *urbp = (struct urb_priv *)urb->hcpriv;
unsigned int io_addr = uhci->io_addr;
+ unsigned long flags;
+ int i, len = 1;
__u16 data = 0;
- struct urb_priv *urbp = (struct urb_priv *)urb->hcpriv;
+ spin_lock_irqsave(&urb->lock, flags);
for (i = 0; i < uhci->rh.numports; i++) {
data |= ((inw(io_addr + USBPORTSC1 + i * 2) & 0xa) > 0 ? (1 << (i +
1)) : 0);
len = (i + 1) / 8 + 1;
@@ -1947,6 +1969,8 @@
urb->actual_length = len;
urbp->status = 0;
+ spin_unlock_irqrestore(&urb->lock, flags);
+
if ((data > 0) && (uhci->rh.send != 0)) {
dbg("root-hub INT complete: port1: %x port2: %x data: %x",
inw(io_addr + USBPORTSC1), inw(io_addr + USBPORTSC2), data);
@@ -1973,7 +1997,6 @@
spin_lock_irqsave(&uhci->urb_list_lock, flags);
head = &uhci->urb_list;
-
tmp = head->next;
while (tmp != head) {
struct urb *u = list_entry(tmp, struct urb, urb_list);
@@ -1981,6 +2004,8 @@
tmp = tmp->next;
+ spin_lock(&urb->lock);
+
/* Check if the FSBR timed out */
if (urbp->fsbr && !urbp->fsbr_timeout && time_after_eq(jiffies,
urbp->fsbrtime + IDLE_TIMEOUT))
uhci_fsbr_timeout(uhci, u);
@@ -1990,6 +2015,8 @@
list_del(&u->urb_list);
list_add_tail(&u->urb_list, &list);
}
+
+ spin_unlock(&urb->lock);
}
spin_unlock_irqrestore(&uhci->urb_list_lock, flags);
@@ -2223,6 +2250,9 @@
return stat;
}
+/*
+ * MUST be called with urb->lock acquired
+ */
static int rh_unlink_urb(struct urb *urb)
{
struct uhci *uhci = (struct uhci *)urb->dev->bus->hcpriv;
@@ -2258,11 +2288,20 @@
static void uhci_call_completion(struct urb *urb)
{
- struct urb_priv *urbp = (struct urb_priv *)urb->hcpriv;
+ struct urb_priv *urbp;
struct usb_device *dev = urb->dev;
struct uhci *uhci = (struct uhci *)dev->bus->hcpriv;
int is_ring = 0, killed, resubmit_interrupt, status;
struct urb *nurb;
+ unsigned long flags;
+
+ spin_lock_irqsave(&urb->lock, flags);
+
+ urbp = (struct urb_priv *)urb->hcpriv;
+ if (!urbp || !urb->dev) {
+ spin_unlock_irqrestore(&urb->lock, flags);
+ return;
+ }
killed = (urb->status == -ENOENT || urb->status == -ECONNABORTED ||
urb->status == -ECONNRESET);
@@ -2310,6 +2349,8 @@
urb->status = status;
urb->dev = NULL;
+ spin_unlock_irqrestore(&urb->lock, flags);
+
if (urb->complete) {
urb->complete(urb);
@@ -2348,11 +2389,14 @@
struct urb_priv *urbp = list_entry(tmp, struct urb_priv,
complete_list);
struct urb *urb = urbp->urb;
- tmp = tmp->next;
-
list_del_init(&urbp->complete_list);
+ spin_unlock_irqrestore(&uhci->complete_list_lock, flags);
uhci_call_completion(urb);
+
+ spin_lock_irqsave(&uhci->complete_list_lock, flags);
+ head = &uhci->complete_list;
+ tmp = head->next;
}
spin_unlock_irqrestore(&uhci->complete_list_lock, flags);
}
@@ -2374,7 +2418,8 @@
list_del_init(&urb->urb_list);
urbp->status = urb->status = -ECONNRESET;
- uhci_call_completion(urb);
+
+ uhci_add_complete(urb);
}
spin_unlock_irqrestore(&uhci->urb_remove_list_lock, flags);
}
@@ -3107,3 +3152,4 @@
MODULE_AUTHOR(DRIVER_AUTHOR);
MODULE_DESCRIPTION(DRIVER_DESC);
MODULE_LICENSE("GPL");
+
diff -Nru a/drivers/usb/uhci.h b/drivers/usb/uhci.h
--- a/drivers/usb/uhci.h Wed Feb 20 16:56:03 2002
+++ b/drivers/usb/uhci.h Wed Feb 20 16:56:03 2002
@@ -121,15 +121,16 @@
* for TD <info>: (a.k.a. Token)
*/
#define TD_TOKEN_TOGGLE 19
-#define TD_PID 0xFF
+#define TD_TOKEN_PID_MASK 0xFF
+#define TD_TOKEN_EXPLEN_MASK 0x7FF /* expected length, encoded as n - 1 */
#define uhci_maxlen(token) ((token) >> 21)
-#define uhci_expected_length(info) (((info >> 21) + 1) & TD_CTRL_ACTLEN_MASK) /*
1-based */
+#define uhci_expected_length(info) (((info >> 21) + 1) & TD_TOKEN_EXPLEN_MASK) /*
+1-based */
#define uhci_toggle(token) (((token) >> TD_TOKEN_TOGGLE) & 1)
#define uhci_endpoint(token) (((token) >> 15) & 0xf)
#define uhci_devaddr(token) (((token) >> 8) & 0x7f)
#define uhci_devep(token) (((token) >> 8) & 0x7ff)
-#define uhci_packetid(token) ((token) & 0xff)
+#define uhci_packetid(token) ((token) & TD_TOKEN_PID_MASK)
#define uhci_packetout(token) (uhci_packetid(token) != USB_PID_IN)
#define uhci_packetin(token) (uhci_packetid(token) == USB_PID_IN)
@@ -163,7 +164,7 @@
struct list_head list; /* P: urb->lock */
int frame;
- struct list_head fl_list; /* P: frame_list_lock */
+ struct list_head fl_list; /* P: uhci->frame_list_lock */
} __attribute__((aligned(16)));
/*
@@ -306,21 +307,25 @@
struct uhci_qh *skelqh[UHCI_NUM_SKELQH]; /* Skeleton QH's */
spinlock_t frame_list_lock;
- struct uhci_frame_list *fl; /* Frame list */
+ struct uhci_frame_list *fl; /* P: uhci->frame_list_lock */
int fsbr; /* Full speed bandwidth reclamation */
int is_suspended;
+ /* Main list of URB's currently controlled by this HC */
+ spinlock_t urb_list_lock;
+ struct list_head urb_list; /* P: uhci->urb_list_lock */
+
+ /* 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;
+ struct list_head qh_remove_list; /* P: uhci->qh_remove_list_lock */
+ /* List of asynchronously unlinked URB's */
spinlock_t urb_remove_list_lock;
- struct list_head urb_remove_list;
-
- spinlock_t urb_list_lock;
- struct list_head urb_list;
+ struct list_head urb_remove_list; /* P: uhci->urb_remove_list_lock */
+ /* List of URB's awaiting completion callback */
spinlock_t complete_list_lock;
- struct list_head complete_list;
+ struct list_head complete_list; /* P: uhci->complete_list_lock */
struct virt_root_hub rh; /* private data of the virtual root hub */
};
@@ -333,7 +338,7 @@
dma_addr_t transfer_buffer_dma_handle;
struct uhci_qh *qh; /* QH for this URB */
- struct list_head td_list;
+ struct list_head td_list; /* P: urb->lock */
int fsbr : 1; /* URB turned on FSBR */
int fsbr_timeout : 1; /* URB timed out on FSBR */
@@ -345,11 +350,36 @@
int status; /* Final status */
unsigned long inserttime; /* In jiffies */
- unsigned long fsbrtime; /* In jiffies */
+ unsigned long fsbrtime; /* In jiffies */
- struct list_head queue_list;
- struct list_head complete_list;
+ struct list_head queue_list; /* P: uhci->frame_list_lock */
+ struct list_head complete_list; /* P: uhci->complete_list_lock */
};
+
+/*
+ * Locking in uhci.c
+ *
+ * spinlocks are used extensively to protect the many lists and data
+ * structures we have. It's not that pretty, but it's necessary. We
+ * need to be done with all of the locks (except complete_list_lock) when
+ * we call urb->complete. I've tried to make it simple enough so I don't
+ * have to spend hours racking my brain trying to figure out if the
+ * locking is safe.
+ *
+ * Here's the safe locking order to prevent deadlocks:
+ *
+ * #1 uhci->urb_list_lock
+ * #2 urb->lock
+ * #3 uhci->urb_remove_list_lock, uhci->frame_list_lock,
+ * uhci->qh_remove_list_lock
+ * #4 uhci->complete_list_lock
+ *
+ * If you're going to grab 2 or more locks at once, ALWAYS grab the lock
+ * at the lowest level FIRST and NEVER grab locks at the same level at the
+ * same time.
+ *
+ * So, if you need uhci->urb_list_lock, grab it before you grab urb->lock
+ */
/* -------------------------------------------------------------------------
Virtual Root HUB
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
