| Message ID | 20190109131422.18966-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] drm/i915: Reduce i915_request_alloc retirement to local context | expand |
On 09/01/2019 13:14, Chris Wilson wrote: > In the continual quest to reduce the amount of global work required when > submitting requests, replace i915_retire_requests() after allocation > failure to retiring just our ring. > > v2: Don't forget the list iteration included an early break, so we would > never throttle on the last request in the ring/timeline. > > References: 11abf0c5a021 ("drm/i915: Limit the backpressure for i915_request allocation") > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > --- > drivers/gpu/drm/i915/i915_request.c | 35 +++++++++++++++++++++-------- > 1 file changed, 26 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c > index 1e158eb8cb97..e183009f47f4 100644 > --- a/drivers/gpu/drm/i915/i915_request.c > +++ b/drivers/gpu/drm/i915/i915_request.c > @@ -477,6 +477,31 @@ submit_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state) > return NOTIFY_DONE; > } > > +static noinline struct i915_request * > +i915_request_alloc_slow(struct intel_context *ce) > +{ > + struct intel_ring *ring = ce->ring; > + struct i915_request *rq, *next; > + > + if (list_empty(&ring->request_list)) > + goto out; > + > + /* Ratelimit ourselves to prevent oom from malicious clients */ > + rq = list_last_entry(&ring->request_list, typeof(*rq), ring_link); > + cond_synchronize_rcu(rq->rcustate); > + > + /* Retire our old requests in the hope that we free some */ > + list_for_each_entry_safe(rq, next, &ring->request_list, ring_link) { > + if (!i915_request_completed(rq)) > + break; > + > + i915_request_retire(rq); > + } Now this is more similar to ring_retire_requests. ;) > + > +out: > + return kmem_cache_alloc(ce->gem_context->i915->requests, GFP_KERNEL); > +} > + > /** > * i915_request_alloc - allocate a request structure > * > @@ -559,15 +584,7 @@ i915_request_alloc(struct intel_engine_cs *engine, struct i915_gem_context *ctx) > rq = kmem_cache_alloc(i915->requests, > GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN); > if (unlikely(!rq)) { > - i915_retire_requests(i915); > - > - /* Ratelimit ourselves to prevent oom from malicious clients */ > - rq = i915_gem_active_raw(&ce->ring->timeline->last_request, > - &i915->drm.struct_mutex); > - if (rq) > - cond_synchronize_rcu(rq->rcustate); > - > - rq = kmem_cache_alloc(i915->requests, GFP_KERNEL); > + rq = i915_request_alloc_slow(ce); > if (!rq) { > ret = -ENOMEM; > goto err_unreserve; > With the retire consolidated: Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Regards, Tvrtko
diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index 1e158eb8cb97..e183009f47f4 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -477,6 +477,31 @@ submit_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state) return NOTIFY_DONE; } +static noinline struct i915_request * +i915_request_alloc_slow(struct intel_context *ce) +{ + struct intel_ring *ring = ce->ring; + struct i915_request *rq, *next; + + if (list_empty(&ring->request_list)) + goto out; + + /* Ratelimit ourselves to prevent oom from malicious clients */ + rq = list_last_entry(&ring->request_list, typeof(*rq), ring_link); + cond_synchronize_rcu(rq->rcustate); + + /* Retire our old requests in the hope that we free some */ + list_for_each_entry_safe(rq, next, &ring->request_list, ring_link) { + if (!i915_request_completed(rq)) + break; + + i915_request_retire(rq); + } + +out: + return kmem_cache_alloc(ce->gem_context->i915->requests, GFP_KERNEL); +} + /** * i915_request_alloc - allocate a request structure * @@ -559,15 +584,7 @@ i915_request_alloc(struct intel_engine_cs *engine, struct i915_gem_context *ctx) rq = kmem_cache_alloc(i915->requests, GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN); if (unlikely(!rq)) { - i915_retire_requests(i915); - - /* Ratelimit ourselves to prevent oom from malicious clients */ - rq = i915_gem_active_raw(&ce->ring->timeline->last_request, - &i915->drm.struct_mutex); - if (rq) - cond_synchronize_rcu(rq->rcustate); - - rq = kmem_cache_alloc(i915->requests, GFP_KERNEL); + rq = i915_request_alloc_slow(ce); if (!rq) { ret = -ENOMEM; goto err_unreserve;
In the continual quest to reduce the amount of global work required when submitting requests, replace i915_retire_requests() after allocation failure to retiring just our ring. v2: Don't forget the list iteration included an early break, so we would never throttle on the last request in the ring/timeline. References: 11abf0c5a021 ("drm/i915: Limit the backpressure for i915_request allocation") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> --- drivers/gpu/drm/i915/i915_request.c | 35 +++++++++++++++++++++-------- 1 file changed, 26 insertions(+), 9 deletions(-)