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

From: Daniel Vetter <daniel@ffwll.ch>
To: Chris Wilson <chris@chris-wilson.co.uk>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915: During shrink_all we only need to idle the GPU
Date: Mon, 29 Jun 2015 09:13:31 +0200	[thread overview]
Message-ID: <20150629071331.GS30960@phenom.ffwll.local> (raw)
In-Reply-To: <1435493199-26869-1-git-send-email-chris@chris-wilson.co.uk>

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.

> Signed-off-by: Chris Wilson <chris@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?
-Daniel

> +
> +	return count;
>  }
>  
>  static bool i915_gem_shrinker_lock(struct drm_device *dev, bool *unlock)
> -- 
> 2.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx