David Miller [mailto:davem@xxxxxxxxxxxxx] > Sent: Tuesday, January 20, 2015 5:14 AM [...] > >> - r8152_submit_rx(tp, agg, GFP_ATOMIC); > >> + if (!ret) { > >> + ret = r8152_submit_rx(tp, agg, GFP_ATOMIC); > >> + } else { > >> + urb->actual_length = 0; > >> + list_add_tail(&agg->list, next); > > > > Do you need a spin_lock_irqsave(&tp->rx_lock, flags) around this? > > Indeed, and rtl_start_rx() seems to also access agg->list without > proper locking. It is unnecessary because I deal with them in a local list_head. My steps are 1. Move the whole list from tp->rx_done to local rx_queue. (with spin lock) 2. dequeue/enqueue the lists in rx_queue. 3. Move the lists in rx_queue to tp->rx_done if it is necessary. (spin lock) For step 2, it wouldn't have race, because the list_head is local and no other function would change it. Therefore, I don't think it needs the spin lock. The rtl_start_rx() also uses the similar way. Best Regards, Hayes -- 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
