Re: [PATCH v2 03/46] net drivers: remove unnecessary dma_sync_to_device(DMA_FROM_DEVICE)

["Vlad Zolotarov" <vladz@broadcom.com>]

>         prod_rx_buf->skb = skb;
> diff --git a/drivers/net/bnx2x/bnx2x_cmn.h b/drivers/net/bnx2x/bnx2x_cmn.h
> index c016e20..c9e49a0 100644
> --- a/drivers/net/bnx2x/bnx2x_cmn.h
> +++ b/drivers/net/bnx2x/bnx2x_cmn.h
> @@ -923,16 +923,11 @@ static inline int bnx2x_alloc_rx_skb(struct bnx2x
> *bp, static inline void bnx2x_reuse_rx_skb(struct bnx2x_fastpath *fp,
>                                       u16 cons, u16 prod)
>  {
> -       struct bnx2x *bp = fp->bp;
>         struct sw_rx_bd *cons_rx_buf = &fp->rx_buf_ring[cons];
>         struct sw_rx_bd *prod_rx_buf = &fp->rx_buf_ring[prod];
>         struct eth_rx_bd *cons_bd = &fp->rx_desc_ring[cons];
>         struct eth_rx_bd *prod_bd = &fp->rx_desc_ring[prod];
> 
> -       dma_sync_single_for_device(&bp->pdev->dev,
> -                                  dma_unmap_addr(cons_rx_buf, mapping),
> -                                  RX_COPY_THRESH, DMA_FROM_DEVICE);
> -
>         dma_unmap_addr_set(prod_rx_buf, mapping,
>                            dma_unmap_addr(cons_rx_buf, mapping));
>         prod_rx_buf->skb = cons_rx_buf->skb;

Michal, pls., note that this function is only called for buffers which were 
previously dma_synced towards CPU (your "[PATCH v2 05/46] net: bnx2x: fix DMA 
sync direction" properly fixes the direction of the first call which was 
incorrect). Then, according to the 3d edition of the "Linux device drivers" 
book, chapter 15, "Setting up streaming DMA mappings" article, end of the page 
449, when we call for dma_syc_single_for_cpu() the buffer ownership gets to 
the CPU and CPU may safely access the buffer (in particular, we read it). Then 
the author says: "Before the device accesses the buffer, however, ownership 
should be transfered back to it with: dma_sync_single_for_device().

The DMA-API.txt document u've referenced doesn't refer the above function, so, 
it's unclear how your fix may be based on it. On the other hand it clearly 
contradicts the "Linux device driver" book.

Pls., comment.

thanks,
vlad


------------------------------------------------------------------------------
All of the data generated in your IT infrastructure is seriously valuable.
Why? It contains a definitive record of application performance, security 
threats, fraudulent activity, and more. Splunk takes this data and makes 
sense of it. IT sense. And common sense.
http://p.sf.net/sfu/splunk-d2d-c2
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired