connector/edid probing races

[Daniel Vetter]

On Fri, Mar 10, 2017 at 11:01:54AM -0500, Sean Paul wrote:
> On Fri, Mar 10, 2017 at 02:12:14PM +1000, Dave Airlie wrote:
> > Talk to Jonas (jadahl) on irc, he had 3 mutters running and on hotplug
> > all 3 of them were diving into the connector getting code. Now I think
> > we can avoid this in userspace by not probing when not owning the VT,
> > but it's still messy behaviour.
> > 
> > It looks like one thread does a getconnector, this fills out the EDID
> > blob with a specific blob id,
> > then second thread does getconnector, which replaces the EDID blob
> > with another one
> > the first thread calls get property blob, but the blob id it is
> > looking for is destroyed.
> > 
> > Ideas?
> 
> I think you could probably fix this by introducing a new
> drm_property_overwrite_global_blob() function. If the size of the current blob is
> the same as the new blob, you could just overwrite the data and preserve the id
> and references. You could then replace the drm_property_replace_global_blob() call in
> drm_mode_connector_update_edid_property() to call the new function. Of course,
> you would need to fall back on *replace_global_blob() if size differed, but it
> would probably solve the issue above.

blobs are imutable (not that userspace can use that much since we can
reuse ids), I think if we do this we should compare the contents of the
blob.

> That said, I'm not convinced it should be kernel's responsibility to really
> solve this problem. How much effort should we put into mitigating the effects of
> a racey and wasteful userspace?

It's a tradeoff between "why should the kernel care" and "every compositor
will get this wrong". Maybe something like libweston will fix this
eventually ....
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch