From patchwork Tue Sep 12 20:14:26 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Florian Fainelli X-Patchwork-Id: 813044 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="VxE+bTJZ"; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3xsGHZ1nTMz9s7f for ; Wed, 13 Sep 2017 06:15:06 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751419AbdILUPD (ORCPT ); Tue, 12 Sep 2017 16:15:03 -0400 Received: from mail-qt0-f196.google.com ([209.85.216.196]:38596 "EHLO mail-qt0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750742AbdILUPA (ORCPT ); Tue, 12 Sep 2017 16:15:00 -0400 Received: by mail-qt0-f196.google.com with SMTP id f24so1112175qte.5 for ; Tue, 12 Sep 2017 13:15:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id; bh=dvYHrdX2ePNQ4IROFclw/r07uUp00ElcbsaRxwXXRYE=; b=VxE+bTJZ4QQZwWDXOCWuDh1KICi/UIr18XsC7QPQ6sUFMg0ZjlBM84X7E66ZcTFLZt /CerRRYjKuTHYZ5v5HUNKPNvwtjPCfVirsiHGCTr7oz09FTeby6woUg64Y1E5dRl8BMt maaa8vhxMzms4CQgmVaGRd3OCVEqQGQO+7bC5I5+/OIFPo2UdLVU5X4Y37yIewmEB6wR h2zo1xDRYgTfZwzJvpw8kuEw5sdHkcNHv+KOWGucg/SG+1bUgpk0HF+CGhes9lH22kBt WseHimL1fEvZtCBYjU7azhuDvd0XBbcDwd/xGurfVNXyvYUv5V45eop/8oK0KHYCIvac 7ORQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=dvYHrdX2ePNQ4IROFclw/r07uUp00ElcbsaRxwXXRYE=; b=J+EQQnIXd4sAXYQMdlR/0gXSS4YVl6bZaIh9/yg7ulUD4ECpuB7QwbS7BgvW1TYf5+ LFWxfiVsDGAl3gT5CaznhbhKMmkpvIQTafVTquP+q09oTrnKKzd6m/WVyhZXvOxfwjWi lGV8biVS077vVgUGTWUDUzvUodHwd8XxbesRuFrUcj6aMKTh7S8I1pYOypNG/5YG/LIV CP5Ar4O1N8zpHHQwFd8qKl2ig3hGfpa6FZRc34OykQ9LFFqGy3Zlb0/WFtxvNlI9rLl6 oUlcFVUUplb+uC1VHbWSDwrEvtHVtLPg6QSaqzQvPVk2klvlNNnXqz2sKJgvVC5bH5bl bMmg== X-Gm-Message-State: AHPjjUi7CMuUOcX3HDDKVeq068Sr6eLvi/t4BhddP6pBUDe6lwJd9pHW 18NSF+pgSUCj0AfzveU= X-Google-Smtp-Source: AOwi7QADgjP+cfopNWaiNcFxn2/vKc1B92WDrKXCtokzSEULwsrLJMu81jOH/l2XCdzb/x4cPZTHHg== X-Received: by 10.200.35.153 with SMTP id q25mr22586654qtq.178.1505247299366; Tue, 12 Sep 2017 13:14:59 -0700 (PDT) Received: from stb-bld-04.irv.broadcom.com ([192.19.255.250]) by smtp.gmail.com with ESMTPSA id t56sm8810376qte.54.2017.09.12.13.14.56 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Tue, 12 Sep 2017 13:14:58 -0700 (PDT) From: Florian Fainelli To: netdev@vger.kernel.org Cc: davem@davemloft.net, edumazet@google.com, jqiaoulk@gmail.com, Florian Fainelli Subject: [PATCH net] net: systemport: Fix 64-bit stats deadlock Date: Tue, 12 Sep 2017 13:14:26 -0700 Message-Id: <1505247266-42195-1-git-send-email-f.fainelli@gmail.com> X-Mailer: git-send-email 1.9.1 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org 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: [] 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): [] net_rx_action+0xc4/0x4e4 [ 5.916624] hardirqs last disabled at (24263): [] _raw_spin_lock_irqsave+0x1c/0x98 [ 5.925143] softirqs last enabled at (24258): [] irq_enter+0x84/0x98 [ 5.932524] softirqs last disabled at (24259): [] 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] [ 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: [] 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: 10377ba7673d ("net: systemport: Support 64bit statistics") Signed-off-by: Florian Fainelli --- drivers/net/ethernet/broadcom/bcmsysport.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c b/drivers/net/ethernet/broadcom/bcmsysport.c index a6572b51435a..c3c53f6cd9e6 100644 --- a/drivers/net/ethernet/broadcom/bcmsysport.c +++ b/drivers/net/ethernet/broadcom/bcmsysport.c @@ -1735,11 +1735,8 @@ static void bcm_sysport_get_stats64(struct net_device *dev, stats->tx_packets += tx_packets; } - /* lockless update tx_bytes and tx_packets */ - u64_stats_update_begin(&priv->syncp); stats64->tx_bytes = stats->tx_bytes; stats64->tx_packets = stats->tx_packets; - u64_stats_update_end(&priv->syncp); do { start = u64_stats_fetch_begin_irq(&priv->syncp);