[Intel-gfx] [PATCH 34/39] drm/i915: Use vm->mutex for serialising GTT insertion

[Chris Wilson]

Quoting Matthew Auld (2019-06-14 20:14:27)
> On Fri, 14 Jun 2019 at 08:11, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> >
> > Serialising insertion into each of the global GTT and ppGTT accounts for
> > a large chunk of the current struct_mutex serialisation requireemnts.
> > (Note that it is not just the drm_mm / gtt management itself being
> > serialised, but the pin count and various flags.) Previously, the main
> > blocker for replacing this mutex was the reset handing, but with the
> > advent of "lockless" resets, we can freely take the vm->mutex and block
> > waiting for the GPU (without fear of deadlock if the GPU hangs). We also
> > proscribe allocations underneath vm->mutex, allowing us to take the
> > mutex inside the shrinker, avoiding the recursive struct_mutex we
> > previously used.
> >
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > ---
> 
> [snip]
> 
> > @@ -248,16 +248,6 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
> >                 goto err_rpm;
> >         }
> >
> > -       ret = i915_mutex_lock_interruptible(dev);
> > -       if (ret)
> > -               goto err_reset;
> > -
> > -       /* Access to snoopable pages through the GTT is incoherent. */
> > -       if (obj->cache_level != I915_CACHE_NONE && !HAS_LLC(i915)) {
> > -               ret = -EFAULT;
> > -               goto err_unlock;
> > -       }
> 
> Intentional? Perhaps harder to enforce now?

Removed, then thought of how to lock it correctly. I was chickening out
at set_cache_level, but all it really needs is object_lock although
there's some scope to moving the cache level onto the vma and then we
would only need to check after pinning the vma (so long as then allow
waiting on pinned vma for set_cache_level). I thought I might have been
able to justify it, but on second thoughts I think it is a machine killer
for older gen iirc so probably best not left open.
-Chris