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;
                }