On Sat, 5 Dec 2020 22:44:07 -0500 Sven Van Asbroeck wrote: > From: Sven Van Asbroeck <thesve...@gmail.com> > > Even if the rx ring is completely full, and there is more rx data > waiting on the chip, the rx napi poll fn will never run more than > once - it will always immediately bail out and re-enable interrupts. > Which results in ping-pong between napi and interrupt. > > This defeats the purpose of napi, and is bad for performance. > > Fix by addressing two separate issues: > > 1. Ensure the rx napi poll fn always updates the rx ring tail > when returning, even when not re-enabling interrupts. > > 2. Up to half of elements in a full rx ring are extension > frames, which do not generate any skbs. Limit the default > napi weight to the smallest no. of skbs that can be generated > by a full rx ring. > > Tested-by: Sven Van Asbroeck <thesve...@gmail.com> # lan7430 > Signed-off-by: Sven Van Asbroeck <thesve...@gmail.com> > --- > > Tree: git://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git # > 905b2032fa42 > > To: Bryan Whitehead <bryan.whiteh...@microchip.com> > To: Microchip Linux Driver Support <unglinuxdri...@microchip.com> > To: "David S. Miller" <da...@davemloft.net> > To: Jakub Kicinski <k...@kernel.org> > Cc: Andrew Lunn <and...@lunn.ch> > Cc: netdev@vger.kernel.org > Cc: linux-ker...@vger.kernel.org > > drivers/net/ethernet/microchip/lan743x_main.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/microchip/lan743x_main.c > b/drivers/net/ethernet/microchip/lan743x_main.c > index 87b6c59a1e03..ebb5e0bc516b 100644 > --- a/drivers/net/ethernet/microchip/lan743x_main.c > +++ b/drivers/net/ethernet/microchip/lan743x_main.c > @@ -2260,10 +2260,11 @@ static int lan743x_rx_napi_poll(struct napi_struct > *napi, int weight) > INT_BIT_DMA_RX_(rx->channel_number)); > } > > +done: > /* update RX_TAIL */ > lan743x_csr_write(adapter, RX_TAIL(rx->channel_number), > rx_tail_flags | rx->last_tail); > -done: > +
I assume this rings the doorbell to let the device know that more buffers are available? If so it's a little unusual to do this at the end of NAPI poll. The more usual place would be to do this every n times a new buffer is allocated (in lan743x_rx_init_ring_element()?) That's to say for example ring the doorbell every time a buffer is put at an index divisible by 16. > return count; > } > > @@ -2405,9 +2406,15 @@ static int lan743x_rx_open(struct lan743x_rx *rx) > if (ret) > goto return_error; > > + /* up to half of elements in a full rx ring are > + * extension frames. these do not generate skbs. > + * to prevent napi/interrupt ping-pong, limit default > + * weight to the smallest no. of skbs that can be > + * generated by a full rx ring. > + */ > netif_napi_add(adapter->netdev, > &rx->napi, lan743x_rx_napi_poll, > - rx->ring_size - 1); > + (rx->ring_size - 1) / 2); This is rather unusual, drivers should generally pass NAPI_POLL_WEIGHT here.
