On Fri, Jul 26, 2013 at 11:24:03PM +0200, Francois Romieu wrote:

Neil Horman [off-list ref] :

quoted

On Fri, Jul 26, 2013 at 10:06:30PM +0200, Francois Romieu wrote:

quoted

Neil Horman [off-list ref] :

[...]

quoted

quoted

quoted

@@ -533,6 +533,11 @@ rx_status_loop:
 
 		mapping = dma_map_single(&cp->pdev->dev, new_skb->data, buflen,
 					 PCI_DMA_FROMDEVICE);
+		if (dma_mapping_error(&cp->pdev->dev, mapping)) {
+			kfree_skb(skb);

			dev->stats.rx_dropped++;

Sorry, I did not notice that skb contained newly received data :o/

Yeah, this isn't needed.  The skb being mapped in is newly allocated, and
contains no data.  We haven't dropped anything here, so theres no need for the
stats bump.

Huh ?

[...error path...]
		kfree(skb);
[...normal path...]
	cp_rx_skb(cp, skb, desc);

Afaiks it's the same skb. We drop data and a stats bump is needed.

[...]

quoted

Thoughts?

Nevermind. cp_start_xmit is a piece of it and it isn't your work to
turn it into something sensible.

So you agree with the patch as it stands?
Neil

-- 
Ueimor