diff options
author | Daniel Vetter <daniel.vetter@ffwll.ch> | 2017-12-04 21:48:18 +0100 |
---|---|---|
committer | Daniel Vetter <daniel.vetter@ffwll.ch> | 2017-12-06 10:22:55 +0100 |
commit | a703c55004e1c5076d57e43771b3e11117796ea0 (patch) | |
tree | 2be0916f74fc5e69786a4347e19f0d793cbf4309 | |
parent | ae64f9bd1d3621b5e60d7363bc20afb46aede215 (diff) | |
download | linux-a703c55004e1c5076d57e43771b3e11117796ea0.tar.bz2 |
drm: safely free connectors from connector_iter
In
commit 613051dac40da1751ab269572766d3348d45a197
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date: Wed Dec 14 00:08:06 2016 +0100
drm: locking&new iterators for connector_list
we've went to extreme lengths to make sure connector iterations works
in any context, without introducing any additional locking context.
This worked, except for a small fumble in the implementation:
When we actually race with a concurrent connector unplug event, and
our temporary connector reference turns out to be the final one, then
everything breaks: We call the connector release function from
whatever context we happen to be in, which can be an irq/atomic
context. And connector freeing grabs all kinds of locks and stuff.
Fix this by creating a specially safe put function for connetor_iter,
which (in this rare case) punts the cleanup to a worker.
Reported-by: Ben Widawsky <ben@bwidawsk.net>
Cc: Ben Widawsky <ben@bwidawsk.net>
Fixes: 613051dac40d ("drm: locking&new iterators for connector_list")
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+
Reviewed-by: Dave Airlie <airlied@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171204204818.24745-1-daniel.vetter@ffwll.ch
-rw-r--r-- | drivers/gpu/drm/drm_connector.c | 28 | ||||
-rw-r--r-- | drivers/gpu/drm/drm_mode_config.c | 2 | ||||
-rw-r--r-- | include/drm/drm_connector.h | 8 |
3 files changed, 36 insertions, 2 deletions
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 25f4b2e9a44f..482014137953 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -152,6 +152,16 @@ static void drm_connector_free(struct kref *kref) connector->funcs->destroy(connector); } +static 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; + + drm_mode_object_unregister(dev, &connector->base); + connector->funcs->destroy(connector); +} + /** * drm_connector_init - Init a preallocated connector * @dev: DRM device @@ -181,6 +191,8 @@ 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; @@ -529,6 +541,18 @@ void drm_connector_list_iter_begin(struct drm_device *dev, } EXPORT_SYMBOL(drm_connector_list_iter_begin); +/* + * Extra-safe connector put function that works in any context. Should only be + * used from the connector_iter functions, where we never really expect to + * actually release the connector when dropping our final reference. + */ +static void +drm_connector_put_safe(struct drm_connector *conn) +{ + if (refcount_dec_and_test(&conn->base.refcount.refcount)) + schedule_work(&conn->free_work); +} + /** * drm_connector_list_iter_next - return next connector * @iter: connectr_list iterator @@ -561,7 +585,7 @@ drm_connector_list_iter_next(struct drm_connector_list_iter *iter) spin_unlock_irqrestore(&config->connector_list_lock, flags); if (old_conn) - drm_connector_put(old_conn); + drm_connector_put_safe(old_conn); return iter->conn; } @@ -580,7 +604,7 @@ void drm_connector_list_iter_end(struct drm_connector_list_iter *iter) { iter->dev = NULL; if (iter->conn) - drm_connector_put(iter->conn); + drm_connector_put_safe(iter->conn); lock_release(&connector_list_iter_dep_map, 0, _RET_IP_); } EXPORT_SYMBOL(drm_connector_list_iter_end); diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c index cda8bfab6d3b..cc78b3d9e5e4 100644 --- a/drivers/gpu/drm/drm_mode_config.c +++ b/drivers/gpu/drm/drm_mode_config.c @@ -431,6 +431,8 @@ void drm_mode_config_cleanup(struct drm_device *dev) drm_connector_put(connector); } drm_connector_list_iter_end(&conn_iter); + /* connector_iter drops references in a work item. */ + flush_scheduled_work(); if (WARN_ON(!list_empty(&dev->mode_config.connector_list))) { drm_connector_list_iter_begin(dev, &conn_iter); drm_for_each_connector_iter(connector, &conn_iter) diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index df9807a3caae..a4649c56ca2f 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -916,6 +916,14 @@ 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; + + /** + * @free_work: + * + * Work used only by &drm_connector_iter to be able to clean up a + * connector from any context. + */ + struct work_struct free_work; }; #define obj_to_connector(x) container_of(x, struct drm_connector, base) |