ice_setup_tc() takes pf->adev_mutex and the RDMA aux device lock, and then
pf->tc_mutex, for mqprio. dcbnl ops take pf->tc_mutex, and then
pf->adev_mutex in ice_send_event_to_aux(), when ice_pf_dcb_cfg() notifies
the RDMA driver about a TC change. Lockdep reports a possible deadlock:
CPU0 CPU1
---- ----
lock(&pf->adev_mutex);
lock(&pf->tc_mutex);
lock(&pf->adev_mutex);
lock(&pf->tc_mutex);
All users of pf->tc_mutex (dcbnl ops, ndo_setup_tc() for mqprio,
ice_dcb_rebuild() and the LLDP MIB change handler) already run under
RTNL, the latter two since they take it first, earlier in this series.
So the inversion can't deadlock, but lockdep still reports it, and the
mutex is redundant. Drop it, instead of reordering it against
pf->adev_mutex.
Reproducer, with RDMA enabled on the PF, but irdma not loaded:
PF=eth3
ethtool --set-priv-flags $PF fw-lldp-agent off
dcb ets set dev $PF tc-tsa all:ets tc-bw all:0 0:50 1:50 prio-tc all:0 4:1
dcb ets set dev $PF tc-tsa all:ets tc-bw all:0 0:100 prio-tc all:0
tc qdisc add dev $PF root mqprio num_tc 2 map 0 0 0 0 1 1 1 1 \
queues 4@0 4@4 hw 1 mode channel
Fixes: 4b31fd4d77ff ("ice: Fix RDMA VSI removal during queue rebuild")
Reported-by: Petr Oros <[email protected]>
Closes: https://lore.kernel.org/all/[email protected]/
Signed-off-by: Przemek Kitszel <[email protected]>
---
drivers/net/ethernet/intel/ice/ice.h | 1 -
drivers/net/ethernet/intel/ice/ice_dcb_lib.c | 6 ------
drivers/net/ethernet/intel/ice/ice_dcb_nl.c | 15 ---------------
drivers/net/ethernet/intel/ice/ice_main.c | 4 ----
4 files changed, 26 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice.h
b/drivers/net/ethernet/intel/ice/ice.h
index 432da05c48c0..12907be77454 100644
--- a/drivers/net/ethernet/intel/ice/ice.h
+++ b/drivers/net/ethernet/intel/ice/ice.h
@@ -592,7 +592,6 @@ struct ice_pf {
struct work_struct serv_task;
struct mutex avail_q_mutex; /* protects access to avail_[rx|tx]qs */
struct mutex sw_mutex; /* lock for protecting VSI alloc flow */
- struct mutex tc_mutex; /* lock to protect TC changes */
struct mutex adev_mutex; /* lock to protect aux device access */
struct mutex lag_mutex; /* protect ice_lag struct in PF */
u32 msg_enable;
diff --git a/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
b/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
index 59fafd296f1c..5355519726df 100644
--- a/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
@@ -555,7 +555,6 @@ void ice_dcb_rebuild(struct ice_pf *pf)
int ret;
rtnl_lock();
- mutex_lock(&pf->tc_mutex);
ret = ice_query_port_ets(pf->hw.port_info, &buf, sizeof(buf), NULL);
if (ret) {
@@ -587,16 +586,14 @@ void ice_dcb_rebuild(struct ice_pf *pf)
goto dcb_error;
}
- mutex_unlock(&pf->tc_mutex);
rtnl_unlock();
return;
dcb_error:
dev_err(dev, "Disabling DCB until new settings occur\n");
err_cfg = kzalloc_obj(*err_cfg);
if (!err_cfg) {
- mutex_unlock(&pf->tc_mutex);
rtnl_unlock();
return;
}
@@ -614,7 +611,6 @@ void ice_dcb_rebuild(struct ice_pf *pf)
ice_pf_dcb_cfg(pf, err_cfg, true);
kfree(err_cfg);
- mutex_unlock(&pf->tc_mutex);
rtnl_unlock();
}
@@ -1077,7 +1073,6 @@ ice_dcb_process_lldp_set_mib_change(struct ice_pf *pf,
/* That a DCB change has happened is now determined */
rtnl_lock();
- mutex_lock(&pf->tc_mutex);
/* store the old configuration */
tmp_dcbx_cfg = pi->qos_cfg.local_dcbx_cfg;
@@ -1142,7 +1137,6 @@ ice_dcb_process_lldp_set_mib_change(struct ice_pf *pf,
/* enable previously downed VSIs */
ice_dcb_ena_dis_vsi(pf, true, true);
out:
- mutex_unlock(&pf->tc_mutex);
rtnl_unlock();
/* Send Execute Pending MIB Change event if it is a Pending event */
diff --git a/drivers/net/ethernet/intel/ice/ice_dcb_nl.c
b/drivers/net/ethernet/intel/ice/ice_dcb_nl.c
index 2b9f5e6c1bbe..bc2af975b1e3 100644
--- a/drivers/net/ethernet/intel/ice/ice_dcb_nl.c
+++ b/drivers/net/ethernet/intel/ice/ice_dcb_nl.c
@@ -78,8 +78,6 @@ static int ice_dcbnl_setets(struct net_device *netdev, struct
ieee_ets *ets)
new_cfg = &pf->hw.port_info->qos_cfg.desired_dcbx_cfg;
- mutex_lock(&pf->tc_mutex);
-
new_cfg->etscfg.willing = ets->willing;
new_cfg->etscfg.cbs = ets->cbs;
ice_for_each_traffic_class(i) {
@@ -117,7 +115,6 @@ static int ice_dcbnl_setets(struct net_device *netdev,
struct ieee_ets *ets)
err = ICE_DCB_HW_CHG_RST;
ets_out:
- mutex_unlock(&pf->tc_mutex);
return err;
}
@@ -277,8 +274,6 @@ static int ice_dcbnl_setpfc(struct net_device *netdev,
struct ieee_pfc *pfc)
return -EINVAL;
}
- mutex_lock(&pf->tc_mutex);
-
new_cfg = &pf->hw.port_info->qos_cfg.desired_dcbx_cfg;
if (pfc->pfc_cap)
@@ -293,7 +288,6 @@ static int ice_dcbnl_setpfc(struct net_device *netdev,
struct ieee_pfc *pfc)
ice_dcbnl_devreset(netdev);
if (err == ICE_DCB_NO_HW_CHG)
err = ICE_DCB_HW_CHG_RST;
- mutex_unlock(&pf->tc_mutex);
return err;
}
@@ -774,9 +768,6 @@ static int ice_dcbnl_setapp(struct net_device *netdev,
struct dcb_app *app)
return -EINVAL;
}
- /* grab TC mutex */
- mutex_lock(&pf->tc_mutex);
-
new_cfg = &pf->hw.port_info->qos_cfg.desired_dcbx_cfg;
old_cfg = &pf->hw.port_info->qos_cfg.local_dcbx_cfg;
@@ -867,7 +858,6 @@ static int ice_dcbnl_setapp(struct net_device *netdev,
struct dcb_app *app)
ret = ICE_DCB_NO_HW_CHG;
setapp_out:
- mutex_unlock(&pf->tc_mutex);
return ret;
}
@@ -895,7 +885,6 @@ static int ice_dcbnl_delapp(struct net_device *netdev,
struct dcb_app *app)
return -EINVAL;
}
- mutex_lock(&pf->tc_mutex);
old_cfg = &pf->hw.port_info->qos_cfg.local_dcbx_cfg;
ret = dcb_ieee_delapp(netdev, app);
@@ -978,7 +967,6 @@ static int ice_dcbnl_delapp(struct net_device *netdev,
struct dcb_app *app)
ret = ICE_DCB_HW_CHG;
delapp_out:
- mutex_unlock(&pf->tc_mutex);
return ret;
}
@@ -1003,11 +991,8 @@ static u8 ice_dcbnl_cee_set_all(struct net_device *netdev)
new_cfg = &pf->hw.port_info->qos_cfg.desired_dcbx_cfg;
- mutex_lock(&pf->tc_mutex);
-
err = ice_pf_dcb_cfg(pf, new_cfg, true);
- mutex_unlock(&pf->tc_mutex);
return (err != ICE_DCB_HW_CHG_RST) ? ICE_DCB_NO_HW_CHG : err;
}
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c
b/drivers/net/ethernet/intel/ice/ice_main.c
index 30de690ecf7c..6b6e5ae533ae 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -3876,7 +3876,6 @@ void ice_deinit_pf(struct ice_pf *pf)
mutex_destroy(&pf->lag_mutex);
mutex_destroy(&pf->adev_mutex);
mutex_destroy(&pf->sw_mutex);
- mutex_destroy(&pf->tc_mutex);
mutex_destroy(&pf->avail_q_mutex);
mutex_destroy(&pf->vfs.table_lock);
@@ -3975,7 +3974,6 @@ int ice_init_pf(struct ice_pf *pf)
int err = -ENOMEM;
mutex_init(&pf->sw_mutex);
- mutex_init(&pf->tc_mutex);
mutex_init(&pf->adev_mutex);
mutex_init(&pf->lag_mutex);
@@ -9461,9 +9459,7 @@ ice_setup_tc(struct net_device *netdev, enum
tc_setup_type type,
}
/* setup traffic classifier for receive side */
- mutex_lock(&pf->tc_mutex);
err = ice_setup_tc_mqprio_qdisc(netdev, type_data);
- mutex_unlock(&pf->tc_mutex);
adev_unlock:
if (locked) {
--
2.54.0
