[Intel-gfx] Deadlock in intel_user_framebuffer_destroy()

[Jani Nikula]

On Mon, 15 Jun 2015, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> On Mon, Jun 15, 2015 at 02:02:23PM +0200, Daniel Vetter wrote:
>> On Mon, Jun 15, 2015 at 08:53:02AM +0100, Chris Wilson wrote:
>> > 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:
>> > > > 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 at 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
>> 
>> Your previous patch placed the mutex_lock before the goto out_unlock - I
>> fail to see what has been broken with that version? Can you resubmit that
>> as a proper patch with sob and Lucas' t-b?
>
> There wasn't, I just rewrote it incorrectly. There's also a
> drm_framebuffer_remove() called by intelfb_alloc which needs to be moved
> out of the mutex. A much larger disentangling of the functions involved
> here is required. Tvrstko volunteered himself. Brave, very brave :)

-ETIMEDOUT

IIUC we still need the fix, but there's no update from Lucas addressing
review. Is that right?

BR,
Jani.


> -Chris
>
> -- 
> Chris Wilson, Intel Open Source Technology Centre

-- 
Jani Nikula, Intel Open Source Technology Center