[Intel-gfx] [PATCH 08/64] drm/i915: Retire oldest completed request before allocating next

[Tvrtko Ursulin]

On 07/07/16 10:45, Chris Wilson wrote:
> On Thu, Jul 07, 2016 at 10:41:03AM +0100, Tvrtko Ursulin wrote:
>>
>> On 07/07/16 09:41, Chris Wilson wrote:
>>> In order to keep the memory allocated for requests reasonably tight, try
>>> to reuse the oldest request (so long as it is completed and has no
>>> external references) for the next allocation.
>>>
>>> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
>>> ---
>>>   drivers/gpu/drm/i915/i915_gem_request.c | 7 +++++++
>>>   1 file changed, 7 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
>>> index 9e9aa6b725f7..ee1189c35509 100644
>>> --- a/drivers/gpu/drm/i915/i915_gem_request.c
>>> +++ b/drivers/gpu/drm/i915/i915_gem_request.c
>>> @@ -226,6 +226,13 @@ __i915_gem_request_alloc(struct intel_engine_cs *engine,
>>>   	if (ret)
>>>   		return ret;
>>>
>>> +	if (!list_empty(&engine->request_list)) {
>>> +		req = list_first_entry(&engine->request_list,
>>> +				       typeof(*req), list);
>>> +		if (i915_gem_request_completed(req))
>>> +			i915_gem_request_retire(req);
>>> +	}
>>> +
>>>   	req = kmem_cache_zalloc(dev_priv->requests, GFP_KERNEL);
>>>   	if (!req)
>>>   		return -ENOMEM;
>>>
>>
>> I am thinking that this does not play well with the execlists which
>> is holding references to requests for a little bit longer than they
>> are on the engine->request_list.
>>
>> In fact I don't see how you can just steal it without looking at the
>> reference count.
>
> ?
>
> There is no stealing, the request list and execlist are independently
> referenced.

Oh my bad - I read the commit message as you will take the oldest 
completed from engine->request_list instead of allocating a new one. 
Code does not do that at all. You are just trying to free one so the 
SLAB allocator might potentially reuse it.

Are you just trying to improve the retire worker? :))

Regards,

Tvrtko