From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sean Wang Subject: Re: [SPAM]Re: [PATCH] net: mediatek: use dma_zalloc_coherent instead of allocator/memset Date: Fri, 20 Jul 2018 14:54:23 +0800 Message-ID: <1532069663.8953.259.camel@mtkswgap22> References: <20180719140955.19444-1-yuehaibing@huawei.com> <20180719141723.GM17271@n2100.armlinux.org.uk> <1532019744.8953.248.camel@mtkswgap22> <9a4cdfc6-7487-5908-b584-f3bf6158c4d4@huawei.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <9a4cdfc6-7487-5908-b584-f3bf6158c4d4@huawei.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=m.gmane.org@lists.infradead.org To: YueHaibing Cc: nbd@openwrt.org, nelson.chang@mediatek.com, netdev@vger.kernel.org, Russell King - ARM Linux , linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org, john@phrozen.org, matthias.bgg@gmail.com, davem@davemloft.net, linux-arm-kernel@lists.infradead.org List-Id: linux-mediatek@lists.infradead.org On Fri, 2018-07-20 at 14:30 +0800, YueHaibing wrote: > On 2018/7/20 1:02, Sean Wang wrote: > > On Thu, 2018-07-19 at 15:17 +0100, Russell King - ARM Linux wrote: > >> On Thu, Jul 19, 2018 at 10:09:55PM +0800, YueHaibing wrote: > >>> Use dma_zalloc_coherent instead of dma_alloc_coherent > >>> followed by memset 0. > >>> > >>> Signed-off-by: YueHaibing > >>> --- > >>> drivers/net/ethernet/mediatek/mtk_eth_soc.c | 7 ++----- > >>> 1 file changed, 2 insertions(+), 5 deletions(-) > >>> > >>> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c > >>> index d8ebf0a..fbdb3e3 100644 > >>> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c > >>> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c > >>> @@ -1221,14 +1221,11 @@ static int mtk_tx_alloc(struct mtk_eth *eth) > >>> if (!ring->buf) > >>> goto no_tx_mem; > >>> > >>> - ring->dma = dma_alloc_coherent(eth->dev, > >>> - MTK_DMA_SIZE * sz, > >>> - &ring->phys, > >>> - GFP_ATOMIC | __GFP_ZERO); > >>> + ring->dma = dma_zalloc_coherent(eth->dev, MTK_DMA_SIZE * sz, > >>> + &ring->phys, GFP_ATOMIC | __GFP_ZERO); > >>> if (!ring->dma) > >>> goto no_tx_mem; > >>> > >>> - memset(ring->dma, 0, MTK_DMA_SIZE * sz); > >> > >> I have to wonder whether this code needs two forms of zeroing... in > >> the original code, __GFP_ZERO _and_ a call to memset() just in case > >> __GFP_ZERO failed to do its job, and in the replacement code, just > >> in case dma_zalloc_coherent() hasn't got the idea... > >> > >> I think you can drop the __GFP_ZERO. ;) > >> > > > > Just now I did an experiment on 4.14.56 on armv7. I found that > > dma_zalloc_coherent does not guarantee that the buffer we get > > is all filled with 0. > > > > > > I really think it's a little bit weird OR what was I missing something > > for enabling dma_zalloc_coherent ? The result seems to tell that we > > can't remove freely the memset with 0 at this moment until we get a > > cause. > > > > That means dma_zalloc_coherent doesn't work as expect on armv7? > I'm not sure if it's true for every armv7. or it's only happening on my device. anyway, i think we can replace all occurrences in the driver for dma_alloc_coherent with __GFP_ZERO by dma_zalloc_coherent, and but keep the extra memset as is. Sean > > > > my test code is > > > > ring->dma = dma_zalloc_coherent(eth->dev, > > MTK_DMA_SIZE * sz, > > &ring->phys, > > GFP_ATOMIC); > > if (!ring->dma) > > goto no_tx_mem; > > > > print_hex_dump(KERN_INFO, "mtk_tx_alloc:", > > DUMP_PREFIX_OFFSET, 16, 1, > > ring->dma, MTK_DMA_SIZE * sz, false); > > > > memset(ring->dma, 0, MTK_DMA_SIZE * sz); > > > > print_hex_dump(KERN_INFO, "mtk_tx_alloc2:", > > DUMP_PREFIX_OFFSET, 16, 1, > > ring->dma, MTK_DMA_SIZE * sz, false); > > > > > > and the output > > > > ... > > > > [ 259.610413] mtk_tx_alloc:00000f40: 00 00 00 00 50 1f 00 bc 00 00 00 c0 00 00 00 00 > > [ 259.617934] mtk_tx_alloc:00000f50: 00 00 00 00 60 1f 00 bc 00 00 00 c0 00 00 00 00 > > [ 259.625470] mtk_tx_alloc:00000f60: 00 00 00 00 70 1f 00 bc 00 00 00 c0 00 00 00 00 > > [ 259.633005] mtk_tx_alloc:00000f70: 00 00 00 00 80 1f 00 bc 00 00 00 c0 00 00 00 00 > > [ 259.640539] mtk_tx_alloc:00000f80: 00 00 00 00 90 1f 00 bc 00 00 00 c0 00 00 00 00 > > [ 259.648073] mtk_tx_alloc:00000f90: 00 00 00 00 a0 1f 00 bc 00 00 00 c0 00 00 00 00 > > [ 259.655590] mtk_tx_alloc:00000fa0: 00 00 00 00 b0 1f 00 bc 00 00 00 c0 00 00 00 00 > > [ 259.663124] mtk_tx_alloc:00000fb0: 00 00 00 00 c0 1f 00 bc 00 00 00 c0 00 00 00 00 > > [ 259.670660] mtk_tx_alloc:00000fc0: 00 00 00 00 d0 1f 00 bc 00 00 00 c0 00 00 00 00 > > [ 259.678196] mtk_tx_alloc:00000fd0: 00 00 00 00 e0 1f 00 bc 00 00 00 c0 00 00 00 00 > > [ 259.685713] mtk_tx_alloc:00000fe0: 00 00 00 00 f0 1f 00 bc 00 00 00 c0 00 00 00 00 > > [ 259.693247] mtk_tx_alloc:00000ff0: 02 c0 90 ad 00 10 00 bc 00 40 5a c0 00 00 00 02 > > [ 259.700782] mtk_tx_alloc2:00000000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > > [ 259.708405] mtk_tx_alloc2:00000010: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > > [ 259.716013] mtk_tx_alloc2:00000020: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > > [ 259.723634] mtk_tx_alloc2:00000030: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > > [ 259.731253] mtk_tx_alloc2:00000040: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > > [ 259.738875] mtk_tx_alloc2:00000050: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > > [ 259.746481] mtk_tx_alloc2:00000060: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > > [ 259.754103] mtk_tx_alloc2:00000070: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > > [ 259.761723] mtk_tx_alloc2:00000080: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > > [ 259.769344] mtk_tx_alloc2:00000090: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > > [ 259.776951] mtk_tx_alloc2:000000a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > > > > .... > > > > > > Sean > > > > > > > > . > > >