On Fri, 2014-02-21 at 11:30 -0800, Soren Brinkmann wrote:
[...]
> --- a/drivers/net/ethernet/cadence/macb.c
> +++ b/drivers/net/ethernet/cadence/macb.c
> @@ -632,11 +632,16 @@ static void gem_rx_refill(struct macb *bp)
>                                          "Unable to allocate sk_buff\n");
>                               break;
>                       }
> -                     bp->rx_skbuff[entry] = skb;
>  
>                       /* now fill corresponding descriptor entry */
>                       paddr = dma_map_single(&bp->pdev->dev, skb->data,
>                                              bp->rx_buffer_size, 
> DMA_FROM_DEVICE);
> +                     if (dma_mapping_error(&bp->pdev->dev, paddr)) {
> +                             dev_kfree_skb(skb);
> +                             break;
> +                     }
> +
> +                     bp->rx_skbuff[entry] = skb;

This bit looks good.

>                       if (entry == RX_RING_SIZE - 1)
>                               paddr |= MACB_BIT(RX_WRAP);
> @@ -1040,6 +1045,10 @@ static int macb_start_xmit(struct sk_buff *skb, struct 
> net_device *dev)
>       netdev_vdbg(bp->dev, "Allocated ring entry %u\n", entry);
>       mapping = dma_map_single(&bp->pdev->dev, skb->data,
>                                len, DMA_TO_DEVICE);
> +     if (dma_mapping_error(&bp->pdev->dev, mapping)) {
> +             dev_kfree_skb(skb);

You need to unlock bp->lock before returning.  Also, I think this should
be kfree_skb(), as that can be observed through dropwatch whereas
dev_kfree_skb() is completely silent.

Ben.

> +             return NETDEV_TX_OK;
> +     }
>  
>       tx_skb = &bp->tx_skb[entry];
>       tx_skb->skb = skb;

-- 
Ben Hutchings
All the simple programs have been written, and all the good names taken.

Attachment: signature.asc
Description: This is a digitally signed message part

Reply via email to