Cancel the output polling work proc before acquiring the struct mutex to avoid acquiring the work proc mutex with the struct mutex held. This avoids inverting the lock order seen when the work proc runs.
Signed-off-by: Keith Packard keithp@keithp.com --- drivers/gpu/drm/i915/intel_display.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 9792285..a4cc072 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -6052,9 +6052,9 @@ void intel_modeset_cleanup(struct drm_device *dev) struct drm_crtc *crtc; struct intel_crtc *intel_crtc;
+ drm_kms_helper_poll_fini(dev); mutex_lock(&dev->struct_mutex);
- drm_kms_helper_poll_fini(dev); intel_fbdev_fini(dev);
list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
On Sun, 3 Oct 2010 19:36:26 -0700, Keith Packard keithp@keithp.com wrote:
Cancel the output polling work proc before acquiring the struct mutex to avoid acquiring the work proc mutex with the struct mutex held. This avoids inverting the lock order seen when the work proc runs.
I thought this was part of Daniel's reordering to avoid race conditions between interrupts and module unload. Fortunately not. The lack of locking during hotplug is worrisome should we ever do dynamic connectors.
Applied to -next. Now you also said that we have memory corruption after unload... :( Thanks, -Chris
On Mon, Oct 4, 2010 at 6:49 PM, Chris Wilson chris@chris-wilson.co.uk wrote:
On Sun, 3 Oct 2010 19:36:26 -0700, Keith Packard keithp@keithp.com wrote:
Cancel the output polling work proc before acquiring the struct mutex to avoid acquiring the work proc mutex with the struct mutex held. This avoids inverting the lock order seen when the work proc runs.
I thought this was part of Daniel's reordering to avoid race conditions between interrupts and module unload. Fortunately not. The lack of locking during hotplug is worrisome should we ever do dynamic connectors.
We don't do dynamic connectors now, so adding locking with no way of actually really testing it would just mean you'd probably have just as much pain when you do add dyanamic connectors.
I also tried to think of a reason to support dynamic connectors, maybe displayport daisychaining, but none of the userspace drivers are ready for it.
Dave.
On Mon, 4 Oct 2010 20:13:33 +1000, Dave Airlie airlied@gmail.com wrote:
We don't do dynamic connectors now, so adding locking with no way of actually really testing it would just mean you'd probably have just as much pain when you do add dyanamic connectors.
I looked in the radeon and nouveau drivers and both of them make this call without holding any locks, and the call looked safe enough to me; relying only on the fact that the work proc structure had been initialized when the driver was first opened up.
dri-devel@lists.freedesktop.org
-
Chris Wilson -
Dave Airlie -
Keith Packard