[PATCH iwl-net v2 3/5] ice: avoid RTNL deadlock when notifying the RDMA auxiliary driver

[Petr Oros <poros@redhat.com>]

ice_send_event_to_aux() blocks on the auxiliary device lock, and the
dcbnl callers reach it with the RTNL lock held. The RDMA auxiliary probe
takes the two in the opposite order, because __driver_attach() holds the
auxiliary device lock across ib_register_device(), which needs the RTNL
lock:

  systemd-udevd   state:D
    really_probe                  <- holds device_lock(&adev->dev)
    icrdma_probe
    irdma_ib_register_device
    ib_register_device
    setup_device
    irdma_roce_port_immutable
    ib_query_port
    irdma_query_port
    ib_get_eth_speed              <- blocks on rtnl_lock()

  lldpad          state:D
    rtnetlink_rcv_msg             <- holds rtnl_lock()
    dcb_doit
    dcbnl_ieee_set
    ice_dcbnl_setets
    ice_pf_dcb_cfg
    ice_send_event_to_aux         <- blocks on device_lock(&adev->dev)

Everything else that needs the RTNL lock piles up behind lldpad and the
boot never completes. The probe side cannot be changed, so take the
auxiliary device lock with device_trylock() around the event and let the
caller decide what to do when it is contended.

The DCB reconfiguration sends IIDC_RDMA_EVENT_BEFORE_TC_CHANGE, on which
the RDMA driver suspends its queue pairs, and
IIDC_RDMA_EVENT_AFTER_TC_CHANGE, on which it resumes them. Neither can be
dropped, the first would change the traffic classes behind the RDMA
driver's back and the second would leave its queue pairs suspended. So
ice_pf_dcb_cfg() takes the locks once before the first event, keeps them
across the whole reconfiguration and sends the second event before
releasing them. The driver core holds the lock while it binds, unbinds or
suspends the auxiliary device and in a few of its sysfs handlers, and the
request is then refused with -EBUSY. The RTNL lock is taken before the
auxiliary locks also for the callers that did not hold it, so every path
takes the two in the same order, and the first event is delivered with
the RTNL lock held on those paths too, as it already was for the dcbnl
callers. The second event is now sent on the error paths too, which used
to leave the queue pairs suspended after a failed commit.
ice_pf_dcb_recfg() still sends the second event on its own for the LLDP
MIB change path, which sends no first event, so a dropped one is ignored
by the RDMA driver there.

ice_pf_dcb_recfg() runs the fallback for non-contiguous traffic classes
only when the caller does not hold the locks. That fallback is a nested
ice_pf_dcb_cfg() and would block on pf->adev_mutex, so ice_pf_dcb_cfg()
runs it after it has released the locks. The other caller that passes
locked, ice_vsi_recfg_qs(), already held pf->adev_mutex there and would
have deadlocked the same way through ice_send_event_to_aux().

ice_pf_dcb_cfg() already returns -EBUSY when the custom Tx scheduler is
enabled, so the refused request takes the same path through its callers.

The critical error and MTU change events are sent from the service task,
which consumes their pending bits before sending, so those are re-armed
instead. The critical error event takes the locks before it consumes
pf->oicr_err_reg, so a contended lock leaves the register value to the
next run instead of merging it back underneath the interrupt handler.

Fixes: 348048e724a0 ("ice: Implement iidc operations")
Signed-off-by: Petr Oros <poros@redhat.com>
---
v2:
- hold the auxiliary device locks across the whole DCB reconfiguration
  and send AFTER_TC_CHANGE under them instead of fire and forget
- take the RTNL lock before the auxiliary locks also for the callers
  that did not hold it
- send AFTER_TC_CHANGE on the error paths of ice_pf_dcb_cfg() too
- add ice_aux_dev_trylock() and ice_aux_dev_unlock(), the callers of
  ice_send_event_to_aux() hold the locks
- take the locks before consuming pf->oicr_err_reg in the service task
  instead of merging the value back when the lock is contended
- run the fallback for non-contiguous traffic classes after the locks
  are released, it is a nested ice_pf_dcb_cfg()

v1: https://lore.kernel.org/all/20261019065301.3983537-2-poros@redhat.com/
---
 drivers/net/ethernet/intel/ice/ice_dcb_lib.c | 46 ++++++++++++++++---
 drivers/net/ethernet/intel/ice/ice_idc.c     | 48 ++++++++++++++++----
 drivers/net/ethernet/intel/ice/ice_idc_int.h |  2 +
 drivers/net/ethernet/intel/ice/ice_main.c    | 14 ++++--
 4 files changed, 89 insertions(+), 21 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_dcb_lib.c b/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
index 0bc6dd37568792..9916bd408071c1 100644
--- a/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
@@ -5,6 +5,9 @@
 #include "ice_dcb_nl.h"
 #include "devlink/devlink.h"
 
