Oliver Hartkopp wrote:
> Matthias Fuchs wrote:
> 
>> But I ran into some issues. Perhaps some of you can help:
>> Q1: Under high TX load the driver runs out of free tx_contexts.
>> (see "couldn't find free context"). I expected the socket can core
>> to take care of this so that the lower driver cannot be overrun
>> by tx jobs. Either this expectation is wrong or there's a bug in
>> the driver :-)
> 
> Hi Matthias,
> 
> i wonder, why you do not stop the netdev tx queue immediately at the 
> beginning of
> 
>> +static netdev_tx_t esd_usb2_start_xmit(struct sk_buff *skb,
>> +                                  struct net_device *netdev)
> 
> but at this (late) point:
> 
>> +    if (err) {
>> +            can_free_echo_skb(netdev, context->echo_index);
>> +
>> +            atomic_dec(&priv->active_tx_urbs);
>> +            usb_unanchor_urb(urb);
>> +
>> +            if (err == -ENODEV)
>> +                    netif_device_detach(netdev);
>> +            else
>> +                    dev_warn(ND2D(netdev), "failed tx_urb %d\n", err);
>> +
>> +            goto releasebuf;
>> +    } else {
>> +            netdev->trans_start = jiffies;
>> +
>> +            /* Slow down tx path */
>> +            if (atomic_read(&priv->active_tx_urbs) >= MAX_TX_URBS)
>> +                    netif_stop_queue(netdev);
>> +    }
> 
> The netdev queues in Linux will give you skbs until you say 'stop' :-)
> This is not SocketCAN specific.
> 
> I assume the handling of netif_[wake|stop]_queue() should be reworked.
> 
> E.g. invoke netif_stop_queue() at the beginning of esd_usb2_start_xmit() and
> when everything is really fine, you can invoke netif_wake_queue() at the and
> of esd_usb2_start_xmit() again.

you don't have to make it that complicated. Just stop the queue at the
end of your tx routine if you're out of tx contexts. And reenable the
queue if you've finished your tx jobs.

> Btw. did you check the above code with
> 
>    if (atomic_read(&priv->active_tx_urbs) >= MAX_TX_URBS - 1)
> 
> to test for an off-by-one problem in the tx_urbs?
> 
> Regards,
> Oliver
> _______________________________________________
> Socketcan-core mailing list
> [email protected]
> https://lists.berlios.de/mailman/listinfo/socketcan-core

cheers, Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |

Attachment: signature.asc
Description: OpenPGP digital signature

_______________________________________________
Socketcan-core mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/socketcan-core

Reply via email to