Hi Huajun, On Sat, Apr 21, 2012 at 2:39 PM, Huajun Li <huajun.li.lee@xxxxxxxxx> wrote: > Hi Ming, > That's why my patch uses skb_queue_walk() to traverse the queue, > it guarantees the skb available in each loop. Is this what you > expected? > > The main idea of my patch(it is based on current mainline: 3.4.0-rc3) is: > 1. If the skb in txq/rxq, then it must be available, > unlink_urbs() can refer to it safely while it holds q->lock; > 2. If the skb in txq/rxq and its state is > rx_done/tx_done/rx_cleanup, that means the skb's URB is complete, then > don't need to unlink it again; As I said before, at least current code is not mistaken, since unlink can handle the case correctly. > 3. Before releasing q->lock in unlink_urbs(), it will increase > the URB's refercount, so even the related skb is freed in future, the > URB is still available. No, increasing URB's reference count does not prevent the referenced skb from being freed, see usbnet_bh(), so 'tmp = skb->next' in skb_queue_walk_safe still may reference a freed pointer. >> As far as I can think of, we can hold lock of done queue to forbid skb free >> during unlinking. The below patch may fix the problem, are you OK >> with it? > > Just skip trying this per your following email's comments. I will prepare a new patch later, if you'd like to try it. Thanks -- Ming Lei -- 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
