Re: Deadlock in intel_user_framebuffer_destroy()

Chris Wilson
2015-06-15T07:53:08Z

On Mon, Jun 15, 2015 at 09:44:15AM +0300, Jani Nikula wrote:
> On Wed, 03 Jun 2015, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > On Wed, Jun 03, 2015 at 03:43:32PM +0200, Lukas Wunner wrote:
> >> Hi,
> >>
> >> a deadlock was introduced by commit 60a5ca015ffd2aacfe5674b5a401cd2a37159e07
> >>
> >> Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> Date: Fri Jun 13 11:10:53 2014 +0300
> >>
> >> drm/i915: Add locking around framebuffer_references--
> >>
> >>
> >> The commit amended intel_display.c:intel_user_framebuffer_destroy() with
> >> mutex_lock(&dev->struct_mutex).
> >>
> >> A few weeks prior Chris Wilson had amended intel_fbdev.c:intelfb_create()
> >> with a call to drm_framebuffer_unreference() while &dev->struct_mutex is
> >> locked (commit edd586fe705e819bc711b5ed7194a0b6f9f1a7e1, "drm/i915: Discard
> >> BIOS framebuffers too small to accommodate chosen mode").
> >>
> >
> > Just move the mutex_lock down a step.
>
> Lucas, did you try this?

There's a goto unlock that also needed to be disabled, such as

diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index dda99c0d6be1..fc7ec5138fb7 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -213,8 +213,6 @@ static int intelfb_create(struct drm_fb_helper *helper,
bool prealloc = false;
int ret;

- mutex_lock(&dev->struct_mutex);
-
if (intel_fb &&
(sizes->fb_width > intel_fb->base.width ||
sizes->fb_height > intel_fb->base.height)) {
@@ -229,7 +227,7 @@ static int intelfb_create(struct drm_fb_helper *helper,
DRM_DEBUG_KMS("no BIOS fb, allocating a new one\n");
ret = intelfb_alloc(helper, sizes);
if (ret)
- goto out_unlock;
+ return ret;
intel_fb = ifbdev->fb;
} else {
DRM_DEBUG_KMS("re-using BIOS fb\n");
@@ -241,6 +239,7 @@ static int intelfb_create(struct drm_fb_helper *helper,
obj = intel_fb->obj;
vma = i915_gem_obj_to_ggtt(obj, NULL);

+ mutex_lock(&dev->struct_mutex);
info = framebuffer_alloc(0, &dev->pdev->dev);
if (!info) {
ret = -ENOMEM;
@@ -311,7 +310,6 @@ static int intelfb_create(struct drm_fb_helper *helper,
out_unpin:
drm_gem_object_unreference(&obj->base);
-out_unlock:
mutex_unlock(&dev->struct_mutex);
return ret;
}

--
Chris Wilson, Intel Open Source Technology Centre