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

[Chris Wilson]

On Thu, Jul 07, 2016 at 11:03:42AM +0100, Tvrtko Ursulin wrote:
> 
> 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? :))

The issue down the line is that we increase the laziness of retirement,
principally to avoid long stalls as we free lots of requests, objects
and pages. The problem with going too lazy is then we starve the system
of memory (and just move the long stall to the extreme case of hitting
the direct reclaim). We curtail that (in order to protect ourselves from
provoking an oom) by handling the RCU at that moment. As alluded to in
that future patch, the other approach is throttling at the point of
consumption. This adds the lightweight version of that with the
advantage of trying to keep the slab cache ready for a new allocation.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre