[Intel-gfx] [PATCH] drm/i915: Double check vma placement upon gaining the vm lock

[Chris Wilson]

Quoting Tvrtko Ursulin (2019-11-26 17:33:09)
> 
> On 26/11/2019 17:25, Chris Wilson wrote:
> > Quoting Chris Wilson (2019-11-26 17:22:23)
> >> Quoting Tvrtko Ursulin (2019-11-26 17:04:43)
> >>>
> >>> On 26/11/2019 15:26, Chris Wilson wrote:
> >>>> diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
> >>>> index e5512f26e20a..f6e661428b02 100644
> >>>> --- a/drivers/gpu/drm/i915/i915_vma.c
> >>>> +++ b/drivers/gpu/drm/i915/i915_vma.c
> >>>> @@ -905,6 +905,12 @@ int i915_vma_pin(struct i915_vma *vma, u64 size, u64 alignment, u64 flags)
> >>>>                        __i915_vma_set_map_and_fenceable(vma);
> >>>>        }
> >>>>    
> >>>> +     /* Somebody else managed to gazump our placement? */
> >>>> +     if (i915_vma_misplaced(vma, size, alignment, flags)) {
> >>>> +             err = -EAGAIN;
> >>>> +             goto err_active;
> >>>> +     }
> >>>> +
> >>>
> >>> What about other callers? They will not be expecting this.
> >>
> >> The other paths should be quite happy with -EAGAIN from vma_pin, it's
> >> already part of their retry procedure. If not, there's always more duct
> >> tape. Hopefully the replacement is much simpler (stop laughing back
> >> there).
> > 
> > The alternative here is to pull in __i915_vma_unbind(), which might be
> > plausible.
> 
> To make unbind and pin atomic? You'd need unlocked vma_pin as well. Or 
> some different idea?

Originally I had planned for an unlocked vma_pin, so that we would take
the lock once in i915_gem_object_ggtt_pin() and do the migration there.
Current plan for quick fix is to add

	if (i915_vma_misplaced() {
		err = __i915_vma_bind();
		if (err)
			goto foo;
	}

to i915_vma_pin() and see how many apple carts that upsets.
-Chris