All of lore.kernel.org
 help / color / mirror / Atom feed
From: <gregkh@linuxfoundation.org>
To: daniel.vetter@ffwll.ch, airlied@gmail.com, ben@bwidawsk.net,
	chris@chris-wilson.co.uk, daniel.vetter@intel.com,
	gregkh@linuxfoundation.org, seanpaul@chromium.org
Cc: <stable@vger.kernel.org>, <stable-commits@vger.kernel.org>
Subject: Patch "drm: safely free connectors from connector_iter" has been added to the 4.14-stable tree
Date: Mon, 11 Dec 2017 22:52:40 +0100	[thread overview]
Message-ID: <15130291603951@kroah.com> (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 <stable@vger.kernel.org> know about it.


>From a703c55004e1c5076d57e43771b3e11117796ea0 Mon Sep 17 00:00:00 2001
From: Daniel Vetter <daniel.vetter@ffwll.ch>
Date: Mon, 4 Dec 2017 21:48:18 +0100
Subject: drm: safely free connectors from connector_iter

From: Daniel Vetter <daniel.vetter@ffwll.ch>

commit a703c55004e1c5076d57e43771b3e11117796ea0 upstream.

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>
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
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 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(-)

--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -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);
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -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)
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -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 daniel.vetter@ffwll.ch 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

                 reply	other threads:[~2017-12-11 21:54 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=15130291603951@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=airlied@gmail.com \
    --cc=ben@bwidawsk.net \
    --cc=chris@chris-wilson.co.uk \
    --cc=daniel.vetter@ffwll.ch \
    --cc=daniel.vetter@intel.com \
    --cc=seanpaul@chromium.org \
    --cc=stable-commits@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.