From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lino Sanfilippo Subject: Re: [PATCH net-next 11/13] net: ethernet: aquantia: Refactoring buffers copying. Date: Sun, 19 Feb 2017 11:36:51 +0100 Message-ID: References: <4262ef98-4ba5-3f1a-5b72-208be51bdd6f@gmx.de> <063D6719AE5E284EB5DD2968C1650D6DCFE68133@AcuExch.aculab.com> <063D6719AE5E284EB5DD2968C1650D6DCFE6842A@AcuExch.aculab.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: Pavel Belous , "David S . Miller" , "netdev@vger.kernel.org" , Simon Edelhaus , Alexey Andriyanov To: David Laight Return-path: Received: from mout.gmx.net ([212.227.17.22]:59482 "EHLO mout.gmx.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751382AbdBSKhR (ORCPT ); Sun, 19 Feb 2017 05:37:17 -0500 In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DCFE6842A@AcuExch.aculab.com> Sender: netdev-owner@vger.kernel.org List-ID: Hi, On 16.02.2017 17:37, David Laight wrote: > From: Lino Sanfilippo >> Sent: 16 February 2017 16:02 > ... >> I was referring to the copy of tx descriptors, not the frames/fragments itself. >> I wrote "tx buffers" because in this driver a descriptor is represented as >> a struct "aq_ring_buff_s". I cannot see a reason why this descriptor copy >> should be necessary. > > Not unless the descriptor memory is on the card. > > Actually it might be worse than that. > The transmit descriptor probably has an 'owner' bit. > The write of the word containing that bit must be last and > preceded by the appropriate barrier (probably dma_wmb()). > Another barrier is needed before the 'doorbell' io write that kicks > the hardware. > After taking a look at the concerning code parts, I can say that there is at least no barrier to ensure dma flush before the doorbell. What about the patch below to fix this? Regards, Lino Subject: [PATCH] net: aquantia: add memory barrier before tx descriptors are passed to the hardware Add a memory barrier to ensure that tx descriptors are written completely before the descriptors are passed to the hardware. Signed-off-by: Lino Sanfilippo Suggested-by: David Laight --- drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_llh.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_llh.c b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_llh.c index 3de651a..c3d73e7 100644 --- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_llh.c +++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_llh.c @@ -526,6 +526,8 @@ void reg_tx_dma_desc_base_addressmswset(struct aq_hw_s *aq_hw, void reg_tx_dma_desc_tail_ptr_set(struct aq_hw_s *aq_hw, u32 tx_dma_desc_tail_ptr, u32 descriptor) { + /* make sure writes to dma descriptors are completed */ + wmb(); aq_hw_write_reg(aq_hw, tx_dma_desc_tail_ptr_adr(descriptor), tx_dma_desc_tail_ptr); } -- 2.7.4