On Fri, Aug 29, 2014 at 9:10 AM, David Herrmann <dh.herrmann@xxxxxxxxx> wrote: > Hi > > On Fri, Aug 29, 2014 at 2:57 PM, David Herrmann <dh.herrmann@xxxxxxxxx> wrote: >> Hi >> >> On Fri, Aug 29, 2014 at 2:51 PM, Daniel Vetter <daniel@xxxxxxxx> wrote: >>> 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. >> >> IDs will still get recycled, but only once all IDs got used. So this is "safe". >> >> Sure, user-space can create 2 billion framebuffers and destroy them >> again, thus causing the ID range to overflow and recycle old IDs. Not >> sure how fast you can run 2 billion syscalls.. If that's a real issue, >> I'd vote for using the high-range for user-space managed objects, >> low-range for kernel-space managed objects ([1...INT_MAX] and >> [INT_MAX+1...UINT_MAX] or so). >> >>> I guess we could save this by doing normal id allocations for fbs and >>> monotonically increasing allocations for all other objects. >> >> This doesn't work. A connector with ID #n might get unplugged and >> another process created a new FB, which will then get ID #n. Sure, I >> doubt there's a real conflict as ioctls check the type, but it still >> sounds really ugly to me. > > On a second thought: maybe your idea isn't as bad as I thought. I > mean, everyone must do type-checking when looking up mode-objects, so > it seems safe to rely on that. note that for atomic there are cases where we do lookup w/ ANY_TYPE.. *but* the idea might still work since FB's are still handled specially(ish) do to refcnting.. BR, -R > Thanks > David > _______________________________________________ > dri-devel mailing list > dri-devel@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/dri-devel _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
