Currently, we only retire the oldest request on the timeline before
allocating the next, but only if there is a spare request. However,
since we rearranged the locking, e.g. commit df9f85d8582e ("drm/i915:
Serialise i915_active_fence_set() with itself"), we no longer benefit
from keeping the active chain intact underneath the struct_mutex. As
such, retire all completed requests in the client's timeline before
creating the next, trying to keep our memory and resource usage tight
and ideally only penalising the heavy users.
References: df9f85d8582e ("drm/i915: Serialise i915_active_fence_set() with
itself")
Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursu...@intel.com>
---
drivers/gpu/drm/i915/i915_request.c | 58 ++++++-----------------------
1 file changed, 11 insertions(+), 47 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_request.c
b/drivers/gpu/drm/i915/i915_request.c
index 9ed0d3bc7249..7bd622aa9c14 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -556,31 +556,20 @@ static void retire_requests(struct intel_timeline *tl)
static noinline struct i915_request *
request_alloc_slow(struct intel_timeline *tl, gfp_t gfp)
{
- struct i915_request *rq;
-
- if (list_empty(&tl->requests))
- goto out;
-
if (!gfpflags_allow_blocking(gfp))
- goto out;
+ return NULL;
- /* Move our oldest request to the slab-cache (if not in use!) */
- rq = list_first_entry(&tl->requests, typeof(*rq), link);
- i915_request_retire(rq);
-
- rq = kmem_cache_alloc(global.slab_requests,
- gfp | __GFP_RETRY_MAYFAIL | __GFP_NOWARN);
- if (rq)
- return rq;
+ if (!list_empty(&tl->requests)) {
+ struct i915_request *rq;
- /* Ratelimit ourselves to prevent oom from malicious clients */
- rq = list_last_entry(&tl->requests, typeof(*rq), link);
- cond_synchronize_rcu(rq->rcustate);
+ /* Ratelimit ourselves to prevent oom from malicious clients */
+ rq = list_last_entry(&tl->requests, typeof(*rq), link);
+ cond_synchronize_rcu(rq->rcustate);
- /* Retire our old requests in the hope that we free some */
- retire_requests(tl);
+ /* Retire our old requests in the hope that we free some */
+ retire_requests(tl);
+ }
-out:
return kmem_cache_alloc(global.slab_requests, gfp);
}
@@ -739,9 +728,7 @@ i915_request_create(struct intel_context *ce)
return ERR_CAST(tl);
/* Move our oldest request to the slab-cache (if not in use!) */
- rq = list_first_entry(&tl->requests, typeof(*rq), link);
- if (!list_is_last(&rq->link, &tl->requests))
- i915_request_retire(rq);
+ retire_requests(tl);
intel_context_enter(ce);
rq = __i915_request_create(ce, GFP_KERNEL);
@@ -1304,14 +1291,13 @@ void i915_request_add(struct i915_request *rq)
{
struct intel_timeline * const tl = i915_request_timeline(rq);
struct i915_sched_attr attr = {};
- struct i915_request *prev;
lockdep_assert_held(&tl->mutex);
lockdep_unpin_lock(&tl->mutex, rq->cookie);
trace_i915_request_add(rq);
- prev = __i915_request_commit(rq);
+ __i915_request_commit(rq);
if (rcu_access_pointer(rq->context->gem_context))
attr = i915_request_gem_context(rq)->sched;
@@ -1344,28 +1330,6 @@ void i915_request_add(struct i915_request *rq)
__i915_request_queue(rq, &attr);
local_bh_enable(); /* Kick the execlists tasklet if just scheduled */
- /*
- * In typical scenarios, we do not expect the previous request on
- * the timeline to be still tracked by timeline->last_request if it
- * has been completed. If the completed request is still here, that
- * implies that request retirement is a long way behind submission,
- * suggesting that we haven't been retiring frequently enough from
- * the combination of retire-before-alloc, waiters and the background
- * retirement worker. So if the last request on this timeline was
- * already completed, do a catch up pass, flushing the retirement queue
- * up to this client. Since we have now moved the heaviest operations
- * during retirement onto secondary workers, such as freeing objects
- * or contexts, retiring a bunch of requests is mostly list management
- * (and cache misses), and so we should not be overly penalizing this
- * client by performing excess work, though we may still performing
- * work on behalf of others -- but instead we should benefit from
- * improved resource management. (Well, that's the theory at least.)
- */
- if (prev &&
- i915_request_completed(prev) &&
- rcu_access_pointer(prev->timeline) == tl)
- i915_request_retire_upto(prev);
-
mutex_unlock(&tl->mutex);
}
--
2.25.0.rc2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
