Re: [regression v4.11] 617f01211baf ("8139too: use napi_complete_done()")

[Eric Dumazet]

On Fri, 2017-04-21 at 08:09 -0700, Eric Dumazet wrote:
> On Fri, 2017-04-21 at 06:29 -0700, Eric Dumazet wrote:
> 
> > Thanks for this report.
> > 
> > Interesting to see how many drivers got the netpoll stuff wrong :/
> > 
> > Can you try :
> > 
> > diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
> > index 81f18a8335276495a59fa93219c4607c2b8a47aa..74e4c72c331d5a6cc5b653970ef4133c8ddf9999 100644
> > --- a/drivers/net/ethernet/realtek/r8169.c
> > +++ b/drivers/net/ethernet/realtek/r8169.c
> > @@ -7668,7 +7668,7 @@ static void rtl8169_netpoll(struct net_device *dev)
> >  {
> >  	struct rtl8169_private *tp = netdev_priv(dev);
> >  
> > -	rtl8169_interrupt(tp->pci_dev->irq, dev);
> > +	napi_schedule(&tp->napi);
> 
> The problem is more likely that netconsole handling can call rtl_tx()
> from hard irq context, while standard NAPI poll calls it from BH
> 
> Meaning that the following sequence triggers a lockdep warning.
> 
> 	u64_stats_update_begin(&tp->tx_stats.syncp);
> 	tp->tx_stats.packets++;
> 	tp->tx_stats.bytes += tx_skb->skb->len;
> 	u64_stats_update_end(&tp->tx_stats.syncp);
> 
> Lockdep does not know that poll_napi() ( called from netpoll_poll_dev())
> uses an cmpxchg() to make sure that there is no race.
> 
> I am not sure how we can teach lockdep to not splat in this case.

Well, could you try the following patch ?
Thanks !

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 0a8f2817ea60f2172eb28177473a4879f85bd18a..f64f812b86029b772bb245e51cdc2263adc4e6ea 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -7313,10 +7313,10 @@ static void rtl_tx(struct net_device *dev, struct rtl8169_private *tp)
 		rtl8169_unmap_tx_skb(&tp->pci_dev->dev, tx_skb,
 				     tp->TxDescArray + entry);
 		if (status & LastFrag) {
-			u64_stats_update_begin(&tp->tx_stats.syncp);
+			u64_stats_update_begin_raw(&tp->tx_stats.syncp);
 			tp->tx_stats.packets++;
 			tp->tx_stats.bytes += tx_skb->skb->len;
-			u64_stats_update_end(&tp->tx_stats.syncp);
+			u64_stats_update_end_raw(&tp->tx_stats.syncp);
 			dev_kfree_skb_any(tx_skb->skb);
 			tx_skb->skb = NULL;
 		}