drm/dp/mst: close deadlock in connector destruction.

I've only seen this once, and I failed to capture the lockdep backtrace, but I did some investigations. If we are calling into the MST layer from EDID probing, we have the mode_config mutex held, if during that EDID probing, the MST hub goes away, then we can get a deadlock where the connector destruction function in the driver tries to retake the mode config mutex. This offloads connector destruction to a workqueue, and avoid the subsequenct lock ordering issue.

Dave Airlie committed 11 years ago

Commit 6b8eeca1 parent 8b72ce13 files · +44 −2Browse files

Unified · Split

3 files

drivers/gpu/drm/drm_dp_mst_topology.c +38 −2
include/drm/drm_crtc.h +2 −0
include/drm/drm_dp_mst_helper.h +4 −0

drivers/gpu/drm/drm_dp_mst_topology.c+38−2

@@ -867,8 +867,16 @@ static void drm_dp_destroy_port(struct kref *kref) · 866 unmodified lines
867	port->vcpi.num_slots = 0;	867	port->vcpi.num_slots = 0;
868		868
869	kfree(port->cached_edid);	869	kfree(port->cached_edid);
870	if (port->connector)	870
871	(*port->mgr->cbs->destroy_connector)(mgr, port->connector);	871	/* we can't destroy the connector here, as
		872	we might be holding the mode_config.mutex
		873	from an EDID retrieval */
		874	if (port->connector) {
		875	mutex_lock(&mgr->destroy_connector_lock);
		876	list_add(&port->connector->destroy_list, &mgr->destroy_connector_list);
		877	mutex_unlock(&mgr->destroy_connector_lock);
		878	schedule_work(&mgr->destroy_connector_work);
		879	}
872	drm_dp_port_teardown_pdt(port, port->pdt);	880	drm_dp_port_teardown_pdt(port, port->pdt);
873		881
874	if (!port->input && port->vcpi.vcpi > 0)	882	if (!port->input && port->vcpi.vcpi > 0)
@@ -2649,6 +2657,30 @@ static void drm_dp_tx_work(struct work_struct *work) · 1774 unmodified lines
2649	mutex_unlock(&mgr->qlock);	2657	mutex_unlock(&mgr->qlock);
2650	}	2658	}
2651		2659
		2660	static void drm_dp_destroy_connector_work(struct work_struct *work)
		2661	{
		2662	struct drm_dp_mst_topology_mgr *mgr = container_of(work, struct drm_dp_mst_topology_mgr, destroy_connector_work);
		2663	struct drm_connector *connector;
		2664
		2665	/*
		2666	* Not a regular list traverse as we have to drop the destroy
		2667	* connector lock before destroying the connector, to avoid AB->BA
		2668	* ordering between this lock and the config mutex.
		2669	*/
		2670	for (;;) {
		2671	mutex_lock(&mgr->destroy_connector_lock);
		2672	connector = list_first_entry_or_null(&mgr->destroy_connector_list, struct drm_connector, destroy_list);
		2673	if (!connector) {
		2674	mutex_unlock(&mgr->destroy_connector_lock);
		2675	break;
		2676	}
		2677	list_del(&connector->destroy_list);
		2678	mutex_unlock(&mgr->destroy_connector_lock);
		2679
		2680	mgr->cbs->destroy_connector(mgr, connector);
		2681	}
		2682	}
		2683
2652	/**	2684	/**
2653	* drm_dp_mst_topology_mgr_init - initialise a topology manager	2685	* drm_dp_mst_topology_mgr_init - initialise a topology manager
2654	* @mgr: manager struct to initialise	2686	* @mgr: manager struct to initialise
@@ -2668,10 +2700,13 @@ int drm_dp_mst_topology_mgr_init(struct drm_dp_mst_topology_mgr *mgr, · 13 unmodified lines
2668	mutex_init(&mgr->lock);	2700	mutex_init(&mgr->lock);
2669	mutex_init(&mgr->qlock);	2701	mutex_init(&mgr->qlock);
2670	mutex_init(&mgr->payload_lock);	2702	mutex_init(&mgr->payload_lock);
		2703	mutex_init(&mgr->destroy_connector_lock);
2671	INIT_LIST_HEAD(&mgr->tx_msg_upq);	2704	INIT_LIST_HEAD(&mgr->tx_msg_upq);
2672	INIT_LIST_HEAD(&mgr->tx_msg_downq);	2705	INIT_LIST_HEAD(&mgr->tx_msg_downq);
		2706	INIT_LIST_HEAD(&mgr->destroy_connector_list);
2673	INIT_WORK(&mgr->work, drm_dp_mst_link_probe_work);	2707	INIT_WORK(&mgr->work, drm_dp_mst_link_probe_work);
2674	INIT_WORK(&mgr->tx_work, drm_dp_tx_work);	2708	INIT_WORK(&mgr->tx_work, drm_dp_tx_work);
		2709	INIT_WORK(&mgr->destroy_connector_work, drm_dp_destroy_connector_work);
2675	init_waitqueue_head(&mgr->tx_waitq);	2710	init_waitqueue_head(&mgr->tx_waitq);
2676	mgr->dev = dev;	2711	mgr->dev = dev;
2677	mgr->aux = aux;	2712	mgr->aux = aux;
@@ -2696,6 +2731,7 @@ EXPORT_SYMBOL(drm_dp_mst_topology_mgr_init); · 18 unmodified lines
2696	*/	2731	*/
2697	void drm_dp_mst_topology_mgr_destroy(struct drm_dp_mst_topology_mgr *mgr)	2732	void drm_dp_mst_topology_mgr_destroy(struct drm_dp_mst_topology_mgr *mgr)
2698	{	2733	{
		2734	flush_work(&mgr->destroy_connector_work);
2699	mutex_lock(&mgr->payload_lock);	2735	mutex_lock(&mgr->payload_lock);
2700	kfree(mgr->payloads);	2736	kfree(mgr->payloads);
2701	mgr->payloads = NULL;	2737	mgr->payloads = NULL;

include/drm/drm_crtc.h+2−0

@@ -743,6 +743,8 @@ struct drm_connector { · 742 unmodified lines
743	uint8_t num_h_tile, num_v_tile;	743	uint8_t num_h_tile, num_v_tile;
744	uint8_t tile_h_loc, tile_v_loc;	744	uint8_t tile_h_loc, tile_v_loc;
745	uint16_t tile_h_size, tile_v_size;	745	uint16_t tile_h_size, tile_v_size;
		746
		747	struct list_head destroy_list;
746	};	748	};
747		749
748	/**	750	/**

include/drm/drm_dp_mst_helper.h+4−0

@@ -463,6 +463,10 @@ struct drm_dp_mst_topology_mgr { · 462 unmodified lines
463	struct work_struct work;	463	struct work_struct work;
464		464
465	struct work_struct tx_work;	465	struct work_struct tx_work;
		466
		467	struct list_head destroy_connector_list;
		468	struct mutex destroy_connector_lock;
		469	struct work_struct destroy_connector_work;
466	};	470	};
467		471
468	int drm_dp_mst_topology_mgr_init(struct drm_dp_mst_topology_mgr mgr, struct device dev, struct drm_dp_aux *aux, int max_dpcd_transaction_bytes, int max_payloads, int conn_base_id);	472	int drm_dp_mst_topology_mgr_init(struct drm_dp_mst_topology_mgr mgr, struct device dev, struct drm_dp_aux *aux, int max_dpcd_transaction_bytes, int max_payloads, int conn_base_id);