Re: fix for CRTC mutex corruption

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Oct 29, 2013 at 11:09:40AM -0400, Ilija Hadzic wrote:
> The following patch series fixes the mutex corruption problem
> due to bit-copying of drm_crtc structure that happens when
> drm_crtc_helper_set_config function takes the error-recovery
> path. The patch series is the alternative solution for the
> patch that was proposed and NACK-ed two weeks ago [1].

On the series:

Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx>

Aside: The horribly ad-hoc calling convetions with some of the (x, y, fb,
mode) parameters being set before calling a lower-level functions, some
being restored, some of them being the old backup value and some of them
being expected to be updated by the called function really gives me the
creeps.

But fixing that is something for a _really_ slow week (month even ...).

> The actual fix is implemented in patch #6; preceding
> 5 patches are necessary prerequisites.

Hm, I don't really see why patches 1,2&4 are required. If we reorder them
to the end of the series as follow-up cleanups then we'd only need to
backport 3 patches. Which is imo reasonable.
-Daniel

> 
> A couple of my own remarks:
> 
> 1) Someone (including myself) may be tempted to eliminate the
> bit-copy for encoder and connector structures as well. I would, however,
> prefer to leave that improvement for a different patch series.
> The primary reason is that this patch series addresses an acute
> (and serious) problem (mutex corruption), while doing the equivalent
> rework for connector and encoder structure would be only for the sake
> of improving the code quality.
> 
> 2) The problem exists in old (stable@...) kernels and my original intention
> (in the patch that was NACK-ed [1]) was to make the fix simple enough
> to be eligible for stable@.... This patch series is now probably more complex
> than what stable@... may be willing to accept. So the question in my mind
> is what we should do with the old kernels.
> 
> 3) This patch series does not include the fix for incidental finding
> described in earlier discussions about this problem [2] because it's not
> the part of the fix that this patch series is targeting, so I'd leave
> it for later.
> 
> regards,
> 
> Ilija
> 
> [1] http://lists.freedesktop.org/archives/dri-devel/2013-October/047479.html
> [2] http://lists.freedesktop.org/archives/dri-devel/2013-October/047557.html
> 
> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
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




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux