Patch "drm: safely free connectors from connector_iter" has been added to the 4.14-stable tree

PATCH [email protected] 1 participant 1 message Dec 11, 2017

Subsystems (3)

Stable branch DRM drivers DRM misc

lore

Share & tools

lore

[email protected]

author maintainer Applied

2017-12-11 21:54 UTC #0 lore raw

This is a note to let you know that I've just added the patch titled

drm: safely free connectors from connector_iter

to the 4.14-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
drm-safely-free-connectors-from-connector_iter.patch
and it can be found in the queue-4.14 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

Quoted 1 line

From a703c55004e1c5076d57e43771b3e11117796ea0 Mon Sep 17 00:00:00 2001

From: Daniel Vetter <[email protected]>
Date: Mon, 4 Dec 2017 21:48:18 +0100
Subject: drm: safely free connectors from connector_iter

From: Daniel Vetter <[email protected]>

commit a703c55004e1c5076d57e43771b3e11117796ea0 upstream.

commit 613051dac40da1751ab269572766d3348d45a197

Author: Daniel Vetter <[email protected]>
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 <[email protected]>

Cc Ben Widawsky <[email protected]>

Fixes 613051dac40d ("drm: locking&new iterators for connector_list")

Cc Dave Airlie <[email protected]>

Cc Chris Wilson <[email protected]>

Cc Sean Paul <[email protected]>

Reviewed-by Dave Airlie <[email protected]>

Signed-off-by Daniel Vetter <[email protected]>

Link https://patchwork.freedesktop.org/patch/msgid/[email protected]

Signed-off-by Greg Kroah-Hartman <[email protected]>

---
 drivers/gpu/drm/drm_connector.c   |   28 ++++++++++++++++++++++++++--
 drivers/gpu/drm/drm_mode_config.c |    2 ++
 include/drm/drm_connector.h       |    8 ++++++++
 3 files changed, 36 insertions(+), 2 deletions(-)

drivers/gpu/drm/drm_connector.c +26 -2

@@ -152,6 +152,16 @@ static void drm_connector_free(struct kr
 	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
 	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;

@@ -525,6 +537,18 @@ void drm_connector_list_iter_begin(struc
 }
 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

@@ -557,7 +581,7 @@ drm_connector_list_iter_next(struct drm_
 	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;
 }

@@ -576,7 +600,7 @@ void drm_connector_list_iter_end(struct
 {
 	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);

drivers/gpu/drm/drm_mode_config.c +2 -0

@@ -428,6 +428,8 @@ void drm_mode_config_cleanup(struct drm_
 		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)

include/drm/drm_connector.h +8 -0

@@ -905,6 +905,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)

Patches currently in stable-queue which might be from [email protected] are

queue-4.14/drm-safely-free-connectors-from-connector_iter.patch queue-4.14/drm-i915-fix-vblank-timestamp-frame-counter-jumps-on-gen2.patch