All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Herrmann <dh.herrmann@gmail.com>
To: dri-devel@lists.freedesktop.org
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>,
	Dave Airlie <airlied@gmail.com>,
	David Herrmann <dh.herrmann@gmail.com>,
	stable@vger.kernel.org
Subject: [PATCH 2/2] drm: don't recycle used modeset IDs
Date: Fri, 29 Aug 2014 14:01:01 +0200	[thread overview]
Message-ID: <1409313661-20696-2-git-send-email-dh.herrmann@gmail.com> (raw)
In-Reply-To: <1409313661-20696-1-git-send-email-dh.herrmann@gmail.com>

With MST, we now have connector hotplugging, yey! Pretty easy to use in
user-space, but introduces some nasty races:
 * If a connector is removed and added again while a compositor is in
   background, it will get the same ID again. If the compositor wakes up,
   it cannot know whether its the same connector or a new one, thus they
   must re-read EDID information, etc.
 * possible_clones, possible_crtcs, etc. depend on indices, not IDs. So if
   an object is removed and a new one is added, those bitmasks are invalid
   and must be refreshed. This currently does not affect connectors, but
   only crtcs and encoders, but it's only a matter of time when this will
   change.

The easiest way to protect against this, is to not recylce modeset IDs.
Instead, always allocate a new, higher ID. All ioctls that affect modeset
objects, take IDs. Thus, during hotplug races, those ioctls will simply
fail if invalid IDs are passed. They will no longer silently run on a
newly hotplugged object.

Furthermore, hotplug-races during state sync can now be easily detected. A
call to GET_RESOURCES returns a list of available IDs atomically.
User-space can now start fetching all those objects via GET_* ioctls. If
any of those fails, they know that the given object was unplugged. Thus,
the "possible_*" bit-fields are invalidated. User-space can now decide
whether to restart the sync all over or wait for the 'change' uevent that
is sent on modeset object modifications (or, well, is supposed to be sent
and probably will be at some point).

With this in place, modeset object hotplugging should work fine for all
modeset objects in the KMS API.

CC'ing stable so we can rely on all kernels with MST to not recycle IDs.

Cc: <stable@vger.kernel.org> # 3.16+
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 drivers/gpu/drm/drm_crtc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 97eba56..ab0fe6d 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -286,7 +286,7 @@ static int drm_mode_object_get_reg(struct drm_device *dev,
 	idr_preload(GFP_KERNEL);
 	spin_lock_irq(&dev->mode_config.idr_lock);
 
-	ret = idr_alloc(&dev->mode_config.crtc_idr, register_obj ? obj : NULL, 1, 0, GFP_NOWAIT);
+	ret = idr_alloc_cyclic(&dev->mode_config.crtc_idr, register_obj ? obj : NULL, 1, 0, GFP_NOWAIT);
 	if (ret >= 0) {
 		/*
 		 * Set up the object linking under the protection of the idr
-- 
2.1.0

  reply	other threads:[~2014-08-29 12:01 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-08-29 12:01 [PATCH 1/2] drm: make idr_mutex a spinlock David Herrmann
2014-08-29 12:01 ` David Herrmann [this message]
2014-08-29 12:51   ` [PATCH 2/2] drm: don't recycle used modeset IDs Daniel Vetter
2014-08-29 12:57     ` David Herrmann
2014-08-29 13:10       ` David Herrmann
2014-08-29 15:22         ` Rob Clark
2014-08-29 13:32       ` Daniel Vetter
2014-08-29 12:53 ` [PATCH 1/2] drm: make idr_mutex a spinlock Daniel Vetter
2014-08-29 13:03   ` David Herrmann
2014-08-29 13:34     ` Daniel Vetter
2014-08-29 13:10 ` Thierry Reding
2014-08-29 13:11   ` David Herrmann
2014-08-29 13:17     ` Thierry Reding

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=1409313661-20696-2-git-send-email-dh.herrmann@gmail.com \
    --to=dh.herrmann@gmail.com \
    --cc=airlied@gmail.com \
    --cc=daniel.vetter@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.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.