> But what makes sure that the URB unlinked in unlink_urbs() stays a valid pointer?
> The bottom half may run and free the URB.
I think it may be safe to remove the lock as we are walking a list of
SKBs not URBs,
the BH can remove SKB's from the list but we are doing a safe list walk.
+ the BH takse the list lock when it does the remove.
I could be wrong though?
On Tue, Dec 7, 2010 at 2:54 PM, Oliver Neukum <oneukum@xxxxxxx> wrote:
> Am Dienstag, 7. Dezember 2010, 15:08:43 schrieb Neil Jones:
>> I think this is due to unlink_urbs():
>
> Definitely.
>
>> static int unlink_urbs (struct usbnet *dev, struct sk_buff_head *q)
>> {
>>     unsigned long      flags;
>>     struct sk_buff     Â*skb, *skbnext;
>>     int           count = 0;
>>
>> Â Â Â Â spin_lock_irqsave (&q->lock, flags);
>> Â Â Â Â skb_queue_walk_safe(q, skb, skbnext) {
>>         struct skb_data     *entry;
>>         struct urb       Â*urb;
>>         int           retval;
>>
>> Â Â Â Â Â Â Â Â entry = (struct skb_data *) skb->cb;
>> Â Â Â Â Â Â Â Â urb = entry->urb;
>>
>> Â Â Â Â Â Â Â Â // during some PM-driven resume scenarios,
>> Â Â Â Â Â Â Â Â // these (async) unlinks complete immediately
>> Â Â Â Â Â Â Â Â retval = usb_unlink_urb (urb);
>> Â Â Â Â Â Â Â Â if (retval != -EINPROGRESS && retval != 0)
>> Â Â Â Â Â Â Â Â Â Â Â Â netdev_dbg(dev->net, "unlink urb err, %d\n", retval);
>> Â Â Â Â Â Â Â Â else
>> Â Â Â Â Â Â Â Â Â Â Â Â count++;
>> Â Â Â Â }
>> Â Â Â Â spin_unlock_irqrestore (&q->lock, flags);
>> Â Â Â Â return count;
>> }
>>
>> I dont think this should hold the list lock when calling usb_unlink_urb (urb).
>> The completion for the URBs takes the lock as required in its bottom half:
>
> But what makes sure that the URB unlinked in unlink_urbs() stays a valid pointer?
> The bottom half may run and free the URB.
>
> Â Â Â ÂRegards
> Â Â Â Â Â Â Â ÂOliver
>
--
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
