[Intel-gfx] [PATCH] drm/i915: Only free the oldest stale object before a fresh allocation

[Tvrtko Ursulin]

On 05/07/2017 18:21, Chris Wilson wrote:
> Inspired by Tvrtko's critique of the reaping of the stale contexts
> before allocating a new one, also limit the freed object reaping to the
> oldest stale object before allocating a fresh object. Unlike contexts,
> objects may have radically different sizes of backing storage, but
> similar to contexts, whilst we want to prevent starvation due to
> excessive freed lists, we also want do not want to delay fresh
> allocations for too long. Only freeing the oldest on the freed object
> list before each allocation is a reasonable compromise.
> 
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> ---
>   drivers/gpu/drm/i915/i915_gem.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 91145c39cd43..2048532c9cae 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4473,9 +4473,13 @@ static void i915_gem_flush_free_objects(struct drm_i915_private *i915)
>   {
>   	struct llist_node *freed;
>   
> -	freed = llist_del_all(&i915->mm.free_list);
> -	if (unlikely(freed))
> +	freed = NULL;
> +	if (!llist_empty(&i915->mm.free_list))
> +		freed = llist_del_first(&i915->mm.free_list);

Looks like llist_del_first already handles the empty list case by 
returning NULL so you could simplify this.

> +	if (unlikely(freed)) { > +		freed->next = NULL;
>   		__i915_gem_free_objects(i915, freed);
> +	}
>   }
>   
>   static void __i915_gem_free_work(struct work_struct *work)
> 

With the simplification:

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>

Regards,

Tvrtko