+static bool ice_dcb_tc_contig(u8 *prio_table);
+static int ice_dcb_noncontig_cfg(struct ice_pf *pf);
+
 /**
  * ice_dcb_get_ena_tc - return bitmap of enabled TCs
  * @dcbcfg: DCB config to evaluate for enabled TCs
@@ -352,11 +355,16 @@ int ice_pf_dcb_cfg(struct ice_pf *pf, struct ice_dcbx_cfg *new_cfg, bool locked)
 	struct ice_aqc_port_ets_elem buf = { 0 };
 	struct ice_dcbx_cfg *old_cfg, *curr_cfg;
 	struct device *dev = ice_pf_to_dev(pf);
+	struct iidc_rdma_priv_dev_info *privd;
+	struct iidc_rdma_core_dev_info *cdev;
 	struct iidc_rdma_event *event;
 	int ret = ICE_DCB_NO_HW_CHG;
 	struct ice_vsi *pf_vsi;
+	bool noncontig = false;
+	bool dcb_ena;
 
 	curr_cfg = &pf->hw.port_info->qos_cfg.local_dcbx_cfg;
+	dcb_ena = test_bit(ICE_FLAG_DCB_ENA, pf->flags);
 
 	/* FW does not care if change happened */
 	if (!pf->hw.port_info->qos_cfg.is_sw_lldp)
@@ -405,16 +413,21 @@ int ice_pf_dcb_cfg(struct ice_pf *pf, struct ice_dcbx_cfg *new_cfg, bool locked)
 		goto free_cfg;
 	}
 
-	set_bit(IIDC_RDMA_EVENT_BEFORE_TC_CHANGE, event->type);
-	ice_send_event_to_aux(pf, event);
-	kfree(event);
-
 	/* avoid race conditions by holding the lock while disabling and
 	 * re-enabling the VSI
 	 */
 	if (!locked)
 		rtnl_lock();
 
+	if (!ice_aux_dev_trylock(pf)) {
+		assign_bit(ICE_FLAG_DCB_ENA, pf->flags, dcb_ena);
+		ret = -EBUSY;
+		goto unlock_rtnl;
+	}
+
+	set_bit(IIDC_RDMA_EVENT_BEFORE_TC_CHANGE, event->type);
+	ice_send_event_to_aux(pf, event);
+
 	/* disable VSIs affected by DCB changes */
 	ice_dcb_ena_dis_vsi(pf, false, true);
 
@@ -441,13 +454,28 @@ int ice_pf_dcb_cfg(struct ice_pf *pf, struct ice_dcbx_cfg *new_cfg, bool locked)
 		goto out;
 	}
 
-	ice_pf_dcb_recfg(pf, false);
+	ice_pf_dcb_recfg(pf, true);
+	noncontig = !ice_dcb_tc_contig(curr_cfg->etscfg.prio_table);
 
 out:
+	cdev = pf->cdev_info;
+	if (cdev) {
+		privd = cdev->iidc_priv;
+		ice_setup_dcb_qos_info(pf, &privd->qos_info);
+	}
+	clear_bit(IIDC_RDMA_EVENT_BEFORE_TC_CHANGE, event->type);
+	set_bit(IIDC_RDMA_EVENT_AFTER_TC_CHANGE, event->type);
+	ice_send_event_to_aux(pf, event);
+	ice_aux_dev_unlock(pf);
+
 	/* enable previously downed VSIs */
 	ice_dcb_ena_dis_vsi(pf, true, true);
+	if (noncontig)
+		ice_dcb_noncontig_cfg(pf);
+unlock_rtnl:
 	if (!locked)
 		rtnl_unlock();
+	kfree(event);
 free_cfg:
 	kfree(old_cfg);
 	return ret;
@@ -761,7 +789,8 @@ void ice_pf_dcb_recfg(struct ice_pf *pf, bool locked)
 			 */
 			if (!ice_dcb_tc_contig(dcbcfg->etscfg.prio_table)) {
 				tc_map = ICE_DFLT_TRAFFIC_CLASS;
-				ice_dcb_noncontig_cfg(pf);
+				if (!locked)
+					ice_dcb_noncontig_cfg(pf);
 			}
 		} else if (vsi->type == ICE_VSI_CHNL) {
 			tc_map = BIT(ice_get_first_droptc(vsi));
@@ -796,7 +825,10 @@ void ice_pf_dcb_recfg(struct ice_pf *pf, bool locked)
 			return;
 
 		set_bit(IIDC_RDMA_EVENT_AFTER_TC_CHANGE, event->type);
-		ice_send_event_to_aux(pf, event);
+		if (ice_aux_dev_trylock(pf)) {
+			ice_send_event_to_aux(pf, event);
+			ice_aux_dev_unlock(pf);
+		}
 		kfree(event);
 	}
 }
diff --git a/drivers/net/ethernet/intel/ice/ice_idc.c b/drivers/net/ethernet/intel/ice/ice_idc.c
index 102d63c3018b1f..018e7f4d46ae9a 100644
--- a/drivers/net/ethernet/intel/ice/ice_idc.c
+++ b/drivers/net/ethernet/intel/ice/ice_idc.c
@@ -30,34 +30,62 @@ ice_get_auxiliary_drv(struct iidc_rdma_core_dev_info *cdev)
 			    struct iidc_rdma_core_auxiliary_drv, adrv.driver);
 }
 
