[Intel-gfx] [PATCH v2 4/4] drm/i915: Fix i915_gem_evict_for_vma (soft-pinning)

[Joonas Lahtinen]

On pe, 2016-11-18 at 10:14 +0000, Chris Wilson wrote:
> On Fri, Nov 18, 2016 at 11:18:09AM +0200, Joonas Lahtinen wrote:
> > i915_vma_is_pinned() being true will exit this loop with -ENOSPC with
> > or without NOBLOCK, just skipping the exec_entry test without it. I
> > would clarify that. Now it's bit odd.
> 
> It's a necessary test for use by execbuf. The interface is that it tests
> a location first with NONBLOCK before deciding on whether it is a good
> final location. (With various other hints as to whether any eviction is
> a good idea, vs whether it mandatory to use this location.)
> 

Well, I do not object the new way of formatting it, it's more explicit.
But does GCC do equally good job still?

> > > -		ret = i915_vma_unbind(vma);
> > > -		if (ret)
> > > -			return ret;
> > > +		__i915_vma_pin(vma);
> > 
> > I don't quite see why? Are you expecting the iteration to hit same vma
> > twice? Or somebody moving it while we iterate.
> 
> The unbind may causes a free of any member on this list, so the pinning
> prevents other vma from being unbound whilst waiting on this one. It
> used to be a big deal, but since the various reworking the deferred free
> hides the oops.

Drop a comment.

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation