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

[Tvrtko Ursulin]

On 06/07/2017 12:44, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2017-07-06 12:40:56)
>>
>> 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.
> 
> I didn't want the extra call here (since object allocation is far more
> frequent than context allocation), but that's just me.

Ah OK then, I didn't spot that angle. R-b applies to v1 then. Could be 
useful to stick a comment in there so someone does not clean it up in 
the future. :)

Regards,

Tvrtko