On Fri, Nov 18, 2016 at 11:18:09AM +0200, Joonas Lahtinen wrote:
> On to, 2016-11-17 at 12:08 +0000, Chris Wilson wrote:
> > + if (flags & PIN_NONBLOCK &&
> > + (i915_vma_is_pinned(vma) || i915_vma_is_active(vma))) {
> > + ret = -ENOSPC;
> > + break;
> > + }
>
> 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.)
> > - return -ENOSPC;
> > + /* Overlap of objects in the same batch? */
> > + if (i915_vma_is_pinned(vma)) {
> > + ret = -ENOSPC;
> > + if (vma->exec_entry &&
> > + vma->exec_entry->flags & EXEC_OBJECT_PINNED)
> > + ret = -EINVAL;
> > + break;
> > }
> >
> > - 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.
> > + list_add(&vma->exec_list, &eviction_list);
>
> I'd prefer an union instead of brutally reusing member for other
> purposes.
There have been patches to add evict_link :-p
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