+/**
+ * ice_aux_dev_trylock - take the locks that guard the RDMA AUX driver
+ * @pf: pointer to PF struct
+ *
+ * Return: true when the locks are held, false when the auxiliary device lock
+ * was contended and nothing is held.
+ */
+bool ice_aux_dev_trylock(struct ice_pf *pf)
+{
+	struct iidc_rdma_core_dev_info *cdev = pf->cdev_info;
+
+	mutex_lock(&pf->adev_mutex);
+	if (cdev && cdev->adev && !device_trylock(&cdev->adev->dev)) {
+		mutex_unlock(&pf->adev_mutex);
+		return false;
+	}
+
+	return true;
+}
+
+/**
+ * ice_aux_dev_unlock - release the locks taken by ice_aux_dev_trylock()
+ * @pf: pointer to PF struct
+ */
+void ice_aux_dev_unlock(struct ice_pf *pf)
+{
+	struct iidc_rdma_core_dev_info *cdev = pf->cdev_info;
+
+	if (cdev && cdev->adev)
+		device_unlock(&cdev->adev->dev);
+	mutex_unlock(&pf->adev_mutex);
+}
+
 /**
  * ice_send_event_to_aux - send event to RDMA AUX driver
  * @pf: pointer to PF struct
  * @event: event struct
+ *
+ * The caller holds the locks taken by ice_aux_dev_trylock().
  */
 void ice_send_event_to_aux(struct ice_pf *pf, struct iidc_rdma_event *event)
 {
+	struct iidc_rdma_core_dev_info *cdev = pf->cdev_info;
 	struct iidc_rdma_core_auxiliary_drv *iadrv;
-	struct iidc_rdma_core_dev_info *cdev;
 
 	if (WARN_ON_ONCE(!in_task()))
 		return;
 
-	cdev = pf->cdev_info;
+	lockdep_assert_held(&pf->adev_mutex);
+
 	if (!cdev)
 		return;
 
-	mutex_lock(&pf->adev_mutex);
-	if (!cdev->adev)
-		goto finish;
-
-	device_lock(&cdev->adev->dev);
 	iadrv = ice_get_auxiliary_drv(cdev);
 	if (iadrv && iadrv->event_handler)
 		iadrv->event_handler(cdev, event);
-	device_unlock(&cdev->adev->dev);
-finish:
-	mutex_unlock(&pf->adev_mutex);
 }
 
 /**
diff --git a/drivers/net/ethernet/intel/ice/ice_idc_int.h b/drivers/net/ethernet/intel/ice/ice_idc_int.h
index 17dbfcfb6a2aff..4cccb4764271ae 100644
--- a/drivers/net/ethernet/intel/ice/ice_idc_int.h
+++ b/drivers/net/ethernet/intel/ice/ice_idc_int.h
@@ -9,6 +9,8 @@
 
 struct ice_pf;
 
+bool ice_aux_dev_trylock(struct ice_pf *pf);
+void ice_aux_dev_unlock(struct ice_pf *pf);
 void ice_send_event_to_aux(struct ice_pf *pf, struct iidc_rdma_event *event);
 
 #endif /* !_ICE_IDC_INT_H_ */
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 8693ca285d5f1e..9de375f5c4deb5 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -2313,13 +2313,16 @@ static void ice_service_task(struct work_struct *work)
 		struct iidc_rdma_event *event;
 
 		event = kzalloc_obj(*event);
-		if (event) {
+		if (event && ice_aux_dev_trylock(pf)) {
 			set_bit(IIDC_RDMA_EVENT_CRIT_ERR, event->type);
 			/* report the entire OICR value to AUX driver */
 			swap(event->reg, pf->oicr_err_reg);
 			ice_send_event_to_aux(pf, event);
-			kfree(event);
+			ice_aux_dev_unlock(pf);
+		} else {
+			set_bit(ICE_AUX_ERR_PENDING, pf->state);
 		}
+		kfree(event);
 	}
 
 	/* unplug aux dev per request, if an unplug request came in
@@ -2336,11 +2339,14 @@ static void ice_service_task(struct work_struct *work)
 		struct iidc_rdma_event *event;
 
 		event = kzalloc_obj(*event);
-		if (event) {
+		if (event && ice_aux_dev_trylock(pf)) {
 			set_bit(IIDC_RDMA_EVENT_AFTER_MTU_CHANGE, event->type);
 			ice_send_event_to_aux(pf, event);
-			kfree(event);
+			ice_aux_dev_unlock(pf);
+		} else {
+			set_bit(ICE_FLAG_MTU_CHANGED, pf->flags);
 		}
+		kfree(event);
 	}
 
 	ice_clean_adminq_subtask(pf);
-- 
2.56.0