Re: [PATCH v4 06/11] net: ethernet: mtk-eth-mac: new driver

From: Arnd Bergmann <arnd@arndb.de>
To: Bartosz Golaszewski <brgl@bgdev.pl>
Cc: Stephane Le Provost <stephane.leprovost@mediatek.com>,
	Pedro Tsai <pedro.tsai@mediatek.com>,
	Andrew Perepech <andrew.perepech@mediatek.com>,
	Rob Herring <robh+dt@kernel.org>,
	"David S . Miller" <davem@davemloft.net>,
	Matthias Brugger <matthias.bgg@gmail.com>,
	John Crispin <john@phrozen.org>,
	Sean Wang <sean.wang@mediatek.com>,
	Mark Lee <Mark-MC.Lee@mediatek.com>,
	Jakub Kicinski <kuba@kernel.org>,
	Fabien Parent <fparent@baylibre.com>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	Edwin Peer <edwin.peer@broadcom.com>,
	DTML <devicetree@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Networking <netdev@vger.kernel.org>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	"moderated list:ARM/Mediatek SoC..." 
	<linux-mediatek@lists.infradead.org>,
	Bartosz Golaszewski <bgolaszewski@baylibre.com>
Subject: Re: [PATCH v4 06/11] net: ethernet: mtk-eth-mac: new driver
Date: Fri, 22 May 2020 09:49:49 +0200	[thread overview]
Message-ID: <CAK8P3a3qXf2NSDEoMHOQnChZmqQdVF--f_PFFHCyOKPhA=iW_w@mail.gmail.com> (raw)
In-Reply-To: <CAMRc=MfVkbDSfEV71SD57dpYthdx5epD0FOvjRx8qQGT+SgsTQ@mail.gmail.com>

On Fri, May 22, 2020 at 9:44 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> śr., 20 maj 2020 o 23:23 Arnd Bergmann <arnd@arndb.de> napisał(a):
> > On Wed, May 20, 2020 at 7:35 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > > śr., 20 maj 2020 o 16:37 Arnd Bergmann <arnd@arndb.de> napisał(a):

> > > My thinking was this: if I mask the relevant interrupt (TX/RX
> > > complete) and ack it right away, the status bit will be asserted on
> > > the next packet received/sent but the process won't get interrupted
> > > and when I unmask it, it will fire right away and I won't have to
> > > recheck the status register. I noticed that if I ack it at the end of
> > > napi poll callback, I end up missing certain TX complete interrupts
> > > and end up seeing a lot of retransmissions even if I reread the status
> > > register. I'm not yet sure where this race happens.
> >
> > Right, I see. If you just ack at the end of the poll function, you need
> > to check the rings again to ensure you did not miss an interrupt
> > between checking observing both rings to be empty and the irq-ack.
> >
> > I suspect it's still cheaper to check the two rings with an uncached
> > read from memory than to to do the read-modify-write on the mmio,
> > but you'd have to measure that to be sure.
> >
>
> Unfortunately the PHY on the board I have is 100Mbps which is the
> limiting factor in benchmarking this driver. :(
>
> If you're fine with this - I'd like to fix the minor issues you
> pointed out and stick with the current approach for now. We can always
> fix the implementation in the future once a board with a Gigabit PHY
> is out. Most ethernet drivers don't use such fine-grained interrupt
> control anyway. I expect the performance differences to be miniscule
> really.

Ok, fair enough. The BQL limiting is the part that matters the most
for performance on slow lines (preventing long latencies from
buffer bloat), and  you have that now.

       Arnd