Re: [PATCH net-next 0/3] macb: support the 2-deep Tx queue on at91

[Alexandre Belloni <alexandre.belloni@bootlin.com>]

On 14/10/2020 05:06:30+0200, Willy Tarreau wrote:
> On Tue, Oct 13, 2020 at 05:03:58PM -0700, Jakub Kicinski wrote:
> > On Sun, 11 Oct 2020 11:09:41 +0200 Willy Tarreau wrote:
> > > while running some tests on my Breadbee board, I noticed poor network
> > > Tx performance. I had a look at the driver (macb, at91ether variant)
> > > and noticed that at91ether_start_xmit() immediately stops the queue
> > > after sending a frame and waits for the interrupt to restart the queue,
> > > causing a dead time after each packet is sent.
> > > 
> > > The AT91RM9200 datasheet states that the controller supports two frames,
> > > one being sent and the other one being queued, so I performed minimal
> > > changes to support this. The transmit performance on my board has
> > > increased by 50% on medium-sized packets (HTTP traffic), and with large
> > > packets I can now reach line rate.
> > > 
> > > Since this driver is shared by various platforms, I tried my best to
> > > isolate and limit the changes as much as possible and I think it's pretty
> > > reasonable as-is. I've run extensive tests and couldn't meet any
> > > unexpected situation (no stall, overflow nor lockup).
> > > 
> > > There are 3 patches in this series. The first one adds the missing
> > > interrupt flag for RM9200 (TBRE, indicating the tx buffer is willing
> > > to take a new packet). The second one replaces the single skb with a
> > > 2-array and uses only index 0. It does no other change, this is just
> > > to prepare the code for the third one. The third one implements the
> > > queue. Packets are added at the tail of the queue, the queue is
> > > stopped at 2 packets and the interrupt releases 0, 1 or 2 depending
> > > on what the transmit status register reports.
> > 
> > LGTM. There's always a chance that this will make other 
> > designs explode, but short of someone from Cadence giving 
> > us a timely review we have only one way to find that out.. :)
> 
> Not that much in fact, given that the at91ether_* functions are only
> used by AT91RM9200 (whose datasheet I used to do this) and Mstar which
> I used for the tests. I initially wanted to get my old SAM9G20 board
> to boot until I noticed that it doesn't even use the same set of
> functions, so the potential victims are extremely limited :-)
> 

I think I'm the only one booting recent linux kernels on at91rm9200 and
I'm currently stuck home while the board is at the office. I'll try to
test as soon as possible, which may not be before 2021... At least I'll
know who is the culprit ;)


-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com