drm: prevent double-(un)registration for connectors

If we're unlucky then the registration from a hotplugged connector
might race with the final registration step on driver load. And since
MST topology discover is asynchronous that's even somewhat likely.

v2: Also update the kerneldoc for @registered!

v3: Review from Chris:
- Improve kerneldoc for late_register/early_unregister callbacks.
- Use mutex_destroy.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Sean Paul <seanpaul@chromium.org>
Reported-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/20161218133545.2106-1-daniel.vetter@ffwll.ch
(cherry picked from commit e73ab00e9a0f1731f34d0620a9c55f5c30c4ad4e)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 5a452628939272969c54239498b9a1ca86cd4a67..8be60a4576b3f2c9099a175d6bfe08dd3aa8b0c7 100644 (file)
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -225,6 +225,7 @@ int drm_connector_init(struct drm_device *dev,
 
        INIT_LIST_HEAD(&connector->probed_modes);
        INIT_LIST_HEAD(&connector->modes);
+       mutex_init(&connector->mutex);
        connector->edid_blob_ptr = NULL;
        connector->status = connector_status_unknown;
 
@@ -359,6 +360,8 @@ void drm_connector_cleanup(struct drm_connector *connector)
                connector->funcs->atomic_destroy_state(connector,
                                                       connector->state);
 
+       mutex_destroy(&connector->mutex);
+
        memset(connector, 0, sizeof(*connector));
 }
 EXPORT_SYMBOL(drm_connector_cleanup);
@@ -374,14 +377,15 @@ EXPORT_SYMBOL(drm_connector_cleanup);
  */
 int drm_connector_register(struct drm_connector *connector)
 {
-       int ret;
+       int ret = 0;
 
+       mutex_lock(&connector->mutex);
        if (connector->registered)
-               return 0;
+               goto unlock;
 
        ret = drm_sysfs_connector_add(connector);
        if (ret)
-               return ret;
+               goto unlock;
 
        ret = drm_debugfs_connector_add(connector);
        if (ret) {
@@ -397,12 +401,14 @@ int drm_connector_register(struct drm_connector *connector)
        drm_mode_object_register(connector->dev, &connector->base);
 
        connector->registered = true;
-       return 0;
+       goto unlock;
 
 err_debugfs:
        drm_debugfs_connector_remove(connector);
 err_sysfs:
        drm_sysfs_connector_remove(connector);
+unlock:
+       mutex_unlock(&connector->mutex);
        return ret;
 }
 EXPORT_SYMBOL(drm_connector_register);
@@ -415,8 +421,11 @@ EXPORT_SYMBOL(drm_connector_register);
  */
 void drm_connector_unregister(struct drm_connector *connector)
 {
-       if (!connector->registered)
+       mutex_lock(&connector->mutex);
+       if (!connector->registered) {
+               mutex_unlock(&connector->mutex);
                return;
+       }
 
        if (connector->funcs->early_unregister)
                connector->funcs->early_unregister(connector);
@@ -425,6 +434,7 @@ void drm_connector_unregister(struct drm_connector *connector)
        drm_debugfs_connector_remove(connector);
 
        connector->registered = false;
+       mutex_unlock(&connector->mutex);
 }
 EXPORT_SYMBOL(drm_connector_unregister);
 

diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index a9b95246e26efcf3d44cd5afc85f7031f0fa77fe..045a97cbeba24f44eb1b1d5582b4145801fcafdd 100644 (file)
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -381,6 +381,8 @@ struct drm_connector_funcs {
         * core drm connector interfaces. Everything added from this callback
         * should be unregistered in the early_unregister callback.
         *
+        * This is called while holding drm_connector->mutex.
+        *
         * Returns:
         *
         * 0 on success, or a negative error code on failure.
@@ -395,6 +397,8 @@ struct drm_connector_funcs {
         * late_register(). It is called from drm_connector_unregister(),
         * early in the driver unload sequence to disable userspace access
         * before data structures are torndown.
+        *
+        * This is called while holding drm_connector->mutex.
         */
        void (*early_unregister)(struct drm_connector *connector);
 
@@ -559,7 +563,6 @@ struct drm_cmdline_mode {
  * @interlace_allowed: can this connector handle interlaced modes?
  * @doublescan_allowed: can this connector handle doublescan?
  * @stereo_allowed: can this connector handle stereo modes?
- * @registered: is this connector exposed (registered) with userspace?
  * @modes: modes available on this connector (from fill_modes() + user)
  * @status: one of the drm_connector_status enums (connected, not, or unknown)
  * @probed_modes: list of modes derived directly from the display
@@ -607,6 +610,13 @@ struct drm_connector {
 
        char *name;
 
+       /**
+        * @mutex: Lock for general connector state, but currently only protects
+        * @registered. Most of the connector state is still protected by the
+        * mutex in &drm_mode_config.
+        */
+       struct mutex mutex;
+
        /**
         * @index: Compacted connector index, which matches the position inside
         * the mode_config.list for drivers not supporting hot-add/removing. Can
@@ -620,6 +630,10 @@ struct drm_connector {
        bool interlace_allowed;
        bool doublescan_allowed;
        bool stereo_allowed;
+       /**
+        * @registered: Is this connector exposed (registered) with userspace?
+        * Protected by @mutex.
+        */
        bool registered;
        struct list_head modes; /* list of modes on this connector */