Re: [PATCH 06/10] drm/i915: Choose not to evict faultable objects from the GGTT

Joonas Lahtinen
2016-08-15T10:20:24Z

On pe, 2016-08-12 at 12:13 +0100, Chris Wilson wrote:
> On Fri, Aug 12, 2016 at 01:50:56PM +0300, Joonas Lahtinen wrote:
> >
> > On pe, 2016-08-12 at 11:28 +0100, Chris Wilson wrote:
> > >
> > > @@ -1715,10 +1716,10 @@ int i915_gem_fault(struct vm_area_struct *area, struct vm_fault *vmf)
> > >   goto err_unlock;
> > >   }
> > >
> > > - /* Use a partial view if the object is bigger than the aperture. */
> > > - /* Now pin it into the GTT if needed */
> > > - vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0,
> > > -        PIN_MAPPABLE | PIN_NONBLOCK);
> > > + flags = PIN_MAPPABLE;
> > > + if (obj->base.size > 2 << 20)
> > Magic number.
> One day there may be a MiB() macro. It is a magic number, just a rule of
> thumb based on minimum chunksize for a partial.

#define the minimum chunk size and use it here too? With a warning of
the number being derived from the wildest approximations.

>
> >
> > >
> > > @@ -55,6 +55,9 @@ mark_free(struct i915_vma *vma, struct list_head *unwind)
> > >   if (WARN_ON(!list_empty(&vma->exec_list)))
> > >   return false;
> > >
> > > + if (flags & PIN_NOFAULT && vma->obj->fault_mappable)
> > > + return false;
> > The flag name is rather counter-intuitive for it describes other VMAs
> > rather than our new VMA...
> As does NONBLOCKING. We could loose this flag in favour of NOEVICT, but
> I haven't run anything to confirm if that's a good tradeoff.

Maybe the flag should be like __PIN_NOFAULTING to distinct in addition
to __PIN_NONBLOCKING? And then make sure they're never set on vma
itself.

Regards, Joonas

> -Chris
>
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation