| Message ID | 20210902163407.60523-1-snelson@pensando.io (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 79a58c06c2d1b93a9d3ec29df08e5b726a8c63e1 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] ionic: fix double use of queue-lock | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for net |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | warning | 5 maintainers not CCed: saeedm@nvidia.com moyufeng@huawei.com tanhuazhong@huawei.com zhengyongjun3@huawei.com allenbh@pensando.io |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 59 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
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
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)
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(-)