Deadlock in intel_user_framebuffer_destroy()

Submitted by Chris Wilson on June 3, 2015, 1:57 p.m.

Details

Message ID 20150603135741.GB27056@nuc-i3427.alporthouse.com
State New
Headers show

Not browsing as part of any series.

Commit Message

Chris Wilson June 3, 2015, 1:57 p.m.
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.

t a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index 6372cfc..a9b8c43 100644

Patch hide | download patch | download mbox

--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -209,8 +209,6 @@  static int intelfb_create(struct drm_fb_helper *helper,
        int size, ret;
        bool prealloc = false;
 
-       mutex_lock(&dev->struct_mutex);
-
        if (intel_fb &&
            (sizes->fb_width > intel_fb->base.width ||
             sizes->fb_height > intel_fb->base.height)) {
@@ -221,6 +219,9 @@  static int intelfb_create(struct drm_fb_helper *helper,
                drm_framebuffer_unreference(&intel_fb->base);
                intel_fb = ifbdev->fb = NULL;
        }
+
+       mutex_lock(&dev->struct_mutex);
+
        if (!intel_fb || WARN_ON(!intel_fb->obj)) {
                DRM_DEBUG_KMS("no BIOS fb, allocating a new one\n");
                ret = intelfb_alloc(helper, sizes);

Comments

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?

BR,
Jani.


>
> t a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> index 6372cfc..a9b8c43 100644
> --- a/drivers/gpu/drm/i915/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> @@ -209,8 +209,6 @@ static int intelfb_create(struct drm_fb_helper *helper,
>         int size, ret;
>         bool prealloc = false;
>  
> -       mutex_lock(&dev->struct_mutex);
> -
>         if (intel_fb &&
>             (sizes->fb_width > intel_fb->base.width ||
>              sizes->fb_height > intel_fb->base.height)) {
> @@ -221,6 +219,9 @@ static int intelfb_create(struct drm_fb_helper *helper,
>                 drm_framebuffer_unreference(&intel_fb->base);
>                 intel_fb = ifbdev->fb = NULL;
>         }
> +
> +       mutex_lock(&dev->struct_mutex);
> +
>         if (!intel_fb || WARN_ON(!intel_fb->obj)) {
>                 DRM_DEBUG_KMS("no BIOS fb, allocating a new one\n");
>                 ret = intelfb_alloc(helper, sizes);
>
> -- 
> Chris Wilson, Intel Open Source Technology Centre
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx