| Message ID | 20190828140315.17048-7-tbogendoerfer@suse.de (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ioc3-eth improvements | expand |
On Wed, 28 Aug 2019 16:03:05 +0200, Thomas Bogendoerfer wrote: > Clean rx ring is just called once after a new ring is allocated, which > is per definition clean. So there is not need for this function. > > Signed-off-by: Thomas Bogendoerfer <tbogendoerfer@suse.de> > --- > drivers/net/ethernet/sgi/ioc3-eth.c | 21 --------------------- > 1 file changed, 21 deletions(-) > > diff --git a/drivers/net/ethernet/sgi/ioc3-eth.c b/drivers/net/ethernet/sgi/ioc3-eth.c > index 6ca560d4ab79..39631e067b71 100644 > --- a/drivers/net/ethernet/sgi/ioc3-eth.c > +++ b/drivers/net/ethernet/sgi/ioc3-eth.c > @@ -761,26 +761,6 @@ static void ioc3_mii_start(struct ioc3_private *ip) > add_timer(&ip->ioc3_timer); > } > > -static inline void ioc3_clean_rx_ring(struct ioc3_private *ip) > -{ > - struct ioc3_erxbuf *rxb; > - struct sk_buff *skb; > - int i; > - > - for (i = ip->rx_ci; i & 15; i++) { > - ip->rx_skbs[ip->rx_pi] = ip->rx_skbs[ip->rx_ci]; > - ip->rxr[ip->rx_pi++] = ip->rxr[ip->rx_ci++]; > - } > - ip->rx_pi &= RX_RING_MASK; > - ip->rx_ci &= RX_RING_MASK; > - > - for (i = ip->rx_ci; i != ip->rx_pi; i = (i + 1) & RX_RING_MASK) { > - skb = ip->rx_skbs[i]; > - rxb = (struct ioc3_erxbuf *)(skb->data - RX_OFFSET); > - rxb->w0 = 0; There's gotta be some purpose to setting this w0 word to zero no? ioc3_rx() uses that to see if the descriptor is done, and dutifully clears it after.. > - } > -} > - > static inline void ioc3_clean_tx_ring(struct ioc3_private *ip) > { > struct sk_buff *skb; > @@ -860,7 +840,6 @@ static void ioc3_init_rings(struct net_device *dev) > ioc3_free_rings(ip); > ioc3_alloc_rings(dev); > > - ioc3_clean_rx_ring(ip); > ioc3_clean_tx_ring(ip); > > /* Now the rx ring base, consume & produce registers. */
On Wed, 28 Aug 2019 16:02:46 -0700 Jakub Kicinski <jakub.kicinski@netronome.com> wrote: > On Wed, 28 Aug 2019 16:03:05 +0200, Thomas Bogendoerfer wrote: > > Clean rx ring is just called once after a new ring is allocated, which > > is per definition clean. So there is not need for this function. > > > > Signed-off-by: Thomas Bogendoerfer <tbogendoerfer@suse.de> > > --- > > drivers/net/ethernet/sgi/ioc3-eth.c | 21 --------------------- > > 1 file changed, 21 deletions(-) > > > > diff --git a/drivers/net/ethernet/sgi/ioc3-eth.c b/drivers/net/ethernet/sgi/ioc3-eth.c > > index 6ca560d4ab79..39631e067b71 100644 > > --- a/drivers/net/ethernet/sgi/ioc3-eth.c > > +++ b/drivers/net/ethernet/sgi/ioc3-eth.c > > @@ -761,26 +761,6 @@ static void ioc3_mii_start(struct ioc3_private *ip) > > add_timer(&ip->ioc3_timer); > > } > > > > -static inline void ioc3_clean_rx_ring(struct ioc3_private *ip) > > -{ > > - struct ioc3_erxbuf *rxb; > > - struct sk_buff *skb; > > - int i; > > - > > - for (i = ip->rx_ci; i & 15; i++) { > > - ip->rx_skbs[ip->rx_pi] = ip->rx_skbs[ip->rx_ci]; > > - ip->rxr[ip->rx_pi++] = ip->rxr[ip->rx_ci++]; > > - } > > - ip->rx_pi &= RX_RING_MASK; > > - ip->rx_ci &= RX_RING_MASK; > > - > > - for (i = ip->rx_ci; i != ip->rx_pi; i = (i + 1) & RX_RING_MASK) { > > - skb = ip->rx_skbs[i]; > > - rxb = (struct ioc3_erxbuf *)(skb->data - RX_OFFSET); > > - rxb->w0 = 0; > > There's gotta be some purpose to setting this w0 word to zero no? > ioc3_rx() uses that to see if the descriptor is done, and dutifully > clears it after.. you are right. I thought this is already done in alloc_rx_bufs, but it isn't. I'll add it there and put it into this patch. /me wonders why testing didn't show this... Thomas.
diff --git a/drivers/net/ethernet/sgi/ioc3-eth.c b/drivers/net/ethernet/sgi/ioc3-eth.c index 6ca560d4ab79..39631e067b71 100644 --- a/drivers/net/ethernet/sgi/ioc3-eth.c +++ b/drivers/net/ethernet/sgi/ioc3-eth.c @@ -761,26 +761,6 @@ static void ioc3_mii_start(struct ioc3_private *ip) add_timer(&ip->ioc3_timer); } -static inline void ioc3_clean_rx_ring(struct ioc3_private *ip) -{ - struct ioc3_erxbuf *rxb; - struct sk_buff *skb; - int i; - - for (i = ip->rx_ci; i & 15; i++) { - ip->rx_skbs[ip->rx_pi] = ip->rx_skbs[ip->rx_ci]; - ip->rxr[ip->rx_pi++] = ip->rxr[ip->rx_ci++]; - } - ip->rx_pi &= RX_RING_MASK; - ip->rx_ci &= RX_RING_MASK; - - for (i = ip->rx_ci; i != ip->rx_pi; i = (i + 1) & RX_RING_MASK) { - skb = ip->rx_skbs[i]; - rxb = (struct ioc3_erxbuf *)(skb->data - RX_OFFSET); - rxb->w0 = 0; - } -} - static inline void ioc3_clean_tx_ring(struct ioc3_private *ip) { struct sk_buff *skb; @@ -860,7 +840,6 @@ static void ioc3_init_rings(struct net_device *dev) ioc3_free_rings(ip); ioc3_alloc_rings(dev); - ioc3_clean_rx_ring(ip); ioc3_clean_tx_ring(ip); /* Now the rx ring base, consume & produce registers. */
Clean rx ring is just called once after a new ring is allocated, which is per definition clean. So there is not need for this function. Signed-off-by: Thomas Bogendoerfer <tbogendoerfer@suse.de> --- drivers/net/ethernet/sgi/ioc3-eth.c | 21 --------------------- 1 file changed, 21 deletions(-)