Matthias Fuchs wrote:
> Wolfgang,
>
>>> +
>>> + err = usb_submit_urb(urb, GFP_ATOMIC);
>>> + if (err) {
>>> + can_free_echo_skb(netdev, context->echo_index);
> skb = NULL;
>>> +
>>> + atomic_dec(&priv->active_tx_jobs);
>>> + 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_jobs) >= MAX_TX_URBS)
>>> + netif_stop_queue(netdev);
>>> + }
>>> +
>>> + /*
>>> + * Release our reference to this URB, the USB core will eventually free
>>> + * it entirely.
>>> + */
>>> + usb_free_urb(urb);
>>> +
>>> + return NETDEV_TX_OK;
>>> +
>>> +releasebuf:
>>> + usb_buffer_free(dev->udev, size, buf, urb->transfer_dma);
>>> +
>>> +nobufmem:
>>> + usb_free_urb(urb);
>>> +
>>> +nourbmem:
>>> + if (ret != NETDEV_TX_BUSY) {
>>> + if (skb)
>>> + dev_kfree_skb(skb);
>>> +
>>> + stats->tx_dropped++;
>> Be aware that can_put_echo_skb() of can_free_echo_skb() above might have
>> already freed the skb, also use kfree_skb() instead of dev_kfree_skb().
>>
> Does skb=NULL fix this? Is skb always identical to the skb that is free'd in
> can_free_echo_skb()
No. "can_put_echo_skb(this_skb, dev, idx)" will store "this_skb" (or a
clone of it). If you call "can_free_echo_skb(dev, idx)" it will normally
free "this_skb" (or the cloned skb).
> I think so, but just to be sure ...
It is not identical if the skb was cloned. Also be aware, that
"can_put_echo_skb(this_skb, dev, idx) will *free* "this_skb"
immediately, if local loopback (echo) is disabled. Well, it would help
to add more documentation for the function.
Wolfgang.
_______________________________________________
Socketcan-core mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/socketcan-core
