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@xxxxxxxxxxxxxxxxxx> > > Cc: Michał Winiarski <michal.winiarski@xxxxxxxxx> > > Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> > > Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> > > 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
