On Thu, Oct 17, 2013 at 1:35 AM, Ilija Hadzic <ilijahadzic@xxxxxxxxx> wrote: > Embedding a mutex inside drm_crtc structure evokes a > subtle and rare corruption in drm_crtc_helper_set_config > failure-recovery path. > > Namely, if drm_crtc_helper_set_config takes the > path under 'fail:' label *and* some other process > has attempted to grab the crtc mutex (and got blocked), > restoring the CRTC structure (which is done by bit-copying > the entire structure from saved_crtcs array) will overwrite > the mutex state and the waiters list pointer within the mutex > structure. Consequently the blocked process will never > be scheduled. > > This patch fixes the issue by un-embeding the mutex structure > and allocating it separately from the drm_crtc structure > when the CRTC is initialized. The bit-copying in > drm_crtc_helper_set_config helper will now overwrite the pointer > which is never modified during the CRTC's lifetime, thus > avoiding corruption. > > Signed-off-by: Ilija Hadzic <ihadzic@xxxxxxxxxxxxxxxxxxxxxx> > Cc: stable@xxxxxxxxxxxxxxx Can't we instead just copy the few things we need to restore back? This wholesale structure copying has rather tricky semantics and is bound to trick up someone else in the future. And indeed we seem to have a similar (but less catastrophic) thing going on with crtc->fb I think. I've looked a bit through the code and I think we don't actually need to restore anything for crtcs. We pass the mode, fb and offsets in explicitly, and everything else in drm_crtc is derived state. This is also the same that i915 does, we only restore the connector/encoder links even. -Daniel -- 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
