[Intel-gfx] [PATCH] drm/i915: During shrink_all we only need to idle the GPU

[Chris Wilson]

On Mon, Jun 29, 2015 at 09:13:31AM +0200, Daniel Vetter wrote:
> On Sun, Jun 28, 2015 at 01:06:39PM +0100, Chris Wilson wrote:
> > We can forgo an evict-everything here as the shrinker operation itself
> > will unbind any vma as required. If we explicitly idle the GPU through a
> > switch to the default context, we not only create a request in an
> > illegal context (e.g. whilst shrinking during execbuf with a request
> > already allocated), but switching to the default context will not free
> > up the memory backing the active contexts - unless in the unlikely
> > situation that context had already been closed (and just kept arrive by
> > being the current context). The saving is near zero and the danger real.
> 
> Has this already blown up in some bugzilla somewhere? Should be a fairly
> recent regression with the olr removal.

It's damn tricky to catch in an igt, since it requires an oom pass
inside the submit callback. Running out of memory is easy, running out
of memory with outstanding rendering is easy, triggering the recursion
is hard. (Which of course meant it happened almost immediately when
running benchmarks on resource limited machines.) Fault injection is the
only tractible solution. (And don't think what happens if we cancel a
request after doing a switch_context, stale requests galore.)

> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/i915_gem_shrinker.c | 13 ++++++++++---
> >  1 file changed, 10 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> > index c41ddf92e404..2d8c79b8c378 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> > @@ -158,9 +158,16 @@ i915_gem_shrink(struct drm_i915_private *dev_priv,
> >   */
> >  unsigned long i915_gem_shrink_all(struct drm_i915_private *dev_priv)
> >  {
> > -	i915_gem_evict_everything(dev_priv->dev);
> > -	return i915_gem_shrink(dev_priv, LONG_MAX,
> > -			       I915_SHRINK_BOUND | I915_SHRINK_UNBOUND);
> > +	unsigned long count;
> > +
> > +	count = i915_gem_shrink(dev_priv, LONG_MAX,
> > +				I915_SHRINK_BOUND | I915_SHRINK_UNBOUND);
> > +
> > +	/* Use a double call to retire to flush any staged frees  */
> > +	i915_gem_retire_requests(dev_priv->dev);
> > +	i915_gem_retire_requests(dev_priv->dev);
> 
> I'm lost - where's that staged free?

Execlists has one, and I'm adding another.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre