From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lukas Wunner Subject: Re: Deadlock in intel_user_framebuffer_destroy() Date: Mon, 15 Jun 2015 09:34:04 +0200 Message-ID: <20150615073404.GA607@wunner.de> References: <20150603134332.GA5548@wunner.de> <20150603135741.GB27056@nuc-i3427.alporthouse.com> <87a8w179og.fsf@intel.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="bg08WKrSYDhXBjb5" Content-Transfer-Encoding: 7bit Return-path: Received: from mailout3.hostsharing.net (mailout3.hostsharing.net [176.9.242.54]) by gabe.freedesktop.org (Postfix) with ESMTP id 4BA6D6E209 for ; Mon, 15 Jun 2015 00:40:47 -0700 (PDT) Content-Disposition: inline In-Reply-To: <87a8w179og.fsf@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Jani Nikula Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org --bg08WKrSYDhXBjb5 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Hi Jani, On Mon, Jun 15, 2015 at 09:44:15AM +0300, Jani Nikula wrote: > On Wed, 03 Jun 2015, Chris Wilson 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 --bg08WKrSYDhXBjb5 Content-Type: text/plain; charset=iso-8859-15 Content-Disposition: attachment; filename="v2-0018-drm-i915-Fix-deadlock-upon-discarding-BIOS-fb.patch" Content-Transfer-Encoding: quoted-printable >>From b502824816e20e65f7b0b17b18efec161f7560ed Mon Sep 17 00:00:00 2001 Message-Id: In-Reply-To: <77dc50305ee6fa2db076907496e373cc237e4901.1434188640.git.luk= as@wunner.de> References: <1dcf15ad548ee6602134a71e2e1af11609b4c32b.1434188640.git.luka= s@wunner.de> <82e0890fdef2fcfa467f78a397641c39d7d9cfdd.1434188640.git.lukas@wunner.de= > <84b1a363ca1860d523ce52b11aa83239ebf2e163.1434188640.git.lukas@wunner.de= > <450b5fe4557644dbc5245a386e94c6ccc6654b92.1434188640.git.lukas@wunner.de= > <4327e6c5baf23370cfb1cd09105693978f6981d3.1434188640.git.lukas@wunner.de= > <1eb55048182574dd0443e7f83a44dbdab99bbe86.1434188640.git.lukas@wunner.de= > <226f1ae95e50070a6b17999d46c850d3abcb4496.1434188640.git.lukas@wunner.de= > <293413a2703dda0c40967031531b20fbab6214a9.1434188640.git.lukas@wunner.de= > <05704f90341a290bae3701b3ca9efc8b84729e20.1434188640.git.lukas@wunner.de= > <5875c0ca739463fae31618bd6a9f07d3953b14cc.1434188640.git.lukas@wunner.de= > <6871930e1d0f3d8dcb702bbb4f9b121d48a334ca.1434188640.git.lukas@wunner.de= > <77dc50305ee6fa2db076907496e373cc237e4901.1434188640.git.lukas@wunner.de= > From: Lukas Wunner 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=3DUTF-8 Content-Transfer-Encoding: 8bit To: dri-devel@lists.freedesktop.org From: Chris Wilson A deadlock was introduced by commit 60a5ca015ffd: Author: Ville Syrj=C3=A4l=C3=A4 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 --- 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/in= tel_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 *helpe= r, int size, ret; bool prealloc =3D false; =20 - 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 *helpe= r, drm_framebuffer_unreference(&intel_fb->base); intel_fb =3D ifbdev->fb =3D 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 =3D intelfb_alloc(helper, sizes); --=20 2.1.0 --bg08WKrSYDhXBjb5 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KSW50ZWwtZ2Z4 IG1haWxpbmcgbGlzdApJbnRlbC1nZnhAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHA6Ly9saXN0 cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9pbnRlbC1nZngK --bg08WKrSYDhXBjb5--