[Intel-gfx] [PATCH 3/3] drm/i915: Use a task to cancel the userptr on invalidate_range

[Chris Wilson]

On Tue, Oct 06, 2015 at 02:04:19PM +0200, Daniel Vetter wrote:
> On Thu, Oct 01, 2015 at 12:34:47PM +0100, Chris Wilson wrote:
> > Whilst discussing possible ways to trigger an invalidate_range on a
> > userptr with an aliased GGTT mmapping (and so cause a struct_mutex
> > deadlock), the conclusion is that we can, and we must, prevent any
> > possible deadlock by avoiding taking the mutex at all during
> > invalidate_range. This has numerous advantages all of which stem from
> > avoid the sleeping function from inside the unknown context. In
> > particular, it simplifies the invalidate_range because we no longer
> > have to juggle the spinlock/mutex and can just hold the spinlock
> > for the entire walk. To compensate, we have to make get_pages a bit more
> > complicated in order to serialise with a pending cancel_userptr worker.
> > As we hold the struct_mutex, we have no choice but to return EAGAIN and
> > hope that the worker is then flushed before we retry after reacquiring
> > the struct_mutex.
> > 
> > The important caveat is that the invalidate_range itself is no longer
> > synchronous. There exists a small but definite period in time in which
> > the old PTE's page remain accessible via the GPU. Note however that the
> > physical pages themselves are not invalidated by the mmu_notifier, just
> > the CPU view of the address space. The impact should be limited to a
> > delay in pages being flushed, rather than a possibility of writing to
> > the wrong pages. The only race condition that this worsens is remapping
> > an userptr active on the GPU where fresh work may still reference the
> > old pages due to struct_mutex contention. Given that userspace is racing
> > with the GPU, it is fair to say that the results are undefined.
> > 
> > v2: Only queue (and importantly only take one refcnt) the worker once.
> > 
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Michał Winiarski <michal.winiarski at intel.com>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> > Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> 
> Pulled in all 3 patches. Btw some pretty kerneldoc explaining the
> high-level interactions would be neat for all the userptr stuff ... I'm
> totally lost in i915_gem_userptr.c ;-)

Probably because it already has too many verbose comments?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre