Hi,

On 06/29/2015 03:39 PM, Lukas Wunner wrote:
Hi,

On Mon, Jun 15, 2015 at 11:49:52AM +0100, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin <tvrtko.ursu...@intel.com>

We had two failure modes here:

1.
Deadlock in intelfb_alloc failure path where it calls drm_framebuffer_remove,
which grabs the struct mutex and intelfb_create (caller of intelfb_alloc) was
already holding it.

s/intelfb_alloc/intelfb_create/
s/(caller of intelfb_alloc)//

I looked again and don't see that I made a mistake with regards to this?

2.
Double unreference on the object if __intel_framebuffer_create fails since
both it and the caller (intelfb_alloc) do the unreference.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursu...@intel.com>
Cc: Chris Wilson <ch...@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrj...@linux.intel.com>

Fixes: 60a5ca015ffd2aacfe5674b5a401cd2a37159e07
Reported-By: Lukas Wunner <lu...@wunner.de>

Sorry probably my oversight, don't remember now how it all came about.

@@ -183,15 +186,18 @@ static int intelfb_alloc(struct drm_fb_helper *helper,
                goto out_fb;
        }

+       mutex_unlock(&dev->struct_mutex);
+
        ifbdev->fb = to_intel_framebuffer(fb);

        return 0;

  out_fb:
-       drm_framebuffer_remove(fb);
-out_unref:
        drm_gem_object_unreference(&obj->base);
  out:
+       mutex_unlock(&dev->struct_mutex);
+       if (fb)
+               drm_framebuffer_remove(fb);

Hm, the order of drm_gem_object_unreference() and drm_framebuffer_remove()
is reversed now, could this cause any issues?

No, that is fine.

Apart from that,

Reviewed-By: Lukas Wunner <lu...@wunner.de>

I will also test the patch and report back in a bit.

That will be very useful. I did not test it extensively myself, just fired it off quickly since I was briefly hitting this failure path myself.

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to