All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chris Wilson <chris@chris-wilson.co.uk>
To: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: intel-gfx <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 5/5] drm/i915: Use partial view in mmap fault handler
Date: Mon, 27 Apr 2015 12:21:25 +0100	[thread overview]
Message-ID: <20150427112125.GO599@nuc-i3427.alporthouse.com> (raw)
In-Reply-To: <1430132519.3390.6.camel@jlahtine-mobl1>

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@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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2015-04-27 11:21 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <cover.1429876733.git.joonas.lahtinen@linux.intel.com>
2015-04-24 12:09 ` [PATCH 1/5] drm/i915: Do not clear mappings beyond VMA size Joonas Lahtinen
2015-04-27 12:55   ` Tvrtko Ursulin
2015-05-04 14:23     ` Daniel Vetter
2015-04-24 12:09 ` [PATCH 2/5] drm/i915: Do not make assumptions on GGTT VMA sizes Joonas Lahtinen
2015-04-27 13:55   ` Tvrtko Ursulin
2015-04-28  7:23     ` Joonas Lahtinen
2015-04-24 12:09 ` [PATCH 3/5] drm/i915: Consider object pinned if any VMA is pinned Joonas Lahtinen
2015-04-24 12:29   ` Chris Wilson
2015-04-27 12:18     ` Joonas Lahtinen
2015-04-24 12:09 ` [PATCH 4/5] drm/i915: Add a partial GGTT view type Joonas Lahtinen
2015-04-27 14:50   ` Tvrtko Ursulin
2015-04-28  8:38     ` Tvrtko Ursulin
2015-04-30 11:02     ` Joonas Lahtinen
2015-04-24 12:10 ` [PATCH 5/5] drm/i915: Use partial view in mmap fault handler Joonas Lahtinen
2015-04-24 12:33   ` Chris Wilson
2015-04-27 11:01     ` Joonas Lahtinen
2015-04-27 11:21       ` Chris Wilson [this message]
2015-04-27 12:12         ` Joonas Lahtinen
2015-04-27 12:25           ` Chris Wilson
2015-04-27 13:46             ` Joonas Lahtinen
2015-04-27 14:52               ` Chris Wilson
2015-04-24 15:28   ` shuang.he

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150427112125.GO599@nuc-i3427.alporthouse.com \
    --to=chris@chris-wilson.co.uk \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=joonas.lahtinen@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.