[Intel-gfx] [PATCH 5/5] drm/i915: Use partial view in mmap fault handler

[Chris Wilson]

On Mon, Apr 27, 2015 at 02:01:59PM +0300, Joonas Lahtinen wrote:
> On pe, 2015-04-24 at 13:33 +0100, Chris Wilson wrote:
> > On Fri, Apr 24, 2015 at 03:10:20PM +0300, Joonas Lahtinen wrote:
> > > Use partial view for huge BOs (bigger than half the mappable aperture)
> > > in fault handler so that they can be accessed withough trying to make
> > > room for them by evicting other objects.
> > > 
> > > Signed-off-by: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_gem.c |   67 ++++++++++++++++++++++++++-------------
> > >  1 file changed, 45 insertions(+), 22 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > > index c2c1819..eb30cee 100644
> > > --- a/drivers/gpu/drm/i915/i915_gem.c
> > > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > > @@ -1635,6 +1635,7 @@ int i915_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
> > >  	struct drm_i915_gem_object *obj = to_intel_bo(vma->vm_private_data);
> > >  	struct drm_device *dev = obj->base.dev;
> > >  	struct drm_i915_private *dev_priv = dev->dev_private;
> > > +	struct i915_ggtt_view view = i915_ggtt_view_normal;
> > >  	pgoff_t page_offset;
> > >  	unsigned long pfn;
> > >  	int ret = 0;
> > > @@ -1667,8 +1668,21 @@ int i915_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
> > >  		goto unlock;
> > >  	}
> > >  
> > > -	/* Now bind it into the GTT if needed */
> > > -	ret = i915_gem_obj_ggtt_pin(obj, 0, PIN_MAPPABLE);
> > > +	/* Use a partial view if the object is bigger than half the aperture. */
> > > +	if (obj->base.size > dev_priv->gtt.mappable_end/2) {
> > > +		static const size_t chunk_size = 256; // 1 MiB
> > > +		memset(&view, 0, sizeof(view));
> > > +		view.type = I915_GGTT_VIEW_PARTIAL;
> > > +		view.params.partial.offset = rounddown(page_offset, chunk_size);
> > > +		view.params.partial.size =
> > > +			min_t(size_t,
> > > +			      chunk_size,
> > > +			      (vma->vm_end - vma->vm_start)/PAGE_SIZE -
> > > +			      view.params.partial.offset);
> > 
> > This isn't what I was imagining.
> > 
> > I was expecting to see error handling inside the fault path if we could
> > not pin the object. This way we could handle fragmentation or display
> > objects pinned outside the aperture.
> 
> After discussion with Daniel, the idea was dropped due to high amount of
> trashing which would occur if each object would be attempted to fit to
> the mappable aperture for each fault to that object.

The point is that we fail to install a partial view for pinned objects
outside of the aperture. Or did I miss how you handle them?

I would suggest try first with a PIN_MAPPABLE | PIN_NOEVICT then. You
can limit thrashing by a vma->release callback to only zap the PTE
mapping into that vma. Also remember to destroy all of the partial vma
when we zap the mapping for an object.

> An alternate path could be added to mark an object partially_mappable if
> mapping it normally fails, then further attempts could be skipped to
> avoid the trashing. But it is still questionable if an object close to
> the whole aperture size should be attempted to map normally, which will
> cause huge trashing if there are plenty of objects in the mappable
> aperture (that problem existed before, if the object was just short of
> the aperture size, it was attempted to be mapped).

Just treat the aperture as first come first served, with perhaps a
reservation of the first 256k for transient partial vma. Fix thrashing
when we a usecase that falls foul of the page-fault-of-doom, but first
try to write one. I think with a mru list of individually faulted vma
thrashing will be limited (i.e. no worse than current mru eviction and
possibly better albeit at a higher pagefault cost).

Also I found a usecase for temporarily mapping invidual pages:
http://cgit.freedesktop.org/~ickle/linux-2.6/commit/?h=bug90137&id=40e8a9cddb4a1e39a2fdcff4878b61c666e1a51e
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre