[PATCH net] ionic: fix double use of queue-lock

[PATCH net] ionic: fix double use of queue-lock

[Shannon Nelson]

Deadlock seen in an instance where the hwstamp configuration
is changed while the driver is running:

[ 3988.736671]  schedule_preempt_disabled+0xe/0x10
[ 3988.736676]  __mutex_lock.isra.5+0x276/0x4e0
[ 3988.736683]  __mutex_lock_slowpath+0x13/0x20
[ 3988.736687]  ? __mutex_lock_slowpath+0x13/0x20
[ 3988.736692]  mutex_lock+0x2f/0x40
[ 3988.736711]  ionic_stop_queues_reconfig+0x16/0x40 [ionic]
[ 3988.736726]  ionic_reconfigure_queues+0x43e/0xc90 [ionic]
[ 3988.736738]  ionic_lif_config_hwstamp_rxq_all+0x85/0x90 [ionic]
[ 3988.736751]  ionic_lif_hwstamp_set_ts_config+0x29c/0x360 [ionic]
[ 3988.736763]  ionic_lif_hwstamp_set+0x76/0xf0 [ionic]
[ 3988.736776]  ionic_eth_ioctl+0x33/0x40 [ionic]
[ 3988.736781]  dev_ifsioc+0x12c/0x420
[ 3988.736785]  dev_ioctl+0x316/0x720

This can be demonstrated with "ptp4l -m -i <intf>"

To fix this, we pull the use of the queue_lock further up above the
callers of ionic_reconfigure_queues() and ionic_stop_queues_reconfig().

Fixes: 7ee99fc5ed2e ("ionic: pull hwstamp queue_lock up a level")
Signed-off-by: Shannon Nelson <snelson@pensando.io>
---
 drivers/net/ethernet/pensando/ionic/ionic_ethtool.c |  5 +++++
 drivers/net/ethernet/pensando/ionic/ionic_lif.c     | 12 ++++++++----
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
index e91b4874a57f..3de1a03839e2 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
@@ -582,7 +582,10 @@ static int ionic_set_ringparam(struct net_device *netdev,
 
 	qparam.ntxq_descs = ring->tx_pending;
 	qparam.nrxq_descs = ring->rx_pending;
+
+	mutex_lock(&lif->queue_lock);
 	err = ionic_reconfigure_queues(lif, &qparam);
+	mutex_unlock(&lif->queue_lock);
 	if (err)
 		netdev_info(netdev, "Ring reconfiguration failed, changes canceled: %d\n", err);
 
@@ -679,7 +682,9 @@ static int ionic_set_channels(struct net_device *netdev,
 		return 0;
 	}
 
+	mutex_lock(&lif->queue_lock);
 	err = ionic_reconfigure_queues(lif, &qparam);
+	mutex_unlock(&lif->queue_lock);
 	if (err)
 		netdev_info(netdev, "Queue reconfiguration failed, changes canceled: %d\n", err);
 
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
index 23c9e196a784..381966e8f557 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
@@ -1715,7 +1715,6 @@ static int ionic_set_mac_address(struct net_device *netdev, void *sa)
 static void ionic_stop_queues_reconfig(struct ionic_lif *lif)
 {
 	/* Stop and clean the queues before reconfiguration */
-	mutex_lock(&lif->queue_lock);
 	netif_device_detach(lif->netdev);
 	ionic_stop_queues(lif);
 	ionic_txrx_deinit(lif);
@@ -1734,8 +1733,7 @@ static int ionic_start_queues_reconfig(struct ionic_lif *lif)
 	 * DOWN and UP to try to reset and clear the issue.
 	 */
 	err = ionic_txrx_init(lif);
-	mutex_unlock(&lif->queue_lock);
-	ionic_link_status_check_request(lif, CAN_SLEEP);
+	ionic_link_status_check_request(lif, CAN_NOT_SLEEP);
 	netif_device_attach(lif->netdev);
 
 	return err;
@@ -1765,9 +1763,13 @@ static int ionic_change_mtu(struct net_device *netdev, int new_mtu)
 		return 0;
 	}
 
+	mutex_lock(&lif->queue_lock);
 	ionic_stop_queues_reconfig(lif);
 	netdev->mtu = new_mtu;
-	return ionic_start_queues_reconfig(lif);
+	err = ionic_start_queues_reconfig(lif);
+	mutex_unlock(&lif->queue_lock);
+
+	return err;
 }
 
 static void ionic_tx_timeout_work(struct work_struct *ws)
@@ -1783,8 +1785,10 @@ static void ionic_tx_timeout_work(struct work_struct *ws)
 	if (!netif_running(lif->netdev))
 		return;
 
+	mutex_lock(&lif->queue_lock);
 	ionic_stop_queues_reconfig(lif);
 	ionic_start_queues_reconfig(lif);
+	mutex_unlock(&lif->queue_lock);
 }
 
 static void ionic_tx_timeout(struct net_device *netdev, unsigned int txqueue)
-- 
2.17.1

Re: [PATCH net] ionic: fix double use of queue-lock

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (refs/heads/master):

On Thu,  2 Sep 2021 09:34:07 -0700 you wrote:
> Deadlock seen in an instance where the hwstamp configuration
> is changed while the driver is running:
> 
> [ 3988.736671]  schedule_preempt_disabled+0xe/0x10
> [ 3988.736676]  __mutex_lock.isra.5+0x276/0x4e0
> [ 3988.736683]  __mutex_lock_slowpath+0x13/0x20
> [ 3988.736687]  ? __mutex_lock_slowpath+0x13/0x20
> [ 3988.736692]  mutex_lock+0x2f/0x40
> [ 3988.736711]  ionic_stop_queues_reconfig+0x16/0x40 [ionic]
> [ 3988.736726]  ionic_reconfigure_queues+0x43e/0xc90 [ionic]
> [ 3988.736738]  ionic_lif_config_hwstamp_rxq_all+0x85/0x90 [ionic]
> [ 3988.736751]  ionic_lif_hwstamp_set_ts_config+0x29c/0x360 [ionic]
> [ 3988.736763]  ionic_lif_hwstamp_set+0x76/0xf0 [ionic]
> [ 3988.736776]  ionic_eth_ioctl+0x33/0x40 [ionic]
> [ 3988.736781]  dev_ifsioc+0x12c/0x420
> [ 3988.736785]  dev_ioctl+0x316/0x720
> 
> [...]

Here is the summary with links:
  - [net] ionic: fix double use of queue-lock
    https://git.kernel.org/netdev/net/c/79a58c06c2d1

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html