[Intel-gfx] Deadlock in intel_user_framebuffer_destroy()

[Lukas Wunner]

Hi Jani,

On Mon, Jun 15, 2015 at 09:44:15AM +0300, Jani Nikula wrote:
> On Wed, 03 Jun 2015, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> > Just move the mutex_lock down a step.
> Lucas, did you try this?

Yes, works for me. I was going to submit the attached patch as part of
the MacBook Pro GPU switching patch set (once it's finished) but feel
free to merge now.

Thanks for following up on this.

Lukas
-------------- next part --------------
>From b502824816e20e65f7b0b17b18efec161f7560ed Mon Sep 17 00:00:00 2001
Message-Id: <b502824816e20e65f7b0b17b18efec161f7560ed.1434188640.git.lukas at wunner.de>
In-Reply-To: <77dc50305ee6fa2db076907496e373cc237e4901.1434188640.git.lukas at wunner.de>
References: <1dcf15ad548ee6602134a71e2e1af11609b4c32b.1434188640.git.lukas at wunner.de>
	<82e0890fdef2fcfa467f78a397641c39d7d9cfdd.1434188640.git.lukas at wunner.de>
	<84b1a363ca1860d523ce52b11aa83239ebf2e163.1434188640.git.lukas at wunner.de>
	<450b5fe4557644dbc5245a386e94c6ccc6654b92.1434188640.git.lukas at wunner.de>
	<4327e6c5baf23370cfb1cd09105693978f6981d3.1434188640.git.lukas at wunner.de>
	<1eb55048182574dd0443e7f83a44dbdab99bbe86.1434188640.git.lukas at wunner.de>
	<e395b70b5680ec8551848550f90f563250323fe8.1434188640.git.lukas at wunner.de>
	<226f1ae95e50070a6b17999d46c850d3abcb4496.1434188640.git.lukas at wunner.de>
	<a151a053833b7e6561479b8e24ab6a669a0f6673.1434188640.git.lukas at wunner.de>
	<afab3acad3d94c25b7f7582d37dd4ec3b35afa3b.1434188640.git.lukas at wunner.de>
	<293413a2703dda0c40967031531b20fbab6214a9.1434188640.git.lukas at wunner.de>
	<05704f90341a290bae3701b3ca9efc8b84729e20.1434188640.git.lukas at wunner.de>
	<5875c0ca739463fae31618bd6a9f07d3953b14cc.1434188640.git.lukas at wunner.de>
	<a5bac6cead2e8f6e3e481abdf5f3476883be64d0.1434188640.git.lukas at wunner.de>
	<6871930e1d0f3d8dcb702bbb4f9b121d48a334ca.1434188640.git.lukas at wunner.de>
	<a6d60e1981004ba1e8dc4ce37b78657b2edf61c8.1434188640.git.lukas at wunner.de>
	<77dc50305ee6fa2db076907496e373cc237e4901.1434188640.git.lukas at wunner.de>
From: Lukas Wunner <lukas at wunner.de>
Date: Wed, 3 Jun 2015 14:57:41 +0100
Subject: [PATCH v2 18/20] drm/i915: Fix deadlock upon discarding BIOS fb
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
To: dri-devel at lists.freedesktop.org

From: Chris Wilson <chris at chris-wilson.co.uk>

A deadlock was introduced by commit 60a5ca015ffd:

Author: Ville SyrjÀlÀ <ville.syrjala at linux.intel.com>
Date:   Fri Jun 13 11:10:53 2014 +0300

    drm/i915: Add locking around framebuffer_references--

The commit amended intel_user_framebuffer_destroy() with locking of
dev->struct_mutex.

A few weeks prior Chris Wilson had amended intelfb_create() with a call
to drm_framebuffer_unreference() while dev->struct_mutex is locked
(edd586fe705e, "drm/i915: Discard BIOS framebuffers too small to
accommodate chosen mode"). That function calls drm_framebuffer_free(),
which calls intel_user_framebuffer_destroy(), which now tries to acquire
the lock once more, rendering the functionality added by Chris Wilson
broken.

Leave the offending commit as is but adapt Chris Wilson's code by moving
mutex_lock() below the call to drm_framebuffer_unreference().

Fixes: 60a5ca015ffd2aacfe5674b5a401cd2a37159e07
[Lukas: Amend Chris' patch with commit message]
Signed-off-by: Lukas Wunner <lukas at wunner.de>
---
 drivers/gpu/drm/i915/intel_fbdev.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index 4e7e7da..bea3218 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -183,8 +183,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)) {
@@ -195,6 +193,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);
-- 
2.1.0