On 05/15/12 07:55pm, Oncaphillis wrote:
>On 05/16/2012 02:56 AM, Huajun Li wrote:
>>Thanks for your reminder, and here is the draft patch for review:
>
>Didn't do the trck. I patched 3.3.4 .. Should i use 3.4-rc7 ?
>
>From 3.3.4 to 3.4-rc7, devio.c has no much changes, maybe this is
not the root cause of your problem.
However, I think there still exists another race in destroy_async_on_interface(),
Care to try another patch, or could you please share a piece of codes which
may reproduce the issue?
===========================================================
diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
index 8df4b76..1c16d0a 100644
--- a/drivers/usb/core/devio.c
+++ b/drivers/usb/core/devio.c
@@ -64,6 +64,7 @@ struct dev_state {
struct usb_device *dev;
struct file *file;
spinlock_t lock; /* protects the async urb lists */
+ struct list_head async_intf;
struct list_head async_pending;
struct list_head async_completed;
wait_queue_head_t wait; /* wake up if a request completed */
@@ -333,17 +334,14 @@ static struct async *async_getcompleted(struct dev_state *ps)
static struct async *async_getpending(struct dev_state *ps,
void __user *userurb)
{
- unsigned long flags;
struct async *as;
- spin_lock_irqsave(&ps->lock, flags);
list_for_each_entry(as, &ps->async_pending, asynclist)
if (as->userurb == userurb) {
list_del_init(&as->asynclist);
- spin_unlock_irqrestore(&ps->lock, flags);
return as;
}
- spin_unlock_irqrestore(&ps->lock, flags);
+
return NULL;
}
@@ -398,6 +396,7 @@ static void cancel_bulk_urbs(struct dev_state *ps, unsigned bulk_addr)
__releases(ps->lock)
__acquires(ps->lock)
{
+ struct urb *urb;
struct async *as;
/* Mark all the pending URBs that match bulk_addr, up to but not
@@ -420,8 +419,11 @@ __acquires(ps->lock)
list_for_each_entry(as, &ps->async_pending, asynclist) {
if (as->bulk_status == AS_UNLINK) {
as->bulk_status = 0; /* Only once */
+ urb = as->urb;
+ usb_get_urb(urb);
spin_unlock(&ps->lock); /* Allow completions */
- usb_unlink_urb(as->urb);
+ usb_unlink_urb(urb);
+ usb_put_urb(urb);
spin_lock(&ps->lock);
goto rescan;
}
@@ -472,6 +474,7 @@ static void async_completed(struct urb *urb)
static void destroy_async(struct dev_state *ps, struct list_head *list)
{
+ struct urb *urb;
struct async *as;
unsigned long flags;
@@ -479,10 +482,13 @@ static void destroy_async(struct dev_state *ps, struct list_head *list)
while (!list_empty(list)) {
as = list_entry(list->next, struct async, asynclist);
list_del_init(&as->asynclist);
+ urb = as->urb;
+ usb_get_urb(urb);
/* drop the spinlock so the completion handler can run */
spin_unlock_irqrestore(&ps->lock, flags);
- usb_kill_urb(as->urb);
+ usb_kill_urb(urb);
+ usb_put_urb(urb);
spin_lock_irqsave(&ps->lock, flags);
}
spin_unlock_irqrestore(&ps->lock, flags);
@@ -491,21 +497,22 @@ static void destroy_async(struct dev_state *ps, struct list_head *list)
static void destroy_async_on_interface(struct dev_state *ps,
unsigned int ifnum)
{
- struct list_head *p, *q, hitlist;
+ struct list_head *p, *q;
unsigned long flags;
- INIT_LIST_HEAD(&hitlist);
+ INIT_LIST_HEAD(&ps->async_intf);
spin_lock_irqsave(&ps->lock, flags);
list_for_each_safe(p, q, &ps->async_pending)
if (ifnum == list_entry(p, struct async, asynclist)->ifnum)
- list_move_tail(p, &hitlist);
+ list_move_tail(p, &ps->async_intf);
spin_unlock_irqrestore(&ps->lock, flags);
- destroy_async(ps, &hitlist);
+ destroy_async(ps, &ps->async_intf);
}
static void destroy_all_async(struct dev_state *ps)
{
destroy_async(ps, &ps->async_pending);
+ destroy_async(ps, &ps->async_intf);
}
/*
@@ -1410,12 +1417,24 @@ static int proc_submiturb(struct dev_state *ps, void __user *arg)
static int proc_unlinkurb(struct dev_state *ps, void __user *arg)
{
+ struct urb *urb;
struct async *as;
+ unsigned long flags;
+ spin_lock_irqsave(&ps->lock, flags);
as = async_getpending(ps, arg);
- if (!as)
+ if (!as) {
+ spin_unlock_irqrestore(&ps->lock, flags);
return -EINVAL;
- usb_kill_urb(as->urb);
+ }
+
+ urb = as->urb;
+ usb_get_urb(urb);
+ spin_unlock_irqrestore(&ps->lock, flags);
+
+ usb_kill_urb(urb);
+ usb_put_urb(urb);
+
return 0;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
