[Intel-gfx] [PATCH] drm/i915: Use uninterruptible mutex_lock for userptr bo creation

[Chris Wilson]

On Mon, May 18, 2015 at 06:10:07PM +0100, Damien Lespiau wrote:
> On Mon, May 18, 2015 at 10:30:06AM +0200, Daniel Vetter wrote:
> > On Fri, May 15, 2015 at 12:09:00PM +0100, Tvrtko Ursulin wrote:
> > > 
> > > Hi,
> > > 
> > > On 05/15/2015 11:42 AM, Chris Wilson wrote:
> > > >Mika encountered one pathological scenario under X where acquiring all
> > > >the mm locks (required to insert a mmu notifier) was very slow, so slow
> > > >that by the time we tried to lock the struct_mutex with the usual call
> > > >to i915_mutex_lock_interruptible(), X's signal timer had fired causing
> > > >us to restart the ioctl (and so looped indefinitely).
> > > 
> > > Indefinite loop? Are you saying userptr creation endlessly fails to manages
> > > to finish in 10ms (or is it even 100ms, forgot what timer Xorg setups up)?
> > > The __mmu_notifier_register call?
> > > 
> > > >While I suspect this is the result of another bug (something leaking mm
> > > >perhaps?) we can forgo the error checking and interuptible nature of the
> > > >lock here so we only have to pay the expense once and get on with it.
> > > >This does expose the userptr creation routine to a driver livelock
> > > >though by not being interruptible.
> > > 
> > > How is this acceptable then if it can live-lock? How does that happen?
> > 
> > If the i915 driver somehow dies it's a lot nicer for the user to be able
> > to hit ^C and get out of trouble again and debug further than make
> > anything touching i915 be stuck forever.
> > 
> > But I think this is a justified exception. Queued for -next, thanks for
> > the patch.
> 
> This breaks my SKL. Reverting make it work again.
> 
> I can still boot in init 3 to see what happens. gem_render_copy (for
> instance) will then cause an infinite loop when initializing libdrm: we
> do call GEM_USERPTR in the bufmgr init that cycles in drmIoctl, the
> ioctl() always returning -EGAIN.

Too late, Daniel's already pushed the fixed version (missing int ret = 0);
and rewrote history to hide the embarassment.  Note that also recent
libdrm doesn't call has_userptr in its init (that alsp applies to Mika! ;-).
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre