* [PATCH] drm/dp/mst: close deadlock in connector destruction. @ 2015-06-15 0:34 Dave Airlie 2015-06-15 5:53 ` Jani Nikula 2015-06-15 7:49 ` Daniel Vetter 0 siblings, 2 replies; 4+ messages in thread From: Dave Airlie @ 2015-06-15 0:34 UTC (permalink / raw) To: dri-devel From: Dave Airlie <airlied@redhat.com> 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. Signed-off-by: Dave Airlie <airlied@redhat.com> --- 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 a88ae51..c98c96d 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -871,8 +871,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 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) @@ -2652,6 +2660,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 @@ -2671,10 +2703,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; @@ -2699,6 +2734,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 ca71c03..5423358 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -731,6 +731,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 04e668a..4786919 100644 --- a/include/drm/drm_dp_mst_helper.h +++ b/include/drm/drm_dp_mst_helper.h @@ -464,6 +464,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); -- 2.4.1 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/dp/mst: close deadlock in connector destruction. 2015-06-15 0:34 [PATCH] drm/dp/mst: close deadlock in connector destruction Dave Airlie @ 2015-06-15 5:53 ` Jani Nikula 2015-06-15 7:49 ` Daniel Vetter 1 sibling, 0 replies; 4+ messages in thread From: Jani Nikula @ 2015-06-15 5:53 UTC (permalink / raw) To: Dave Airlie, dri-devel On Mon, 15 Jun 2015, Dave Airlie <airlied@gmail.com> wrote: > From: Dave Airlie <airlied@redhat.com> > > 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. > > Signed-off-by: Dave Airlie <airlied@redhat.com> > --- > 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 a88ae51..c98c96d 100644 > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > @@ -871,8 +871,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 destroy the connector here, as we *can't* ? BR, Jani. > + 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) > @@ -2652,6 +2660,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 > @@ -2671,10 +2703,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; > @@ -2699,6 +2734,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 ca71c03..5423358 100644 > --- a/include/drm/drm_crtc.h > +++ b/include/drm/drm_crtc.h > @@ -731,6 +731,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 04e668a..4786919 100644 > --- a/include/drm/drm_dp_mst_helper.h > +++ b/include/drm/drm_dp_mst_helper.h > @@ -464,6 +464,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); > -- > 2.4.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/dp/mst: close deadlock in connector destruction. 2015-06-15 0:34 [PATCH] drm/dp/mst: close deadlock in connector destruction Dave Airlie 2015-06-15 5:53 ` Jani Nikula @ 2015-06-15 7:49 ` Daniel Vetter 2015-06-15 8:47 ` Dave Airlie 1 sibling, 1 reply; 4+ messages in thread From: Daniel Vetter @ 2015-06-15 7:49 UTC (permalink / raw) To: Dave Airlie; +Cc: dri-devel On Mon, Jun 15, 2015 at 10:34:28AM +1000, Dave Airlie wrote: > From: Dave Airlie <airlied@redhat.com> > > 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. > > Signed-off-by: Dave Airlie <airlied@redhat.com> The trouble here is (afai understand it at least) that we must synchronously destroy the connector since the drm core and mst helpers really don't like zombies. I thought that mode_config->mutex is providing all that race prevention, but obviously it's not (since we'd deadlock otherwise all the time). I think the proper fix for this is to - refcount connectors (well no surprise there) - in the dp mst destroy callback only unregister the connector (i.e. make it invisible to userspace), which would also drop one reference (from the kms idr). But for clarity maybe better if the port holds its own proper reference. - send out hotplug event so that userspace/fbdev notice the change and can do a suitable modeset, which would drop all the connector references due to the connector still being in use in an active mode - add checks to the fake encoder's compute_config to make sure no one can enable it again, only disable it (to avoid races with the destroy against mst unplug). Probably a call to mst_get_validate_ref plus immediate put (maybe wrapped up into a nice mst_validate_port helper) should be all we need for synchronization. - Same synchronization needs to happen in the probe helper, but I think the mst helpers already take care of that. Since the port->connector link is by pointer this design shouldn't result in any trouble with 2 refcounts for the same structure. Cheers, Daniel > --- > 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 a88ae51..c98c96d 100644 > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > @@ -871,8 +871,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 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) > @@ -2652,6 +2660,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 > @@ -2671,10 +2703,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; > @@ -2699,6 +2734,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 ca71c03..5423358 100644 > --- a/include/drm/drm_crtc.h > +++ b/include/drm/drm_crtc.h > @@ -731,6 +731,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 04e668a..4786919 100644 > --- a/include/drm/drm_dp_mst_helper.h > +++ b/include/drm/drm_dp_mst_helper.h > @@ -464,6 +464,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); > -- > 2.4.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/dp/mst: close deadlock in connector destruction. 2015-06-15 7:49 ` Daniel Vetter @ 2015-06-15 8:47 ` Dave Airlie 0 siblings, 0 replies; 4+ messages in thread From: Dave Airlie @ 2015-06-15 8:47 UTC (permalink / raw) To: Daniel Vetter; +Cc: dri-devel On 15 June 2015 at 17:49, Daniel Vetter <daniel@ffwll.ch> wrote: > On Mon, Jun 15, 2015 at 10:34:28AM +1000, Dave Airlie wrote: >> From: Dave Airlie <airlied@redhat.com> >> >> 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. >> >> Signed-off-by: Dave Airlie <airlied@redhat.com> > > The trouble here is (afai understand it at least) that we must > synchronously destroy the connector since the drm core and mst helpers > really don't like zombies. I thought that mode_config->mutex is providing > all that race prevention, but obviously it's not (since we'd deadlock > otherwise all the time). I don't think the mst core cares at all, not sure what problems the drm core would have since all that happens is the callbacks into the mst layer will fail to validate the port pointer. So the connectors aren't zombies as such. Maybe you can illuminate that more before we go with the major rework, to fix a corner case. I'll take a look at the design below, but I've no real time to work on it at the moment. Dave. > > I think the proper fix for this is to > - refcount connectors (well no surprise there) > - in the dp mst destroy callback only unregister the connector (i.e. make > it invisible to userspace), which would also drop one reference (from > the kms idr). But for clarity maybe better if the port holds its own > proper reference. > - send out hotplug event so that userspace/fbdev notice the change and can > do a suitable modeset, which would drop all the connector references due > to the connector still being in use in an active mode > - add checks to the fake encoder's compute_config to make sure no one can > enable it again, only disable it (to avoid races with the destroy > against mst unplug). Probably a call to mst_get_validate_ref plus > immediate put (maybe wrapped up into a nice mst_validate_port helper) > should be all we need for synchronization. > - Same synchronization needs to happen in the probe helper, but I think > the mst helpers already take care of that. > > Since the port->connector link is by pointer this design shouldn't result > in any trouble with 2 refcounts for the same structure. > > Cheers, Daniel > >> --- >> 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 a88ae51..c98c96d 100644 >> --- a/drivers/gpu/drm/drm_dp_mst_topology.c >> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c >> @@ -871,8 +871,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 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) >> @@ -2652,6 +2660,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 >> @@ -2671,10 +2703,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; >> @@ -2699,6 +2734,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 ca71c03..5423358 100644 >> --- a/include/drm/drm_crtc.h >> +++ b/include/drm/drm_crtc.h >> @@ -731,6 +731,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 04e668a..4786919 100644 >> --- a/include/drm/drm_dp_mst_helper.h >> +++ b/include/drm/drm_dp_mst_helper.h >> @@ -464,6 +464,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); >> -- >> 2.4.1 >> >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-06-15 8:47 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-06-15 0:34 [PATCH] drm/dp/mst: close deadlock in connector destruction Dave Airlie 2015-06-15 5:53 ` Jani Nikula 2015-06-15 7:49 ` Daniel Vetter 2015-06-15 8:47 ` Dave Airlie
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.