RE: [PATCH net-next 1/7] r8152: adjust rx_bottom

From: Hayes Wang
Date: Mon Jan 19 2015 - 21:49:14 EST


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-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/