From mboxrd@z Thu Jan 1 00:00:00 1970 From: Neil Horman Subject: Re: [PATCHv2] 3c59x: fix missing dma_mapping_error check and bad ring refill logic Date: Wed, 3 Jan 2018 11:41:29 -0500 Message-ID: <20180103164129.GC18309@hmswarspite.think-freely.org> References: <20180103.095849.985156664792555290.davem@davemloft.net> <20180103151333.GB22129@hmswarspite.think-freely.org> <20180103.102606.1453240990448791901.davem@davemloft.net> <20180103.102831.1474605321968855786.davem@davemloft.net> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Cc: nhorman@redhat.com, netdev@vger.kernel.org, tedheadster@gmail.com, klassert@mathematik.tu-chemnitz.de To: David Miller Return-path: Received: from charlotte.tuxdriver.com ([70.61.120.58]:41629 "EHLO smtp.tuxdriver.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750992AbeACQmQ (ORCPT ); Wed, 3 Jan 2018 11:42:16 -0500 Content-Disposition: inline In-Reply-To: <20180103.102831.1474605321968855786.davem@davemloft.net> Sender: netdev-owner@vger.kernel.org List-ID: On Wed, Jan 03, 2018 at 10:28:31AM -0500, David Miller wrote: > From: David Miller > Date: Wed, 03 Jan 2018 10:26:06 -0500 (EST) > > > From: Neil Horman > > Date: Wed, 3 Jan 2018 10:13:33 -0500 > > > >> Thats exactly what this patch does, instead of creating a second loop to > >> traverse all the emptied ring buffers, now I: > >> > >> 1) Pre-allocate a new skb when I know I'm going to receive the in-place skb > >> 2) Map the skb into the appropriate dma device domain > >> 3) If (1) and (2) succede, then I swap the newly allocate skb and dma address > >> with the old one and recieve the old into the network stack > >> 4) If (1) or (2) fail, then I goto clear_complete, which leaves the old skb and > >> dma address in place, sets the buffer status back to 0 (indicating completion), > >> and write the new ring status back to the hardware > >> > >> This is what you wanted, a pre-allocate and swap-if-successful, recycle-if-not > >> approach, rather than the leave-a-hole-in-the-ring approach that is there > >> currently, no? Or did I miss something else? > > > > I misread the code sorry, you're absolutely right. > > > > I'll apply this patch, thanks Neil. :) > > Hmmm, maybe we need a V3 after all :) > > CC [M] drivers/net/ethernet/3com/3c59x.o > drivers/net/ethernet/3com/3c59x.c: In function ‘boomerang_rx’: > drivers/net/ethernet/3com/3c59x.c:2605:13: warning: unused variable ‘dma’ [-Wunused-variable] > dma_addr_t dma; > ^~~ Thats....odd, I built it twice here, and it didn't bomb out. I must not have -Werror enabled, apologies. I'll respin Neil