Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755316AbbGPBiG (ORCPT ); Wed, 15 Jul 2015 21:38:06 -0400 Received: from youngberry.canonical.com ([91.189.89.112]:55974 "EHLO youngberry.canonical.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754927AbbGPBLR (ORCPT ); Wed, 15 Jul 2015 21:11:17 -0400 From: Kamal Mostafa To: linux-kernel@vger.kernel.org, stable@vger.kernel.org, kernel-team@lists.ubuntu.com Cc: Dave Airlie , Kamal Mostafa Subject: [PATCH 3.19.y-ckt 163/251] drm/dp/mst: close deadlock in connector destruction. Date: Wed, 15 Jul 2015 18:08:04 -0700 Message-Id: <1437008972-9140-164-git-send-email-kamal@canonical.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1437008972-9140-1-git-send-email-kamal@canonical.com> References: <1437008972-9140-1-git-send-email-kamal@canonical.com> X-Extended-Stable: 3.19 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5077 Lines: 141 3.19.8-ckt4 -stable review patch. If anyone has any objections, please let me know. ------------------ From: Dave Airlie commit 6b8eeca65b18ae77e175cc2b6571731f0ee413bf upstream. 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. Acked-by: Daniel Vetter Signed-off-by: Dave Airlie Signed-off-by: Kamal Mostafa --- drivers/gpu/drm/drm_dp_mst_topology.c | 40 +++++++++++++++++++++++++++++++++-- include/drm/drm_crtc.h | 2 ++ include/drm/drm_dp_mst_helper.h | 4 ++++ 3 files changed, 44 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 4d7a4d9..30308ab 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -863,8 +863,16 @@ static void drm_dp_destroy_port(struct kref *kref) port->vcpi.num_slots = 0; kfree(port->cached_edid); - if (port->connector) - (*port->mgr->cbs->destroy_connector)(mgr, port->connector); + + /* we can't destroy the connector here, as + we might be holding the mode_config.mutex + from an EDID retrieval */ + if (port->connector) { + mutex_lock(&mgr->destroy_connector_lock); + list_add(&port->connector->destroy_list, &mgr->destroy_connector_list); + mutex_unlock(&mgr->destroy_connector_lock); + schedule_work(&mgr->destroy_connector_work); + } drm_dp_port_teardown_pdt(port, port->pdt); if (!port->input && port->vcpi.vcpi > 0) @@ -2631,6 +2639,30 @@ static void drm_dp_tx_work(struct work_struct *work) mutex_unlock(&mgr->qlock); } +static void drm_dp_destroy_connector_work(struct work_struct *work) +{ + struct drm_dp_mst_topology_mgr *mgr = container_of(work, struct drm_dp_mst_topology_mgr, destroy_connector_work); + struct drm_connector *connector; + + /* + * Not a regular list traverse as we have to drop the destroy + * connector lock before destroying the connector, to avoid AB->BA + * ordering between this lock and the config mutex. + */ + for (;;) { + mutex_lock(&mgr->destroy_connector_lock); + connector = list_first_entry_or_null(&mgr->destroy_connector_list, struct drm_connector, destroy_list); + if (!connector) { + mutex_unlock(&mgr->destroy_connector_lock); + break; + } + list_del(&connector->destroy_list); + mutex_unlock(&mgr->destroy_connector_lock); + + mgr->cbs->destroy_connector(mgr, connector); + } +} + /** * drm_dp_mst_topology_mgr_init - initialise a topology manager * @mgr: manager struct to initialise @@ -2650,10 +2682,13 @@ int drm_dp_mst_topology_mgr_init(struct drm_dp_mst_topology_mgr *mgr, mutex_init(&mgr->lock); mutex_init(&mgr->qlock); mutex_init(&mgr->payload_lock); + mutex_init(&mgr->destroy_connector_lock); INIT_LIST_HEAD(&mgr->tx_msg_upq); INIT_LIST_HEAD(&mgr->tx_msg_downq); + INIT_LIST_HEAD(&mgr->destroy_connector_list); INIT_WORK(&mgr->work, drm_dp_mst_link_probe_work); INIT_WORK(&mgr->tx_work, drm_dp_tx_work); + INIT_WORK(&mgr->destroy_connector_work, drm_dp_destroy_connector_work); init_waitqueue_head(&mgr->tx_waitq); mgr->dev = dev; mgr->aux = aux; @@ -2678,6 +2713,7 @@ EXPORT_SYMBOL(drm_dp_mst_topology_mgr_init); */ void drm_dp_mst_topology_mgr_destroy(struct drm_dp_mst_topology_mgr *mgr) { + flush_work(&mgr->destroy_connector_work); mutex_lock(&mgr->payload_lock); kfree(mgr->payloads); mgr->payloads = NULL; diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index b863298..3dab77c 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -689,6 +689,8 @@ struct drm_connector { uint8_t num_h_tile, num_v_tile; uint8_t tile_h_loc, tile_v_loc; uint16_t tile_h_size, tile_v_size; + + struct list_head destroy_list; }; /** diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h index 00c1da9..180f8a3 100644 --- a/include/drm/drm_dp_mst_helper.h +++ b/include/drm/drm_dp_mst_helper.h @@ -463,6 +463,10 @@ struct drm_dp_mst_topology_mgr { struct work_struct work; struct work_struct tx_work; + + struct list_head destroy_connector_list; + struct mutex destroy_connector_lock; + struct work_struct destroy_connector_work; }; 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); -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/