All of lore.kernel.org
 help / color / mirror / Atom feed
From: Qiao Ma <mqaio@linux.alibaba.com>
To: davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, gustavoars@kernel.org, cai.huoqing@linux.dev,
	aviad.krawczyk@huawei.com, zhaochen6@huawei.com
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH] net: hinic: avoid kernel hung in hinic_get_stats64()
Date: Wed, 29 Jun 2022 15:28:26 +0800	[thread overview]
Message-ID: <07736c2b7019b6883076a06129e06e8f7c5f7154.1656487154.git.mqaio@linux.alibaba.com> (raw)

When using hinic device as a bond slave device, and reading device stats of
master bond device, the kernel may hung.

The kernel panic calltrace as follows:
Kernel panic - not syncing: softlockup: hung tasks
Call trace:
  native_queued_spin_lock_slowpath+0x1ec/0x31c
  dev_get_stats+0x60/0xcc
  dev_seq_printf_stats+0x40/0x120
  dev_seq_show+0x1c/0x40
  seq_read_iter+0x3c8/0x4dc
  seq_read+0xe0/0x130
  proc_reg_read+0xa8/0xe0
  vfs_read+0xb0/0x1d4
  ksys_read+0x70/0xfc
  __arm64_sys_read+0x20/0x30
  el0_svc_common+0x88/0x234
  do_el0_svc+0x2c/0x90
  el0_svc+0x1c/0x30
  el0_sync_handler+0xa8/0xb0
  el0_sync+0x148/0x180

And the calltrace of task that actually caused kernel hungs as follows:
  __switch_to+124
  __schedule+548
  schedule+72
  schedule_timeout+348
  __down_common+188
  __down+24
  down+104
  hinic_get_stats64+44 [hinic]
  dev_get_stats+92
  bond_get_stats+172 [bonding]
  dev_get_stats+92
  dev_seq_printf_stats+60
  dev_seq_show+24
  seq_read_iter+964
  seq_read+220
  proc_reg_read+164
  vfs_read+172
  ksys_read+108
  __arm64_sys_read+28
  el0_svc_common+132
  do_el0_svc+40
  el0_svc+24
  el0_sync_handler+164
  el0_sync+324

When getting device stats from bond, kernel will call bond_get_stats().
It first holds the spinlock bond->stats_lock, and then call
hinic_get_stats64() to collect hinic device's stats.
However, hinic_get_stats64() calls `down(&nic_dev->mgmt_lock)` to
protect its critical section, which may schedule current task out.
And if system is under high pressure, the task cannot be woken up
immediately, which eventually triggers kernel hung panic.

Fixes: edd384f682cc ("net-next/hinic: Add ethtool and stats")
Signed-off-by: Qiao Ma <mqaio@linux.alibaba.com>
---
 drivers/net/ethernet/huawei/hinic/hinic_dev.h  | 1 +
 drivers/net/ethernet/huawei/hinic/hinic_main.c | 7 +++----
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/huawei/hinic/hinic_dev.h b/drivers/net/ethernet/huawei/hinic/hinic_dev.h
index fb3e89141a0d..1fb343d03fd5 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_dev.h
+++ b/drivers/net/ethernet/huawei/hinic/hinic_dev.h
@@ -97,6 +97,7 @@ struct hinic_dev {
 
 	struct hinic_txq_stats          tx_stats;
 	struct hinic_rxq_stats          rx_stats;
+	spinlock_t			stats_lock;
 
 	u8				rss_tmpl_idx;
 	u8				rss_hash_engine;
diff --git a/drivers/net/ethernet/huawei/hinic/hinic_main.c b/drivers/net/ethernet/huawei/hinic/hinic_main.c
index 56a89793f47d..32a3d700ad26 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_main.c
+++ b/drivers/net/ethernet/huawei/hinic/hinic_main.c
@@ -125,11 +125,13 @@ static void update_nic_stats(struct hinic_dev *nic_dev)
 {
 	int i, num_qps = hinic_hwdev_num_qps(nic_dev->hwdev);
 
+	spin_lock(&nic_dev->stats_lock);
 	for (i = 0; i < num_qps; i++)
 		update_rx_stats(nic_dev, &nic_dev->rxqs[i]);
 
 	for (i = 0; i < num_qps; i++)
 		update_tx_stats(nic_dev, &nic_dev->txqs[i]);
+	spin_unlock(&nic_dev->stats_lock);
 }
 
 /**
@@ -859,13 +861,9 @@ static void hinic_get_stats64(struct net_device *netdev,
 	nic_rx_stats = &nic_dev->rx_stats;
 	nic_tx_stats = &nic_dev->tx_stats;
 
-	down(&nic_dev->mgmt_lock);
-
 	if (nic_dev->flags & HINIC_INTF_UP)
 		update_nic_stats(nic_dev);
 
-	up(&nic_dev->mgmt_lock);
-
 	stats->rx_bytes   = nic_rx_stats->bytes;
 	stats->rx_packets = nic_rx_stats->pkts;
 	stats->rx_errors  = nic_rx_stats->errors;
@@ -1239,6 +1237,7 @@ static int nic_dev_init(struct pci_dev *pdev)
 
 	u64_stats_init(&tx_stats->syncp);
 	u64_stats_init(&rx_stats->syncp);
+	spin_lock_init(&nic_dev->stats_lock);
 
 	nic_dev->vlan_bitmap = devm_bitmap_zalloc(&pdev->dev, VLAN_N_VID,
 						  GFP_KERNEL);
-- 
1.8.3.1


             reply	other threads:[~2022-06-29  7:28 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-29  7:28 Qiao Ma [this message]
2022-06-30  9:56 ` [PATCH] net: hinic: avoid kernel hung in hinic_get_stats64() Paolo Abeni
2022-06-30 14:20   ` maqiao
2022-06-30 10:07 ` Paolo Abeni
2022-06-30 10:23 ` Eric Dumazet
2022-06-30 13:57   ` maqiao
2022-06-30 13:59     ` Eric Dumazet
2022-06-30 14:15       ` maqiao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=07736c2b7019b6883076a06129e06e8f7c5f7154.1656487154.git.mqaio@linux.alibaba.com \
    --to=mqaio@linux.alibaba.com \
    --cc=aviad.krawczyk@huawei.com \
    --cc=cai.huoqing@linux.dev \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=gustavoars@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=zhaochen6@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.