Wolfgang Grandegger wrote:
> 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).
Well, skb==NULL would work but I think it's cleaner to call
stats->tx_dropped++ and kfree_skb(skb) within the relevant if statement.
Wolfgang.
_______________________________________________
Socketcan-core mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/socketcan-core
