On Fri, Aug 29, 2014 at 02:01:01PM +0200, David Herrmann wrote:
> 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@xxxxxxxxxxxxxxx> # 3.16+
> Signed-off-by: David Herrmann <dh.herrmann@xxxxxxxxx>
So userspace just needs to cycle through piles of framebuffer objects to
make bad things happen? Doesn't sound like a terribly solid plan.
I guess we could save this by doing normal id allocations for fbs and
monotonically increasing allocations for all other objects.
-Daniel
> ---
> 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
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
