Prashant Malani [mailto:pmal...@chromium.org]
> Sent: Tuesday, September 24, 2019 6:27 AM
> To: Hayes Wang
[...]
> - do {
> + while (1) {
> struct tx_agg *agg;
> + struct net_device *netdev = tp->netdev;
>
> if (skb_queue_empty(&tp->tx_queue))
> break;
> @@ -2188,26 +2189,25 @@ static void tx_bottom(struct r8152 *tp)
> break;
>
> res = r8152_tx_agg_fill(tp, agg);
> - if (res) {
> - struct net_device *netdev = tp->netdev;
> + if (!res)
> + break;
I let the loop run continually until an error occurs or the queue is empty.
However, you stop the loop when r8152_tx_agg_fill() is successful.
If an error occurs continually, the loop may not be broken.
> - if (res == -ENODEV) {
> - rtl_set_unplug(tp);
> - netif_device_detach(netdev);
> - } else {
> - struct net_device_stats *stats = &netdev->stats;
> - unsigned long flags;
> + if (res == -ENODEV) {
> + rtl_set_unplug(tp);
> + netif_device_detach(netdev);
> + } else {
> + struct net_device_stats *stats = &netdev->stats;
> + unsigned long flags;
>
> - netif_warn(tp, tx_err, netdev,
> - "failed tx_urb %d\n", res);
> - stats->tx_dropped += agg->skb_num;
> + netif_warn(tp, tx_err, netdev,
> + "failed tx_urb %d\n", res);
> + stats->tx_dropped += agg->skb_num;
>
> - spin_lock_irqsave(&tp->tx_lock, flags);
> - list_add_tail(&agg->list, &tp->tx_free);
> - spin_unlock_irqrestore(&tp->tx_lock, flags);
> - }
> + spin_lock_irqsave(&tp->tx_lock, flags);
> + list_add_tail(&agg->list, &tp->tx_free);
> + spin_unlock_irqrestore(&tp->tx_lock, flags);
> }
> - } while (res == 0);
> + }
I think the behavior is different from the current one.
Best Regards,
Hayes
