netdev - RE: [EXT] [PATCH net-next 06/16] qlge: Remove useless dma synchronization calls

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <BYAPR18MB2696CEF6D42DAE1E467582ABABEB0@BYAPR18MB2696.namprd18.prod.outlook.com>
Date:   Mon, 17 Jun 2019 09:44:59 +0000
From:   Manish Chopra <manishc@...vell.com>
To:     Benjamin Poirier <bpoirier@...e.com>,
        GR-Linux-NIC-Dev <GR-Linux-NIC-Dev@...vell.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: [EXT] [PATCH net-next 06/16] qlge: Remove useless dma
 synchronization calls

> -----Original Message-----
> From: Benjamin Poirier <bpoirier@...e.com>
> Sent: Monday, June 17, 2019 1:19 PM
> To: Manish Chopra <manishc@...vell.com>; GR-Linux-NIC-Dev <GR-Linux-
> NIC-Dev@...vell.com>; netdev@...r.kernel.org
> Subject: [EXT] [PATCH net-next 06/16] qlge: Remove useless dma
> synchronization calls
> 
> External Email
> 
> ----------------------------------------------------------------------
> This is unneeded for two reasons:
> 1) the mapping is not written by the cpu
> 2) calls like ..._sync_..._for_device(..., ..._FROMDEVICE) are
>    nonsensical, see commit 3f0fb4e85b38 ("Documentation/DMA-API-
> HOWTO.txt:
>    fix misleading example")
> 
> Signed-off-by: Benjamin Poirier <bpoirier@...e.com>
> ---
>  drivers/net/ethernet/qlogic/qlge/qlge_main.c | 12 ------------
>  1 file changed, 12 deletions(-)
> 
> diff --git a/drivers/net/ethernet/qlogic/qlge/qlge_main.c
> b/drivers/net/ethernet/qlogic/qlge/qlge_main.c
> index 6b932bb6ce8f..70a284857488 100644
> --- a/drivers/net/ethernet/qlogic/qlge/qlge_main.c
> +++ b/drivers/net/ethernet/qlogic/qlge/qlge_main.c
> @@ -1110,9 +1110,6 @@ static void ql_update_lbq(struct ql_adapter *qdev,
> struct rx_ring *rx_ring)
>  			dma_unmap_addr_set(lbq_desc, mapaddr, map);
>  			*lbq_desc->addr = cpu_to_le64(map);
> 
> -			pci_dma_sync_single_for_device(qdev->pdev, map,
> -						       qdev->lbq_buf_size,
> -						       PCI_DMA_FROMDEVICE);
>  			clean_idx++;
>  			if (clean_idx == rx_ring->lbq_len)
>  				clean_idx = 0;
> @@ -1598,10 +1595,6 @@ static void ql_process_mac_rx_skb(struct
> ql_adapter *qdev,
> 
>  	skb_put_data(new_skb, skb->data, length);
> 
> -	pci_dma_sync_single_for_device(qdev->pdev,
> -				       dma_unmap_addr(sbq_desc, mapaddr),
> -				       SMALL_BUF_MAP_SIZE,
> -				       PCI_DMA_FROMDEVICE);

This was introduced in commit 2c9a266afefe ("qlge: Fix receive packets drop").
So hoping that it is fine, the buffer shouldn't be synced for the device back after the synced for CPU call in context of any ownership etc. ?