Quoting Daniel Vetter (2017-12-13 10:45:53)
PROBE_DEFER also uses system_wq to reprobe drivers, which means when that again fails, and we try to flush the overall system_wq (to get all the delayed connectore cleanup work_struct completed), we deadlock.
Fix this by using just a single cleanup work, so that we can only flush that one and don't block on anything else. That means a free list plus locking, a standard pattern.
Fixes: a703c55004e1 ("drm: safely free connectors from connector_iter") Fixes: 613051dac40d ("drm: locking&new iterators for connector_list") Cc: Ben Widawsky ben@bwidawsk.net Cc: Dave Airlie airlied@gmail.com Cc: Chris Wilson chris@chris-wilson.co.uk Cc: Sean Paul seanpaul@chromium.org Cc: stable@vger.kernel.org # v4.11+: 613051dac40d ("drm: locking&new iterators for connector_list" Cc: stable@vger.kernel.org # v4.11+ Cc: Daniel Vetter daniel.vetter@intel.com Cc: Jani Nikula jani.nikula@linux.intel.com Cc: Gustavo Padovan gustavo@padovan.org Cc: David Airlie airlied@linux.ie Cc: Javier Martinez Canillas javier@dowhile0.org Cc: Shuah Khan shuahkh@osg.samsung.com Cc: Guillaume Tucker guillaume.tucker@collabora.com Cc: Mark Brown broonie@kernel.org Cc: Kevin Hilman khilman@baylibre.com Cc: Matt Hart matthew.hart@linaro.org Cc: Thierry Escande thierry.escande@collabora.co.uk Cc: Tomeu Vizoso tomeu.vizoso@collabora.com Cc: Enric Balletbo i Serra enric.balletbo@collabora.com Signed-off-by: Daniel Vetter daniel.vetter@intel.com
drivers/gpu/drm/drm_connector.c | 46 ++++++++++++++++++++++++++----------- drivers/gpu/drm/drm_crtc_internal.h | 1 + drivers/gpu/drm/drm_mode_config.c | 5 +++- include/drm/drm_connector.h | 4 ++-- include/drm/drm_mode_config.h | 17 +++++++++++++- 5 files changed, 55 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 0b7e0974e6da..2f43dc217936 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -153,14 +153,23 @@ static void drm_connector_free(struct kref *kref) connector->funcs->destroy(connector); } -static void drm_connector_free_work_fn(struct work_struct *work) +void drm_connector_free_work_fn(struct work_struct *work) {
struct drm_connector *connector =container_of(work, struct drm_connector, free_work);struct drm_device *dev = connector->dev;
struct drm_connector *connector;struct drm_device *dev =container_of(work, struct drm_device, mode_config.connector_free_work);struct drm_mode_config *config = &dev->mode_config;unsigned long flags;LIST_HEAD(list_head);
drm_mode_object_unregister(dev, &connector->base);connector->funcs->destroy(connector);
spin_lock_irqsave(&config->connector_list_lock, flags);list_splice(&config->connector_free_list, &list_head);spin_unlock_irqrestore(&config->connector_list_lock, flags);list_for_each_entry(connector, &list_head, free_head) {drm_mode_object_unregister(dev, &connector->base);connector->funcs->destroy(connector);}} /** @@ -192,8 +201,6 @@ int drm_connector_init(struct drm_device *dev, if (ret) return ret;
INIT_WORK(&connector->free_work, drm_connector_free_work_fn);connector->base.properties = &connector->properties; connector->dev = dev; connector->funcs = funcs;@@ -550,10 +557,15 @@ EXPORT_SYMBOL(drm_connector_list_iter_begin);
- actually release the connector when dropping our final reference.
*/ static void -drm_connector_put_safe(struct drm_connector *conn) +__drm_connector_put_safe(struct drm_connector *conn) {
struct drm_mode_config *config = &conn->dev->mode_config;lockdep_assert_held(&config->connector_list_lock);list_add(&conn->free_head, &config->connector_free_list);
Adding to the free_list before refcount==0?
May I suggest,
if (!refcount_dec_and_test()) return;
if (llist_add(&conn->free_link, &config->connector_free_list)) schedule_work(&config->connector_free_work);
llist being much more friendly for freelists:
void drm_connector_free_work_fn(struct work_struct *work) { struct drm_device *dev = container_of(work, struct drm_device, mode_config.connector_free_work); struct drm_mode_config *config = &dev->mode_config; struct drm_connector *connector, *n; struct llist_node *freed;
freed = llist_del_all(config->connector_free_list); llist_for_each_entry_safe(connector, n, freed, free_link) { drm_mode_object_unregister(dev, &connector->base); connector->funcs->destroy(connector); } } -Chris