[Intel-gfx] [PATCH v2 11/16] drm/i915: Call i915_gem_evict_vm in vm_fault_gtt to prevent new ENOSPC errors

[Matthew Auld]

On Mon, 29 Nov 2021 at 13:58, Maarten Lankhorst
<maarten.lankhorst at linux.intel.com> wrote:
>
> Now that we cannot unbind kill the currently locked object directly

Can this be reworded slightly? Not sure what is meant by "unbind kill" here.

> because we're removing short term pinning, we may have to unbind the
> object from gtt manually, using a i915_gem_evict_vm() call.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/gem/i915_gem_mman.c | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> index 65fc6ff5f59d..6d557bb9926f 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> @@ -357,8 +357,22 @@ static vm_fault_t vm_fault_gtt(struct vm_fault *vmf)
>                         vma = i915_gem_object_ggtt_pin_ww(obj, &ww, &view, 0, 0, flags);
>                 }
>
> -               /* The entire mappable GGTT is pinned? Unexpected! */
> -               GEM_BUG_ON(vma == ERR_PTR(-ENOSPC));
> +               /*
> +                * The entire mappable GGTT is pinned? Unexpected!
> +                * Try to evict the object we locked too, as normally we skip it
> +                * due to lack of short term pinning inside execbuf.
> +                */
> +               if (vma == ERR_PTR(-ENOSPC)) {
> +                       ret = mutex_lock_interruptible(&ggtt->vm.mutex);
> +                       if (!ret) {
> +                               ret = i915_gem_evict_vm(&ggtt->vm, &ww);

Would it make sense to pass an extra flag for the above ggtt_pin(maybe
PIN_EVICT_SHARED)? Such that evict_for_something can handle the
already locked object and then also any vma sharing the same dma-resv
object here? Or at least trying to nuke the entire vm, just for the
mappable portion seems maybe overkill? Or perhaps we never expect to
hit this in the real world?

Reviewed-by: Matthew Auld <matthew.auld at intel.com>

> +                               mutex_unlock(&ggtt->vm.mutex);
> +                       }
> +                       if (ret)
> +                               goto err_reset;
> +                       vma = i915_gem_object_ggtt_pin_ww(obj, &ww, &view, 0, 0, flags);
> +               }
> +               GEM_WARN_ON(vma == ERR_PTR(-ENOSPC));
>         }
>         if (IS_ERR(vma)) {
>                 ret = PTR_ERR(vma);
> --
> 2.34.0
>