Skip to content
  • Florian Fainelli's avatar
    net: systemport: Fix 64-bit stats deadlock · 7095c973
    Florian Fainelli authored
    We can enter a deadlock situation because there is no sufficient protection
    when ndo_get_stats64() runs in process context to guard against RX or TX NAPI
    contexts running in softirq, this can lead to the following lockdep splat and
    actual deadlock was experienced as well with an iperf session in the background
    and a while loop doing ifconfig + ethtool.
    
    [    5.780350] ================================
    [    5.784679] WARNING: inconsistent lock state
    [    5.789011] 4.13.0-rc7-02179-g32fae27c725d #70 Not tainted
    [    5.794561] --------------------------------
    [    5.798890] inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} usage.
    [    5.804971] swapper/0/0 [HC0[0]:SC1[1]:HE0:SE0] takes:
    [    5.810175]  (&syncp->seq#2){+.?...}, at: [<c0768a28>] bcm_sysport_tx_reclaim+0x30/0x54
    [    5.818327] {SOFTIRQ-ON-W} state was registered at:
    [    5.823278]   bcm_sysport_get_stats64+0x17c/0x258
    [    5.828053]   dev_get_stats+0x38/0xac
    [    5.831776]   rtnl_fill_stats+0x30/0x118
    [    5.835761]   rtnl_fill_ifinfo+0x538/0xe24
    [    5.839921]   rtmsg_ifinfo_build_skb+0x6c/0xd8
    [    5.844430]   rtmsg_ifinfo_event.part.5+0x14/0x44
    [    5.849201]   rtmsg_ifinfo+0x20/0x28
    [    5.852837]   register_netdevice+0x628/0x6b8
    [    5.857171]   register_netdev+0x14/0x24
    [    5.861051]   bcm_sysport_probe+0x30c/0x438
    [    5.865280]   platform_drv_probe+0x50/0xb0
    [    5.869418]   driver_probe_device+0x2e8/0x450
    [    5.873817]   __driver_attach+0x104/0x120
    [    5.877871]   bus_for_each_dev+0x7c/0xc0
    [    5.881834]   bus_add_driver+0x1b0/0x270
    [    5.885797]   driver_register+0x78/0xf4
    [    5.889675]   do_one_initcall+0x54/0x190
    [    5.893646]   kernel_init_freeable+0x144/0x1d0
    [    5.898135]   kernel_init+0x8/0x110
    [    5.901665]   ret_from_fork+0x14/0x2c
    [    5.905363] irq event stamp: 24263
    [    5.908804] hardirqs last  enabled at (24262): [<c08eecf0>] net_rx_action+0xc4/0x4e4
    [    5.916624] hardirqs last disabled at (24263): [<c0a7da00>] _raw_spin_lock_irqsave+0x1c/0x98
    [    5.925143] softirqs last  enabled at (24258): [<c022a7fc>] irq_enter+0x84/0x98
    [    5.932524] softirqs last disabled at (24259): [<c022a918>] irq_exit+0x108/0x16c
    [    5.939985]
    [    5.939985] other info that might help us debug this:
    [    5.946576]  Possible unsafe locking scenario:
    [    5.946576]
    [    5.952556]        CPU0
    [    5.955031]        ----
    [    5.957506]   lock(&syncp->seq#2);
    [    5.960955]   <Interrupt>
    [    5.963604]     lock(&syncp->seq#2);
    [    5.967227]
    [    5.967227]  *** DEADLOCK ***
    [    5.967227]
    [    5.973222] 1 lock held by swapper/0/0:
    [    5.977092]  #0:  (&(&ring->lock)->rlock){..-...}, at: [<c0768a18>] bcm_sysport_tx_reclaim+0x20/0x54
    
    So just remove the u64_stats_update_begin()/end() pair in ndo_get_stats64()
    since it does not appear to be useful for anything. No inconsistency was
    observed with either ifconfig or ethtool, global TX counts equal the sum of
    per-queue TX counts on a 32-bit architecture.
    
    Fixes: 10377ba7
    
     ("net: systemport: Support 64bit statistics")
    Signed-off-by: default avatarFlorian Fainelli <f.fainelli@gmail.com>
    Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
    7095c973