All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marc Zyngier <maz@kernel.org>
To: netdev@vger.kernel.org
Cc: lipeng <lipeng321@huawei.com>,
	Yisen Zhuang <yisen.zhuang@huawei.com>,
	Salil Mehta <salil.mehta@huawei.com>,
	"David S . Miller" <davem@davemloft.net>
Subject: [PATCH] net: hns: Ensure that interface teardown cannot race with TX interrupt
Date: Mon,  4 Nov 2019 19:56:04 +0000	[thread overview]
Message-ID: <20191104195604.17109-1-maz@kernel.org> (raw)

On a lockdep-enabled kernel, bringing down a HNS interface results
in a loud splat. It turns out that  the per-ring spinlock is taken
both in the TX interrupt path, and when bringing down the interface.

Lockdep sums it up with:

[32099.424453]        CPU0
[32099.426885]        ----
[32099.429318]   lock(&(&ring->lock)->rlock);
[32099.433402]   <Interrupt>
[32099.436008]     lock(&(&ring->lock)->rlock);
[32099.440264]
[32099.440264]  *** DEADLOCK ***

To solve this, turn the NETIF_TX_{LOCK,UNLOCK} macros from standard
spin_[un]lock to their irqsave/irqrestore version.

Fixes: f2aaed557ecff ("net: hns: Replace netif_tx_lock to ring spin lock")
Cc: lipeng <lipeng321@huawei.com>
Cc: Yisen Zhuang <yisen.zhuang@huawei.com>
Cc: Salil Mehta <salil.mehta@huawei.com>
Cc: David S. Miller <davem@davemloft.net>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 drivers/net/ethernet/hisilicon/hns/hns_enet.c | 22 ++++++++++---------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns/hns_enet.c b/drivers/net/ethernet/hisilicon/hns/hns_enet.c
index a48396dd4ebb..9fbe4e1e6853 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_enet.c
@@ -945,11 +945,11 @@ static int is_valid_clean_head(struct hnae_ring *ring, int h)
 
 /* netif_tx_lock will turn down the performance, set only when necessary */
 #ifdef CONFIG_NET_POLL_CONTROLLER
-#define NETIF_TX_LOCK(ring) spin_lock(&(ring)->lock)
-#define NETIF_TX_UNLOCK(ring) spin_unlock(&(ring)->lock)
+#define NETIF_TX_LOCK(ring, flags) spin_lock_irqsave(&(ring)->lock, flags)
+#define NETIF_TX_UNLOCK(ring, flags) spin_unlock_irqrestore(&(ring)->lock, flags)
 #else
-#define NETIF_TX_LOCK(ring)
-#define NETIF_TX_UNLOCK(ring)
+#define NETIF_TX_LOCK(ring, flags)
+#define NETIF_TX_UNLOCK(ring, flags)
 #endif
 
 /* reclaim all desc in one budget
@@ -962,16 +962,17 @@ static int hns_nic_tx_poll_one(struct hns_nic_ring_data *ring_data,
 	struct net_device *ndev = ring_data->napi.dev;
 	struct netdev_queue *dev_queue;
 	struct hns_nic_priv *priv = netdev_priv(ndev);
+	unsigned long flags;
 	int head;
 	int bytes, pkts;
 
-	NETIF_TX_LOCK(ring);
+	NETIF_TX_LOCK(ring, flags);
 
 	head = readl_relaxed(ring->io_base + RCB_REG_HEAD);
 	rmb(); /* make sure head is ready before touch any data */
 
 	if (is_ring_empty(ring) || head == ring->next_to_clean) {
-		NETIF_TX_UNLOCK(ring);
+		NETIF_TX_UNLOCK(ring, flags);
 		return 0; /* no data to poll */
 	}
 
@@ -979,7 +980,7 @@ static int hns_nic_tx_poll_one(struct hns_nic_ring_data *ring_data,
 		netdev_err(ndev, "wrong head (%d, %d-%d)\n", head,
 			   ring->next_to_use, ring->next_to_clean);
 		ring->stats.io_err_cnt++;
-		NETIF_TX_UNLOCK(ring);
+		NETIF_TX_UNLOCK(ring, flags);
 		return -EIO;
 	}
 
@@ -994,7 +995,7 @@ static int hns_nic_tx_poll_one(struct hns_nic_ring_data *ring_data,
 	ring->stats.tx_pkts += pkts;
 	ring->stats.tx_bytes += bytes;
 
-	NETIF_TX_UNLOCK(ring);
+	NETIF_TX_UNLOCK(ring, flags);
 
 	dev_queue = netdev_get_tx_queue(ndev, ring_data->queue_index);
 	netdev_tx_completed_queue(dev_queue, pkts, bytes);
@@ -1052,10 +1053,11 @@ static void hns_nic_tx_clr_all_bufs(struct hns_nic_ring_data *ring_data)
 	struct hnae_ring *ring = ring_data->ring;
 	struct net_device *ndev = ring_data->napi.dev;
 	struct netdev_queue *dev_queue;
+	unsigned long flags;
 	int head;
 	int bytes, pkts;
 
-	NETIF_TX_LOCK(ring);
+	NETIF_TX_LOCK(ring, flags);
 
 	head = ring->next_to_use; /* ntu :soft setted ring position*/
 	bytes = 0;
@@ -1063,7 +1065,7 @@ static void hns_nic_tx_clr_all_bufs(struct hns_nic_ring_data *ring_data)
 	while (head != ring->next_to_clean)
 		hns_nic_reclaim_one_desc(ring, &bytes, &pkts);
 
-	NETIF_TX_UNLOCK(ring);
+	NETIF_TX_UNLOCK(ring, flags);
 
 	dev_queue = netdev_get_tx_queue(ndev, ring_data->queue_index);
 	netdev_tx_reset_queue(dev_queue);
-- 
2.20.1


             reply	other threads:[~2019-11-04 19:56 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-04 19:56 Marc Zyngier [this message]
2019-11-05  9:16 ` [PATCH] net: hns: Ensure that interface teardown cannot race with TX interrupt Salil Mehta
2019-11-05 18:41 ` Salil Mehta
2019-11-06  8:17   ` Marc Zyngier
2019-11-06 11:19     ` Salil Mehta
2019-11-06 12:16       ` Marc Zyngier
2019-11-06 19:05         ` Salil Mehta

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=20191104195604.17109-1-maz@kernel.org \
    --to=maz@kernel.org \
    --cc=davem@davemloft.net \
    --cc=lipeng321@huawei.com \
    --cc=netdev@vger.kernel.org \
    --cc=salil.mehta@huawei.com \
    --cc=yisen.zhuang@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.